RFR: 8309894: compiler/vectorapi/VectorLogicalOpIdentityTest.java fails on SVE system with UseSVE=0

Wed Jun 21 10:26:04 UTC 2023

On Wed, 21 Jun 2023 09:45:40 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> @XiaohongGong Thanks for looking into this. But it seems to me this is not the same approach as we are taking with x86 SSE and AVX, where the `UseAVX` and `UseSSE` flags affect both the VM features and also the `applyIfCPUFeature` from the IR framework. We check in all sorts of places for `sve` feature, so why do you now only change it for this particular test?
>> 
>> https://github.com/openjdk/jdk/blob/8d899925dc281c5dabbef14d85a6df807f8d300e/src/hotspot/cpu/x86/vm_version_x86.cpp#L954-L955
>> 
>> Can you do a similar thing in `src/hotspot/cpu/aarch64/vm_version_aarch64.cpp` ?
>> 
>> It would be nice not to have to check for the flag and the features in every test, but just for the features. And the features should depend on what is present on the hardware, minus the restrictions by the flags.
>
>> @XiaohongGong Thanks for looking into this. But it seems to me this is not the same approach as we are taking with x86 SSE and AVX, where the `UseAVX` and `UseSSE` flags affect both the VM features and also the `applyIfCPUFeature` from the IR framework. We check in all sorts of places for `sve` feature, so why do you now only change it for this particular test?
>> 
>> https://github.com/openjdk/jdk/blob/8d899925dc281c5dabbef14d85a6df807f8d300e/src/hotspot/cpu/x86/vm_version_x86.cpp#L954-L955
>> 
>> Can you do a similar thing in `src/hotspot/cpu/aarch64/vm_version_aarch64.cpp` ?
>> 
>> It would be nice not to have to check for the flag and the features in every test, but just for the features. And the features should depend on what is present on the hardware, minus the restrictions by the flags.
> 
> Thanks for looking at this PR @eme64 ! Yes, that's the main difference between aarch64 and x86 platforms.  It actually makes things simpler that changing the CPU features based on the vm option.  But per my understanding, CPU features are the hardware's feature which is the objective fact, while the `UseSVE` are the JVM's option that people can set different values. And they cannot be mixed. Besides, x86 just mask off the CPU features for JVM instead of really changing the hardware's features. I'm not sure, but I'm afraid doing such changes like x86 may have some risks in current aarch64's backend. 
> 
>> We check in all sorts of places for `sve` feature, so why do you now only change it for this particular test?
> 
> For each SVE test, we have tried to add flag `UseSVE=1` in the test's `main` function to make sure this option is not changed by others, and current test is run with the  expected sve feature. For example:
> 
>     public static void main(String[] args) {
>         TestFramework.runWithFlags("--add-modules=jdk.incubator.vector",
>                                    "-XX:UseSVE=1");
>     }
> 
> For this test, we cannot add such an option in the test file, since it is also used to test other platforms like x86.

@XiaohongGong I see, you are worried that it would take a lot of work in the `aarch64` code? So in the backend you are using the `UseSVE` flag instead of feature support?

Are you going to add the `UseSVE` flag to all these cases too?

emanuel at emanuel-oracle:/oracle-work/jdk-fork8/open$ grep sve test/hotspot/jtreg/compiler/ -r
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(failOn = IRNode.AND_VB, applyIfCPUFeatureAnd = {"asimd", "true", "sve", "false"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(failOn = IRNode.AND_VS, applyIfCPUFeatureAnd = {"asimd", "true", "sve", "false"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.AND_VI, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.AND_VL, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.AND_VI, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(failOn = IRNode.OR_VB, applyIfCPUFeatureAnd = {"asimd", "true", "sve", "false"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(failOn = IRNode.OR_VB, applyIfCPUFeatureAnd = {"asimd", "true", "sve", "false"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.OR_VI, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.OR_VL, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(counts = {IRNode.OR_VI, "1"}, applyIfCPUFeatureOr = {"sve", "true", "avx512", "true"})
test/hotspot/jtreg/compiler/vectorapi/VectorLogicalOpIdentityTest.java:    @IR(failOn = IRNode.XOR_VS, applyIfCPUFeatureAnd = {"asimd", "true", "sve", "false"})
test/hotspot/jtreg/compiler/vectorization/runner/LoopArrayIndexComputeTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/LoopArrayIndexComputeTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/LoopArrayIndexComputeTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/LoopArrayIndexComputeTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx512dq", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/BasicLongOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx512dq", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/BasicLongOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "sse4.1", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/BasicLongOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "sse4.1", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/BasicLongOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "sse4.1", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/ArrayIndexFillTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/LoopReductionOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},
test/hotspot/jtreg/compiler/vectorization/runner/LoopReductionOpTest.java:    @IR(applyIfCPUFeatureOr = {"sve", "true", "avx2", "true"},

I just really don't like the duplication of IR rules, and the checking of both the flag and the cpu feature.

Another solution: we filter out the `sve` feature at the IR framework, if we have `UseSVE=0`?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14533#issuecomment-1600580843