RFR(XS) : 8155192 : [TESTBUG] SHA tests assumes that x86 does not have SHA intrinsics

Igor Ignatyev igor.ignatyev at oracle.com
Fri Jun 22 18:41:15 UTC 2018 

Hi Gustavo,

thanks for checking. I'll push webrev.01 + your diff later today.

-- Igor

> On Jun 22, 2018, at 5:42 AM, Gustavo Romero <gromero at linux.vnet.ibm.com> wrote:
> 
> Hi Igor,
> 
> On 06/21/2018 08:41 PM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8155192/webrev.01/index.html
>>> 33 lines changed: 1 ins; 10 del; 22 mod;
>> I've removed Platforms::* from UseSHAIntrinsicsSpecificTestCaseForUnsupportedCPU and UseSHASpecificTestCaseForSupportedCPU. all compiler/intrinsics/sha tests passed in our labs on sparc and x86 hosts.
>> new version of webrev -- http://cr.openjdk.java.net/~iignatyev//8155192/webrev.01/index.html
>> Gustavo, could you please test this version of path on your platforms?
> 
> Sure.
> 
> So after applying it I got the following state:
> 
> FAILED: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnUnsupportedCPU.java
> FAILED: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnUnsupportedCPU.java
> FAILED: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnUnsupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnUnsupportedCPU.java
> FAILED: compiler/intrinsics/sha/cli/TestUseSHAOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA512Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA256Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA512MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA256MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/TestSHA.java
> Test results: passed: 11; failed: 4
> 
> due to (Please see full log at http://cr.openjdk.java.net/~gromero/logs/sha_jtreg_failure.log):
> 
> java.lang.Error: Support for CPUs different fromn AARCH64, S390x, SPARC, and X86 is not implemented
>        at compiler.intrinsics.sha.cli.SHAOptionsBase.getWarningForUnsupportedCPU(SHAOptionsBase.java:92)
>        at compiler.intrinsics.sha.cli.testcases.GenericTestCaseForSupportedCPU.verifyWarnings(GenericTestCaseForSupportedCPU.java:50)
>        at compiler.intrinsics.sha.cli.SHAOptionsBase$TestCase.test(SHAOptionsBase.java:152)
>        at compiler.intrinsics.sha.cli.SHAOptionsBase.runTestCases(SHAOptionsBase.java:129)
>        at jdk.test.lib.cli.CommandLineOptionTest.test(CommandLineOptionTest.java:537)
>        at compiler.intrinsics.sha.cli.TestUseSHA512IntrinsicsOptionOnSupportedCPU.main(TestUseSHA512IntrinsicsOptionOnSupportedCPU.java:47)
>        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
>        at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:115)
>        at java.base/java.lang.Thread.run(Thread.java:832)
> 
> which seems resolved by the following tiny change:
> 
> diff -r 027c909decaf test/hotspot/jtreg/compiler/intrinsics/sha/cli/SHAOptionsBase.java
> --- a/test/hotspot/jtreg/compiler/intrinsics/sha/cli/SHAOptionsBase.java        Fri Jun 22 08:10:43 2018 -0400
> +++ b/test/hotspot/jtreg/compiler/intrinsics/sha/cli/SHAOptionsBase.java        Fri Jun 22 08:33:42 2018 -0400
> @@ -75,7 +75,7 @@
>      */
>     public static String getWarningForUnsupportedCPU(String optionName) {
>         if (Platform.isAArch64() || Platform.isS390x() || Platform.isSparc() ||
> -            Platform.isX64() || Platform.isX86()) {
> +            Platform.isX64() || Platform.isX86() || Platform.isPPC()) {
>             switch (optionName) {
>             case SHAOptionsBase.USE_SHA_OPTION:
>                 return SHAOptionsBase.SHA_INSTRUCTIONS_ARE_NOT_AVAILABLE;
> @@ -89,7 +89,7 @@
>                 throw new Error("Unexpected option " + optionName);
>             }
>         } else {
> -            throw new Error("Support for CPUs different fromn AARCH64, S390x, SPARC, and X86 "
> +            throw new Error("Support for CPUs different from AARCH64, S390x, SPARC, X86, and PPC "
>                             + "is not implemented");
>         }
>     }
> 
> Thus with your fix plus that fix above ^ the current state looks ok:
> 
> Passed: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnUnsupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnUnsupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnUnsupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnUnsupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1Intrinsics.java
> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnSupportedCPU.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA1MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA512Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA256Intrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA512MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/sanity/TestSHA256MultiBlockIntrinsics.java
> Passed: compiler/intrinsics/sha/TestSHA.java
> Test results: passed: 15
> 
> 
> Thanks,
> Gustavo
> 
>> Thanks,
>> -- Igor
>>> On Jun 21, 2018, at 2:43 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>> 
>>> Hi Gustavo,
>>> 
>>> please see my answers inline.
>>> 
>>> Thanks,
>>> -- Igor
>>> 
>>>> On Jun 21, 2018, at 1:34 PM, Gustavo Romero <gromero at linux.vnet.ibm.com <mailto:gromero at linux.vnet.ibm.com>> wrote:
>>>> 
>>>> Hi Vladimir, Igor
>>>> 
>>>> On 06/21/2018 02:47 PM, Vladimir Kozlov wrote:
>>>>> But I find it strange that PPC64 (Platform::isPPC) is only listed in one files. According to vm_version_ppc.cpp it supports SHA256 and SHA512.
>>>> 
>>>> Yes, that's correct.
>>>> 
>>>> Igor, do you mind to confirm which tests exactly must be enabled on PPC64 (or
>>>> disabled if the list is smaller...)?
>>> all the tests are already enabled on PPC64, but they don't do much testing, as they will be skipped by the tests themselves.
>>>> 
>>>> For instance, it's not clear to me yet why the suggested fix in [1] is adding PPC to
>>>> UseSHAIntrinsicsSpecificTestCaseForUnsupportedCPU.java.
>>> UseSHAIntrinsicsSpecificTestCaseForUnsupportedCPU is supposed to be run to hosts which *do* support some sha instructions, but don't support a specific one. I don't know if it's a case for PPC or not. in any case, it might be better to get rid of Platform::* predicates in this class and just use IntrinsicPredicates and SHAOptionsBase.getPredicateForOption, so its c-tor will be:
>>>>         super(optionName, new AndPredicate(
>>>>                         IntrinsicPredicates.ANY_SHA_INSTRUCTION_AVAILABLE,
>>>>                         new NotPredicate(SHAOptionsBase.getPredicateForOption(optionName))));
>>>>    < ...>
>>> 
>>> 
>>> and UseSHASpecificTestCaseForSupportedCPU's c-tor can be changed to:
>>>>         super(SHAOptionsBase.USE_SHA_OPTION, IntrinsicPredicates.ANY_SHA_INSTRUCTION_AVAILABLE);
>>> 
>>>>    < ...>
>>> 
>>> I'll retest the patch for 8155192 w/ these changes.
>>> 
>>>> 
>>>> Current status on PPC64 is:
>>>> 
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnUnsupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnSupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnUnsupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnSupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA512IntrinsicsOptionOnUnsupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA256IntrinsicsOptionOnSupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHAOptionOnSupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/cli/TestUseSHA1IntrinsicsOptionOnUnsupportedCPU.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA1Intrinsics.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA1MultiBlockIntrinsics.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA512Intrinsics.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA256Intrinsics.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA512MultiBlockIntrinsics.java
>>>> Passed: compiler/intrinsics/sha/sanity/TestSHA256MultiBlockIntrinsics.java
>>>> Passed: compiler/intrinsics/sha/TestSHA.java
>>>> Test results: passed: 15
>>> as I explained above, all the tests are run on all platforms, but some of their parts are just skipped on PPC64.
>>> 
>>>> 
>>>> Thanks.
>>>> 
>>>> Best regards,
>>>> Gustavo
>>>> 
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8205492
>>>> 
>>>>> Thanks,
>>>>> Vladimir
>>>>> On 6/20/18 8:28 PM, Igor Ignatyev wrote:
>>>>>> http://cr.openjdk.java.net/~iignatyev//8155192/webrev.00/index.html
>>>>>>> 29 lines changed: 10 ins; 3 del; 16 mod;
>>>>>> 
>>>>>> Hi all,
>>>>>> 
>>>>>> could you please review this tiny fix for SHA tests? the fix adjusts definitions of supported/unsupported CPUs in sha tests.
>>>>>> 
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8155192
>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev//8155192/webrev.00/index.htm
>>>>>> testing: test/hotspot/jtreg/compiler/intrinsics tests
>>>>>> 
>>>>>> Thanks,
>>>>>> -- Igor
>

More information about the hotspot-compiler-dev mailing list