I’m seeing no issue with rcx being aliased in this code. Fred
On Oct 30, 2017, at 15:44, Paul Sandoz <paul.sandoz@oracle.com> wrote:
Hi,
Thanks for reviewing.
On 30 Oct 2017, at 11:05, Dmitry Samersoff <dmitry.samersoff@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-... <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.