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