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.
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>
