RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"

Mon May 28 14:31:43 UTC 2018

Hi Martin,

On 05/28/2018 06:43 AM, Doerr, Martin wrote:
> Hi Gustavo,
> 
> thanks for fixing AIX. Looks correct.
> 
> I think the tests don't fail because of the warning. I was just wondering why we're spending time for something we don't want to support. I think you can remove the warning with this change if you like.

Got it. The warning message is removed and now the JVM behaves as
on x86 regarding UseRTMLocking:

http://cr.openjdk.java.net/~gromero/POWER9/8203669/v3/

Best regards,
Gustavo

> Maybe the tests don't fit for PPC64. We have too many other things to do in jdk11 time frame. Maybe it'd be better to switch them off for PPC64.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: Gustavo Romero [mailto:gromero at linux.vnet.ibm.com]
> Sent: Freitag, 25. Mai 2018 22:44
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Subject: Re: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
> 
> Hi Martin,
> 
> On 05/25/2018 10:29 AM, Doerr, Martin wrote:
>> Hi Gustavo,
>>
>> after talking with Götz, we found a couple of problems.
>>
>> vm_version_ppc.cpp:
>> AIX 7.2 on Power7 would report "rtm" in the feature string which is not true.
>> The check for Power8 is missing here. However, PowerArchitecturePPC64 can't be used here because it's not initialized at this point. "has_lqarx()" could be used.
> 
> I see. In the end "rtm" for AIX is being reported if AIX version matches and is
> not taking the CPU version into account. On Linux that seems not an issue
> because if we find any HTM support on AUXV it's pretty sure that the CPU
> supports it so reporting "rtm" after only checking AUXV is correct. So I kept
> code unchanged for Linux. As has_lqarx() checks '_features' and not 'features' I
> decided for moving the code below '_features = features' attribution.
> 
> Would that version work on AIX?
> 
> http://cr.openjdk.java.net/~gromero/POWER9/8203669/v2/
> 
> 
>> Some tests complain:
>> OpenJDK 64-Bit Server VM warning: UseRTMLocking is only available as experimental option on this platform.
> 
> hmm right. Do you mean on Linux as well? That change was supposed to not cause
> that kind of test complain... Can you share some logs on that?
> 
> On POWER8 LE (just like before 8203305 when in) I see only 10 test failing:
> 
> FAILED: compiler/rtm/locking/TestRTMAbortThreshold.java
> FAILED: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
> FAILED: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
> FAILED: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
> FAILED: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
> FAILED: compiler/rtm/locking/TestRTMLockingThreshold.java
> FAILED: compiler/rtm/locking/TestRTMRetryCount.java
> FAILED: compiler/rtm/locking/TestRTMSpinLoopCount.java
> FAILED: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
> FAILED: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
> 
> and none is because of RTM being an experimental feature. Here are the reasons
> (not in the same order of the above list):
> 
> gromero at gromero16:~/hg/jdk11/jdk$ egrep -E "TEST.*failed:" rtm.log
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Expected that method with rtm lock elision was deoptimized after 1 lock attempts: expected 10000 to equal 1
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Total aborts count (2001) should be less or equal to 1001: expected that 2001 <= 1001
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: VM output should contain two RTM locking statistics entries.: expected 1 to equal 2
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Expected to get only one deoptimization due to rtm state change: expected 0 to equal 1
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Expected to get only one deoptimization due to rtm state change: expected 0 to equal 1
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: At least one deoptimization due to rtm_state_chage is expected: expected 0 > 0
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: RTM locking statistics should contain non zero total aborts count: expected 0 > 0
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: Two uncommon traps with reason rtm_state_change should be fired.: expected 0 to equal 2
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: It is expected to get 2 aborts: expected 3 to equal 2
> TEST RESULT: Failed. Execution failed: `main' threw exception: java.lang.RuntimeException: VM output should contain exactly one rtm locking statistics entry for method compiler.testlibrary.rtm.BusyLock::test: expected 0 to equal 1
> 
> 
>> I think the warnings or vm_exit stuff could be removed if you want to support it officially. X86 only uses it for certain older processors.
> 
> Got it. I see the same failing RTM tests on my x86 machine (Skylake) with RTM
> support. I intended to set it as non-experimental once I was able to get all
> tests passing on PPC64. But ok, as you noted maybe on x86 it's just there on
> because of the initial broken support for RTM on old processors.
> 
> Would you like to address it in that bug or should I open another bug?
> 
> 
>> We'll have to check other tests, too. We'll do that next week.
> 
> Thank you so much. I wished I had a change to read/reply that email earlier but
> the whole email system was down until now.
> 
> 
> Best regards,
> Gustavo
> 
>> Best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: Doerr, Martin
>> Sent: Donnerstag, 24. Mai 2018 11:19
>> To: 'Gustavo Romero' <gromero at linux.vnet.ibm.com>; hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
>> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>> Subject: RE: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
>>
>> Hi Gustavo,
>>
>> the fix looks good. I especially like that you use "rtm" like x86, now.
>> I can sponsor it after we got a 2nd review.
>>
>> Best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Gustavo Romero
>> Sent: Mittwoch, 23. Mai 2018 15:49
>> To: hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
>> Subject: RFR(S): 8203669: PPC64: Fix jtreg RTM tests after "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
>>
>> Hi,
>>
>> Could the following small fix be reviewed please?
>>
>> It touches shared code.
>>
>> bug:    https://bugs.openjdk.java.net/browse/JDK-8203669
>> webrev: http://cr.openjdk.java.net/~gromero/POWER9/8203669
>>
>> Since change "8203305: Improve TM detection for enabling RTM on Linux / POWER9"
>> JVM on PPC64 is not advertising anymore the feature "tcheck" which indicated
>> that RTM was supported by the JVM in case both CPU and OS supported hardware
>> transactional memory.
>>
>> It happens that several tests under /test/hotspot/jtreg/compiler/rtm rely on the
>> JVM to advertise if RTM is supported by the JVM on a given CPU/OS and so they
>> were failing to be skipped (or not) on different PPC64/Linux combinations.
>>
>> That change fixes it by re-introducing a feature to inform that RTM is supported
>> by the JVM when appropriated (given valid OS/CPU support) and now it does it by
>> advertising that feature as on x86_64, reporting the feature as "rtm".
>>
>> It also incorporates a check for when OS does not support RTM but CPU does in
>> test:
>>
>> TestUseRTMLockingOptionOnUnsupportedCPU.java.
>>
>> Just on PPC64 it might be the case that although CPU supports RTM (HTM)
>> instructions the OS version might not support it, like on old kernels that do
>> not abort transactions on syscalls. JVM correctly reports each case so the test
>> was adapted to test them.
>>
>> That change also corrects a couple of nit typos elsewhere not related to
>> "8203305".
>>
>>
>> Thank you.
>>
>> Best regards,
>> Gustavo
>>
> 