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

Sun Jun 25 03:15:19 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?

Yes, that may need more overhead on AArch64 code. I think the changing should not only limit to `UseSVE` flag and the `sve` cpu feature, but also to all other flags. We have to keep the design consistent. 

And yes, for the current backend, we're almost using `UseSVE` flag instead of the cpu feature check.

> 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'm afraid we have to modify all these tests, and what I know is my colleague @pfustc will take charge of the vectorization tests  once this PR is merged. Besides, we have to only care about the rules that `sve` is `true` like `"@IR(applyIfCPUFeatureOr = {"sve", "true", ...}`. If the `sve` feature is `false`, `UseSVE` is always `0` in the VM, which is synced.

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

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