RFR(s): 8205581: PPC64: RTM: Fix abort on native calls

[Lindenmaier, Goetz]

Hi Gustavo, 

change looks good, Reviewed.

Best regards,
  Goetz.

> -----Original Message-----
> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> Sent: Dienstag, 17. Juli 2018 15:44
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Subject: Re: RFR(s): 8205581: PPC64: RTM: Fix abort on native calls
> 
> Hi,
> 
> Could I get a second review for that change please?
> 
> Best regards,
> Gustavo
> 
> On 06/26/2018 09:54 AM, Doerr, Martin wrote:
> > Hi Gustavo,
> >
> > thanks for the update. Looks good to me.
> >
> > Best regards,
> > Martin
> >
> >
> > -----Original Message-----
> > From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> > Sent: Dienstag, 26. Juni 2018 14:41
> > To: Doerr, Martin <martin.doerr at sap.com>; Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com>; hotspot-compiler-dev at openjdk.java.net
> > Cc: ppc-aix-port-dev at openjdk.java.net
> > Subject: Re: RFR(s): 8205581: PPC64: RTM: Fix abort on native calls
> >
> > Hi Martin,
> >
> > Thanks for the quick review!
> >
> > On 06/25/2018 01:37 PM, Doerr, Martin wrote:
> >> I wonder why you placed the tabort so late in generate_native_wrapper.
> I'd put it at the Verified Entry Point.
> >
> > Actually for no particular reason.
> >
> > So previously:
> >
> > 2247 [Verified Entry Point]
> > 2248   0x00007fff9c816520: mfcr    r22
> > 2249   0x00007fff9c816524: std     r22,8(r1)
> > 2250   0x00007fff9c816528: mflr    r22
> > 2251   0x00007fff9c81652c: std     r22,16(r1)
> > 2252   0x00007fff9c816530: addis   r11,r1,-2
> > 2253   0x00007fff9c816534: std     r0,0(r11)
> > 2254   0x00007fff9c816538: mr      r21,r1
> > 2255   0x00007fff9c81653c: stdu    r1,-176(r1)
> > 2256   0x00007fff9c816540: std     r3,96(r1)
> > 2257   0x00007fff9c816544: addi    r4,r1,96
> > 2258   0x00007fff9c816548: cmpdi   r3,0
> > 2259   0x00007fff9c81654c: bne-    0x00007fff9c816554
> > 2260   0x00007fff9c816550: li      r4,0
> > 2261   0x00007fff9c816554: addi    r3,r16,824        ;
> ImmutableOopMap{[96]=Oop }
> > 2262   0x00007fff9c816558: addis   r28,r29,7
> > 2263   0x00007fff9c81655c: addi    r28,r28,25944     ;   {internal_word}
> > 2264   0x00007fff9c816560: tabort. r0 <==
> > ...
> >
> > Now:
> >
> > 2169 [Verified Entry Point]
> > 2170   0x00007fff78816320: tabort. r0 <==
> > 2171   0x00007fff78816324: mfcr    r22
> > 2172   0x00007fff78816328: std     r22,8(r1)
> > 2173   0x00007fff7881632c: mflr    r22
> > 2174   0x00007fff78816330: std     r22,16(r1)
> > 2175   0x00007fff78816334: addis   r11,r1,-2
> > 2176   0x00007fff78816338: std     r0,0(r11)
> > 2177   0x00007fff7881633c: mr      r21,r1
> > 2178   0x00007fff78816340: stdu    r1,-176(r1)
> > 2179   0x00007fff78816344: std     r3,96(r1)
> > 2180   0x00007fff78816348: addi    r4,r1,96
> > 2181   0x00007fff7881634c: cmpdi   r3,0
> > 2182   0x00007fff78816350: bne-    0x00007fff78816358
> > 2183   0x00007fff78816354: li      r4,0
> > 2184   0x00007fff78816358: addi    r3,r16,824        ;
> ImmutableOopMap{[96]=Oop }
> > 2185   0x00007fff7881635c: addis   r28,r29,7
> > 2186   0x00007fff78816360: addi    r28,r28,25436     ;   {internal_word}
> > ...
> >
> > Yep, it's better to abort sooner.
> >
> > new webrev: http://cr.openjdk.java.net/~gromero/8205581/v2/
> >
> >
> > Best regards,
> > Gustavo
> >
> >> Besides that, it looks good to me.
> >>
> >> Thanks,
> >> Martin
> >>
> >>
> >> -----Original Message-----
> >> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> >> Sent: Montag, 25. Juni 2018 10:21
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Doerr, Martin
> <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net
> >> Cc: ppc-aix-port-dev at openjdk.java.net
> >> Subject: RFR(s): 8205581: PPC64: RTM: Fix abort on native calls
> >>
> >> Hi,
> >>
> >> Could the following change be reviewed please?
> >>
> >> bug   : https://bugs.openjdk.java.net/browse/JDK-8205581
> >> webrev: http://cr.openjdk.java.net/~gromero/8205581/v1/
> >>
> >> It forces a transactional state to abort before calling native methods,
> before
> >> calling runtime, and on uncommon trap checking, mostly because
> transaction will
> >> be aborted soon or latter in either case, similarly to what happens on
> Intel.
> >> The abort instruction (tabort) is only emitted if UseRTMLocking is "true"
> and
> >> any 'tabort' instruction is treated as a 'nop' instruction if TM state is
> >> non-transactional.
> >>
> >> It fixes the following tests:
> >>
> >> +Passed: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
> >> +Passed: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
> >>
> >>
> >> Thank you and best regards,
> >> Gustavo
> >>
> >