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