Paul,

Thank you!

-Dmitry


On 31.10.2017 20:32, Paul Sandoz wrote:
> 
>> On 31 Oct 2017, at 05:58, Dmitry Samersoff <dmitry.samers...@bell-sw.com> 
>> wrote:
>>
>> Paul and Frederic,
>>
>> Thank you.
>>
>> One more question. Do we need to call verify_oop below?
>>
>> 509   { // Check for the null sentinel.
>> ...
>> 517      xorptr(result, result);  // NULL object reference
>> ...
>>
>> 521   if (VerifyOops) {
>> 522      verify_oop(result);
>> 523   }
>>
> 
> I believe it’s harmless.
> 
> When the flag is on it eventually results in a call to the stub generated by 
> generate_verify_oop:
> 
> http://hg.openjdk.java.net/jdk10/hs/file/tip/src/hotspot/cpu/x86/stubGenerator_x86_64.cpp#l1023
> 
>     // make sure object is 'reasonable'
>     __ testptr(rax, rax);
>     __ jcc(Assembler::zero, exit); // if obj is NULL it is OK
> 
> If the oop is null the verification will exit safely.
> 
> Paul.
> 
>> -Dmitry
>>
>>
>> On 31.10.2017 00:56, Frederic Parain wrote:
>>> I’m seeing no issue with rcx being aliased in this code.
>>>
>>> Fred
>>>
>>>> On Oct 30, 2017, at 15:44, Paul Sandoz <paul.san...@oracle.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks for reviewing.
>>>>
>>>>> On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samers...@bell-sw.com> 
>>>>> wrote:
>>>>>
>>>>> Paul,
>>>>>
>>>>> templateTable_x86.cpp:
>>>>>
>>>>> 564   const Register flags = rcx;
>>>>> 565   const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
>>>>>
>>>>> Should we use another register for rarg under NOT_LP64 ?
>>>>>
>>>>
>>>> I think it should be ok, it i ain’t an expert here on the interpreter and 
>>>> the calling conventions, so please correct me.
>>>>
>>>> Some more context:
>>>>
>>>> +  const Register flags = rcx;
>>>> +  const Register rarg = NOT_LP64(rcx) LP64_ONLY(c_rarg1);
>>>> +  __ movl(rarg, (int)bytecode());
>>>>
>>>> The current bytecode code is loaded into “rarg”
>>>>
>>>> +  call_VM(obj, CAST_FROM_FN_PTR(address, 
>>>> InterpreterRuntime::resolve_ldc), rarg);
>>>>
>>>> Then “rarg" is the argument to the call to 
>>>> InterpreterRuntime::resolve_ldc, after which it is no longer referred to.
>>>>
>>>> +#ifndef _LP64
>>>> +  // borrow rdi from locals
>>>> +  __ get_thread(rdi);
>>>> +  __ get_vm_result_2(flags, rdi);
>>>> +  __ restore_locals();
>>>> +#else
>>>> +  __ get_vm_result_2(flags, r15_thread);
>>>> +#endif
>>>>
>>>> The result from the call is then loaded into flags.
>>>>
>>>> So i don’t think it matters in this case if rcx is aliased.
>>>>
>>>> Paul.
>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 10/26/2017 08:03 PM, Paul Sandoz wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review the following patch for minimal dynamic constant support:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/
>>>>>>  
>>>>>> <http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8186046-minimal-condy-support-hs/webrev/>
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8186046 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8186046>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8186209 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8186209>
>>>>>>
>>>>>> This patch is based on the JDK 10 unified HotSpot repository. Testing so 
>>>>>> far looks good.
>>>>>>
>>>>>> By minimal i mean just the support in the runtime for a dynamic constant 
>>>>>> pool entry to be referenced by a LDC instruction or a bootstrap method 
>>>>>> argument. Much of the work leverages the foundations built by invoke 
>>>>>> dynamic but is arguably simpler since resolution is less complex.
>>>>>>
>>>>>> A small set of bootstrap methods will be proposed as a follow on issue 
>>>>>> for 10 (these are currently being refined in the amber repository).
>>>>>>
>>>>>> Bootstrap method invocation has not changed (and the rules are the same 
>>>>>> for dynamic constants and indy). It is planned to enhance this in a 
>>>>>> further major release to support lazy resolution of bootstrap method 
>>>>>> arguments.
>>>>>>
>>>>>> The CSR for the VM specification is here:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8189199 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8189199>
>>>>>>
>>>>>> the j.l.invoke package documentation was also updated but please 
>>>>>> consider the VM specification as the definitive "source of truth" (we 
>>>>>> may clean up this area further later on so it becomes more informative, 
>>>>>> and that may also apply to duplicative text on MethodHandles/VarHandles).
>>>>>>
>>>>>> Any AoT-related work will be deferred to a future release.
>>>>>>
>>>>>> —
>>>>>>
>>>>>> This patch only supports x64 platforms. There is a small set of changes 
>>>>>> specific to x64 (specifically to support null and primitives constants, 
>>>>>> as prior to this patch null was used as a sentinel for resolution and 
>>>>>> certain primitives types would never have been encountered, such as say 
>>>>>> byte).
>>>>>>
>>>>>> We will need to follow up with the SPARC platform and it is 
>>>>>> hoped/anticipated that OpenJDK members responsible for other platforms 
>>>>>> (namely ARM and PPC) will separately provide patches.
>>>>>>
>>>>>> —
>>>>>>
>>>>>> Many of tests rely on an experimental byte code API that supports the 
>>>>>> generation of byte code with dynamic constants.
>>>>>>
>>>>>> One test uses class file bytes produced from a modified version of 
>>>>>> asmtools.  The modifications have now been pushed but a new version of 
>>>>>> asmtools need to be rolled into jtreg before the test can operate 
>>>>>> directly on asmtools information rather than embedding class file bytes 
>>>>>> directly in the test.
>>>>>>
>>>>>> —
>>>>>>
>>>>>> Paul.
>>>>>>
>>>>>
>>>>
>>>
>>
>>
> 


Reply via email to