Hi Martin,
the change looks good.
I would appreciate a test with -XX:-UseSIGTRAP.
I guess you can run any test with that flag, maybe with a change like the one below.
Maybe you want to do a follow up with the test change.
Best regards,
Goetz.
diff -r 0358dad944c7 test/hotspot/jtreg/runtime/handshake/HandshakeWalkStackTest.java
--- a/test/hotspot/jtreg/runtime/handshake/HandshakeWalkStackTest.java Wed Jun 27 12:56:21 2018 +0530
+++ b/test/hotspot/jtreg/runtime/handshake/HandshakeWalkStackTest.java Wed Jun 27 11:17:54 2018 +0200
@@ -22,7 +22,7 @@
*
*/
-/*
+/**
* @test HandshakeWalkStackTest
* @library /testlibrary /test/lib
* @build HandshakeWalkStackTest
@@ -31,6 +31,16 @@
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI HandshakeWalkStackTest
*/
+/**
+ * @test HandshakeWalkStackTest
+ * @library /testlibrary /test/lib
+ * @requires vm.simpleArch == "ppc64" | vm.simpleArch == "ppc64le"
+ * @build HandshakeWalkStackTest
+ * @run driver ClassFileInstaller sun.hotspot.WhiteBox
+ * sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -XX:-UseSIGTRAP -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI HandshakeWalkStackTest
+ */
+
import jdk.test.lib.Asserts;
import sun.hotspot.WhiteBox;
> -----Original Message-----
> From: Doerr, Martin
> Sent: Dienstag, 26. Juni 2018 13:02
> To: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Subject: RE: RFR(S): 8205609: [PPC64] Fix PPC64 part of 8010319 and TLH
> without UseSIGTRAP on AIX
>
> Hi David,
>
> indeed, this is very confusing. The constant pool cache contents were
> changed for "Better interface invocations" so the register names don't fit to
> the usage in invokeinterface anymore (on all platforms).
>
> You're right, Rmethod should be used. I had copied the wrong name.
>
> I just noticed that the required tests were not run before I had created the
> webrev. I'll run them with the new webrev.
>
> New webrev using Rmethod + improved comments:
> http://cr.openjdk.java.net/~mdoerr/8205609_ppc64_TI_and_TLH_fixes/we
> brev.01/
>
> Thank you and best regards,
> Martin
>
>
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Dienstag, 26. Juni 2018 04:11
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-
> dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Subject: Re: RFR(S): 8205609: [PPC64] Fix PPC64 part of 8010319 and TLH
> without UseSIGTRAP on AIX
>
> Hi Martin,
>
> On 26/06/2018 12:34 AM, Doerr, Martin wrote:
> > Hi,
> >
> > I'd like to fix issues with the current PPC64 implementation:
> >
> > * Wrong register usage in template interpreter after JDK-8010319.
>
> Well that's disappointing - I thought it had all been pre-tested.
>
> This code is quite confusing. We have:
>
> 3584 prepare_invoke(byte_no, Rinterface_klass, Rret_addr, Rmethod,
> Rreceiver, Rflags, Rscratch1);
>
> but the definition of prepare_invoke is:
>
> 3332 void TemplateTable::prepare_invoke(int byte_no,
> 3333 Register Rmethod, // linked method (or i-klass)
> 3334 Register Rret_addr,// return address
> 3335 Register Rindex, // itable index, MethodType, etc.
> 3336 Register Rrecv, // If caller wants to see it.
> 3337 Register Rflags, // If caller wants to test it.
> 3338 Register Rscratch
> 3339 ) {
>
> So this passes Rinterface_klass where Rmethod is expected; and Rmethod
> where Rindex is expected. So in that sense I think the register content
> will be correct but the name seems completely wrong! ???
>
> Cheers,
> David
>
> > * SafepointMechanism implementation for AIX misses the protection of
> the bad page (ThreadLocalHandshakes with -XX:-UseSIGTRAP). I prefer to
> implement it using 2 pages like on the other platforms.
> > * Remove unused function which is obsolete since the introduction of
> ThreadLocalHanshakes.
> >
> > Please review:
> >
> http://cr.openjdk.java.net/~mdoerr/8205609_ppc64_TI_and_TLH_fixes/we
> brev.00/
> >
> > Thanks and best regards,
> > Martin
> >