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