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