RFR (S): 8209972: [GRAAL] Don't run RTM tests with Graal
Gustavo Romero
gromero at linux.vnet.ibm.com
Mon Sep 3 22:15:23 UTC 2018
Hi Vladimir,
Thanks a lot for reviewing it and for your comments.
On 08/31/2018 03:12 PM, Vladimir Kozlov wrote:
> Hi Gustavo,
>
> I think you should replace !Compiler.isGraalEnabled() with Compiler.isC2Enabled() because C2 may be switched off with TieredStopAtLevel < 4 flag
Yes, although currently afaics all tests will explicitly enabled C2 (for
instance, passing -Xcomp and -XX:-TieredCompilation) or implicitly enable C2
through a warming up before testing, I agree that nothing forbids one to
switch off C2 with TieredStopAtLevel = level < 4 for a test case. It also
looks better to list explicitly which compilers do support RTM instead of
the ones that don't support it.
I've updated the webrev accordingly:
http://cr.openjdk.java.net/~gromero/8209972/v2/
diff in there looks odd so I generated another one with --patience for a
better (IMO) diff format:
http://cr.openjdk.java.net/~gromero/8209972/v2/8209972_GRAAL_dont_run_RTM_tests_with_Graal_v2.diff
> Also can platforms check be replaced with one vmRTMCPU()? Is ppc64 return true from vmRTMCPU()?
For example, on Linux the following cases are possible regarding CPU / OS
RTM support:
POWER7 : cpu = false, os = false => vm.rtm.cpu = false
POWER8 : cpu = true, os = false | true => vm.rtm.cpu = false | true
POWER9 VM: cpu = true, os = false | true => vm.rtm.cpu = false | true
POWER9 NV: cpu = true, os = false => vm.rtm.cpu = false
PPC64 will return 'true' for vmRTMCPU() _only_ if both CPU and OS support
RTM. In other words, when @requires asks for "vm.rtm.cpu & vm.rtm.os" it
really looks like a tautology because if "vm.rtm.cpu" is 'true' it implies
"vm.rtm.os" being 'true' as well, otherwise the JVM would never advertise
the "rtm" feature (which is used by "vm.rtm.cpu"). That seems true for
Linux and for AIX.
That said I don't think that the platforms check can be replaced with one
vmRTMCPU(), because in some cases it's necessary to run a test for
cpu = false and compiler = true, i.e. it's necessary to run a test on an
unsupported CPU for a given platform _only if_ the compiler in use supports
RTM (like C2). So if, for instance, we do:
'vm.rtm.compiler = C2 && vmRTMCPU()' and use 'vm.rtm.compiler' in @requires
we "tie" CPU+OS RTM support to compiler RTM support and the evaluation
returns 'false' for cpu = false and compiler = true, skipping the test
(vm.rtm.compiler = false). On the other hand, to evaluate 'vm.rtm.compiler'
as 'true' and run the test in that case one could match for
'!vm.rtm.compiler', but if compiler = false (Graal) '!vm.rtm.compiler' will
be evaluated as 'true' and the test will run even thought the Graal
compiler is selected, which is wrong.
Hence 'vm.rtm.compiler' property must not rely on vmRTMCPU() and must
contain its own list of supported compilers with RTM support for each
platform IMO. Basically we can't ask the JVM about the compiler's support
for RTM, since the JVM can only tell us about the CPU+OS support for RTM
regarding the CPU and OS in which the JVM is running on.
> And since you check cpu in here why not to replace all vm.rtm.* with one vm.rtm.supported? In such case you would need only one @requires checks in tests instead of:
>
> vm.flavor == "server" & !vm.emulatedClient & vm.rtm.cpu & vm.rtm.os & vm.rtm.compiler
I think it's not possible either. Currently there are 5 match cases in
RTM tests:
gromero at moog:~/hg/jdk/jdk/test/hotspot/jtreg/compiler/rtm$ fgrep @require -RIn | cut -d ' ' -f 2-40 | sort -u
* @requires !(vm.flavor == "server" & !vm.emulatedClient & vm.rtm.cpu & vm.rtm.os)
* @requires vm.flavor == "server" & !vm.emulatedClient & vm.rtm.cpu & vm.rtm.os
* @requires (!vm.rtm.cpu) & (vm.flavor == "server" & !vm.emulatedClient)
* @requires vm.rtm.cpu & !(vm.flavor == "server" & !vm.emulatedClient)
which can be simplified 5 cases as:
1: !(flavor == "server" & !emulatedClient & cpu & os)
2: flavor == "server" & !emulatedClient & cpu & os
3: (!cpu) & (flavor == "server" & !emulatedClient)
4: cpu & !(flavor == "server" & !emulatedClient)
5: no @requires
I understand that case 1 and 2 (since CPU implies OS) can be simplified as:
1: !(flavor == "server" & !emulatedClient & cpu)
2: flavor == "server" & !emulatedClient & cpu
3: (!cpu) & (flavor == "server" & !emulatedClient)
4: cpu & !(flavor == "server" & !emulatedClient)
5: no @requires
and case 1 and 2 are mere opposites, so we have 4 cases:
1: !(flavor == "server" & !emulatedClient & cpu)
3: (!cpu) & (flavor == "server" & !emulatedClient)
4: cpu & !(flavor == "server" & !emulatedClient)
5: no @requires
We could simplify further making P = (flavor == "server" & !emulatedClient),
and make:
1: !(P & cpu)
3: (!cpu) & (P)
4: cpu & !(P)
5: no @requires
So if we add a compiler = C2 && (x64 | PPC) property to each of them in
order to control running the tests only if the selected compiler on a
given platform has RTM support (skipping Graal, for instance):
1: !(P & cpu) & compiler
3: (!cpu) & (P) & compiler
4: cpu & !(P) & compiler
5: no @requires & compiler
So it looks like that at minimum we would need 3 properties, but IMO it's
not worth to add another property P = (flavor == "server" & !emulatedClient)
just to simplify further the @requires line.
In summing up, I think it's only possible to replace 'cpu & os' by 'cpu',
so I updated the webrev removing the vm.rtm.os property and keeping only
vm.rtm.cpu and vm.rtm.compiler (plus flavor and emulatedClient checks).
I've tested the following scenarios and observed no regression [1]:
1. X86_64 w/ RTM
2. X86_64 w/ RTM + Graal enabled
3. POWER7: no CPU+OS support for RTM
4. POWER8: CPU+OS support for RTM
But I think we need a confirmation from SAP about AIX.
Best regards,
Gustavo
[1]
** X86_64 w/ RTM **
Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestRTMTotalCountIncrRateOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMLockingOptionWithBiasedLocking.java
Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
Passed: compiler/rtm/locking/TestRTMAbortThreshold.java
Passed: compiler/rtm/locking/TestRTMAbortRatio.java
Passed: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
Passed: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
Passed: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
Passed: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
Passed: compiler/rtm/locking/TestRTMLockingThreshold.java
Passed: compiler/rtm/locking/TestRTMRetryCount.java
Passed: compiler/rtm/locking/TestRTMSpinLoopCount.java
Passed: compiler/rtm/locking/TestUseRTMAfterLockInflation.java
Passed: compiler/rtm/locking/TestRTMTotalCountIncrRate.java
Passed: compiler/rtm/locking/TestUseRTMForInflatedLocks.java
Passed: compiler/rtm/locking/TestUseRTMDeopt.java
Passed: compiler/rtm/locking/TestUseRTMForStackLocks.java
Passed: compiler/rtm/method_options/TestNoRTMLockElidingOption.java
Passed: compiler/rtm/method_options/TestUseRTMLockElidingOption.java
Passed: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
Passed: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
Test results: passed: 30
** X86_64 w/ RTM + Graal enabled **
Test results: no tests selected (all RTM tests skipped)
** POWER7: no CPU+OS support for RTM **
Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnUnsupportedConfig.java
Passed: compiler/rtm/cli/TestRTMAbortThresholdOption.java
Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnUnsupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnUnsupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnUnsupportedCPU.java
Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
Test results: passed: 10
** POWER8: CPU+OS support for RTM **
Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestRTMAbortThresholdOption.java
Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
Passed: compiler/rtm/cli/TestRTMTotalCountIncrRateOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnSupportedConfig.java
Passed: compiler/rtm/cli/TestUseRTMLockingOptionWithBiasedLocking.java
Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
Passed: compiler/rtm/locking/TestRTMAbortRatio.java
Passed: compiler/rtm/locking/TestRTMAbortThreshold.java
Passed: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
Passed: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
Passed: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
Passed: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
Passed: compiler/rtm/locking/TestRTMLockingThreshold.java
Passed: compiler/rtm/locking/TestRTMRetryCount.java
Passed: compiler/rtm/locking/TestRTMSpinLoopCount.java
Passed: compiler/rtm/locking/TestRTMTotalCountIncrRate.java
Passed: compiler/rtm/locking/TestUseRTMAfterLockInflation.java
Passed: compiler/rtm/locking/TestUseRTMDeopt.java
Passed: compiler/rtm/locking/TestUseRTMForInflatedLocks.java
Passed: compiler/rtm/locking/TestUseRTMForStackLocks.java
Passed: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
Passed: compiler/rtm/method_options/TestNoRTMLockElidingOption.java
Passed: compiler/rtm/method_options/TestUseRTMLockElidingOption.java
Passed: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
Test results: passed: 30
> Thanks,
> Vladimir
>
> On 8/31/18 8:38 AM, Gustavo Romero wrote:
>> Hi,
>>
>> Could the following small change be reviewed please?
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8209972
>> Webrev: http://cr.openjdk.java.net/~gromero/8209972/v1/
>>
>> It disables all RTM tests when a compiler not supporting RTM (e.g. Graal)
>> is selected on platforms that can have CPU/OS with RTM support.
>>
>> It also disables all RTM tests for any other platform that has not a single
>> compiler supporting RTM.
>>
>> The RTM support was first added to C2 compiler and once checkers for RTM
>> (notably vm.rtm.cpu) find the feature "rtm" advertised by the JVM they
>> assume that a compiler supporting RTM is available for sure ("rtm" is
>> advertised only if RTM is supported by both CPU and OS). Later the JVM
>> began to allow the selection of a compiler different from C2, like Graal,
>> and it became possible to select a compiler without RTM support despite the
>> fact that both the CPU and the OS support RTM. Thus for platforms
>> supporting Graal or any other specific compiler the compiler availability for
>> the RTM tests must be adjusted and if the selected compiler does not
>> support RTM then all RTM tests must be skipped, including the ones meant
>> for platforms without CPU or OS RTM support (e.g. *Unsupported*.java)
>> because in some cases, like in TestUseRTMLockingOptionOnUnsupportedCPU.java,
>> the test expects JVM initialization errors that will never occur because the
>> problem is not that the RTM support for CPU or OS is missing, but rather
>> because the selected compiler does not support RTM.
>>
>> That change adds a new VM property 'vm.rtm.compiler' which can be used to
>> filter out compilers without RTM support for specific platforms and adapts
>> the current RTM tests to use that new property.
>>
>> Nothing changes regarding the number of passing/selected tests for the
>> various cpu/os/compiler combinations on platforms that currently might
>> support RTM [1], except when Graal is in use.
>>
>> Thank you.
>>
>> Best regards,
>> Gustavo
>>
>>
>> [1]
>>
>> ** X64 w/ CPU and OS supporting RTM **
>> Passed: compiler/rtm/cli/TestRTMAbortThresholdOption.java
>> Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
>> Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
>> Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
>> Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
>> Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestRTMTotalCountIncrRateOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMLockingOptionWithBiasedLocking.java
>> Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
>> Passed: compiler/rtm/locking/TestRTMAbortThreshold.java
>> Passed: compiler/rtm/locking/TestRTMAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
>> Passed: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
>> Passed: compiler/rtm/locking/TestRTMLockingThreshold.java
>> Passed: compiler/rtm/locking/TestRTMRetryCount.java
>> Passed: compiler/rtm/locking/TestRTMSpinLoopCount.java
>> Passed: compiler/rtm/locking/TestRTMTotalCountIncrRate.java
>> Passed: compiler/rtm/locking/TestUseRTMAfterLockInflation.java
>> Passed: compiler/rtm/locking/TestUseRTMForInflatedLocks.java
>> Passed: compiler/rtm/locking/TestUseRTMDeopt.java
>> Passed: compiler/rtm/locking/TestUseRTMForStackLocks.java
>> Passed: compiler/rtm/method_options/TestNoRTMLockElidingOption.java
>> Passed: compiler/rtm/method_options/TestUseRTMLockElidingOption.java
>> Passed: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
>> Passed: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
>> Test results: passed: 30
>>
>> ** X64 w/ CPU and OS supporting RTM + Graal compiler wo/ RTM support **
>> Test results: no tests selected (all RTM tests skipped)
>>
>> ** POWER8 w/ CPU and OS supporting RTM **
>> Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestRTMAbortThresholdOption.java
>> Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
>> Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
>> Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
>> Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
>> Passed: compiler/rtm/cli/TestRTMTotalCountIncrRateOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnSupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMLockingOptionWithBiasedLocking.java
>> Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
>> Passed: compiler/rtm/locking/TestRTMAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMAbortThreshold.java
>> Passed: compiler/rtm/locking/TestRTMAfterNonRTMDeopt.java
>> Passed: compiler/rtm/locking/TestRTMDeoptOnHighAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMDeoptOnLowAbortRatio.java
>> Passed: compiler/rtm/locking/TestRTMLockingCalculationDelay.java
>> Passed: compiler/rtm/locking/TestRTMLockingThreshold.java
>> Passed: compiler/rtm/locking/TestRTMRetryCount.java
>> Passed: compiler/rtm/locking/TestRTMSpinLoopCount.java
>> Passed: compiler/rtm/locking/TestRTMTotalCountIncrRate.java
>> Passed: compiler/rtm/locking/TestUseRTMAfterLockInflation.java
>> Passed: compiler/rtm/locking/TestUseRTMDeopt.java
>> Passed: compiler/rtm/locking/TestUseRTMForInflatedLocks.java
>> Passed: compiler/rtm/locking/TestUseRTMForStackLocks.java
>> Passed: compiler/rtm/locking/TestUseRTMXendForLockBusy.java
>> Passed: compiler/rtm/method_options/TestNoRTMLockElidingOption.java
>> Passed: compiler/rtm/method_options/TestUseRTMLockElidingOption.java
>> Passed: compiler/rtm/print/TestPrintPreciseRTMLockingStatistics.java
>> Test results: passed: 30
>>
>> ** POWER7 wo/ CPU and OS supporting RTM **
>> Passed: compiler/rtm/cli/TestPrintPreciseRTMLockingStatisticsOptionOnUnsupportedConfig.java
>> Passed: compiler/rtm/cli/TestRTMAbortThresholdOption.java
>> Passed: compiler/rtm/cli/TestRTMLockingCalculationDelayOption.java
>> Passed: compiler/rtm/cli/TestRTMLockingThresholdOption.java
>> Passed: compiler/rtm/cli/TestRTMRetryCountOption.java
>> Passed: compiler/rtm/cli/TestRTMSpinLoopCountOption.java
>> Passed: compiler/rtm/cli/TestUseRTMDeoptOptionOnUnsupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMForStackLocksOptionOnUnsupportedConfig.java
>> Passed: compiler/rtm/cli/TestUseRTMLockingOptionOnUnsupportedCPU.java
>> Passed: compiler/rtm/cli/TestUseRTMXendForLockBusyOption.java
>> Test results: passed: 10
>>
>
More information about the hotspot-compiler-dev
mailing list