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@linux.vnet.ibm.com] Sent: Dienstag, 26. Juni 2018 14:41 To: Doerr, Martin <martin.doerr@sap.com>; Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; hotspot-compiler-dev@openjdk.java.net Cc: ppc-aix-port-dev@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@linux.vnet.ibm.com] Sent: Montag, 25. Juni 2018 10:21 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; Doerr, Martin <martin.doerr@sap.com>; hotspot-compiler-dev@openjdk.java.net Cc: ppc-aix-port-dev@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