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   }

-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