RFR: 8317121: vector_masked_load instruction is moved too early after JDK-8286941

[Vladimir Kozlov]

On Tue, 3 Oct 2023 00:03:34 GMT, David Schlosnagle <duke at openjdk.org> wrote:

>> [JDK-8286941](https://bugs.openjdk.org/browse/JDK-8286941) changed how masked vector operations are created and processed. It calls `LoadNode::Ideal()` which tries to find best place in code for it.
>> Unfortunately some places do not expect to have vector access to modified memory.
>> In `LoadNode::find_previous_arraycopy()` the call to `ArrayCopyNode::modifies()` assumes that it access only one element and did not take into account that a vector may access wider memory.
>> The same issue exists with Unsafe mismatching (bigger size) access.
>> 
>> The fix is to take into account size of memory access.
>> 
>> New regression test is added which show the issue.
>> 
>> Tested tier1-5. xcomp, stress. Testing tier6-7 in progress.
>
> test/hotspot/jtreg/compiler/vectorization/TestMaskedVectors.java line 31:
> 
>> 29:  * @modules java.base/jdk.internal.misc
>> 30:  * @library /test/lib /
>> 31:  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
> 
> Does this need to enable diagnostic VM options the `Stress*` flags?
> 
> https://github.com/openjdk/jdk/blob/6e1aacdfba5a32f7b071eea8039888d275827e83/src/hotspot/share/opto/c2_globals.hpp#L44-L60
> 
> Suggestion:
> 
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+UnlockDiagnosticVMOptions -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors

Yes, I have it in additional changes which are coming. I am waiting for our internal tests to finish.

> test/hotspot/jtreg/compiler/vectorization/TestMaskedVectors.java line 31:
> 
>> 29:  * @modules java.base/jdk.internal.misc
>> 30:  * @library /test/lib /
>> 31:  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
> 
> Should this also test the different AVX and SVE levels where feasible?
> 
> Suggestion:
> 
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:UseAVX=3 -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:UseAVX=2 -XX:UseSVE=2 -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:UseAVX=1 -XX:UseSVE=1 -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors
>  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions -XX:UseAVX=0 -XX:UseSVE=0 -XX:-TieredCompilation -Xbatch -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,TestMaskedVectors::test* -XX:+StressLCM -XX:+StressGCM -XX:StressSeed=2210259638 TestMaskedVectors

We have internal setup which run all vectorization tests with different AVX and SSE settings.
I don't know about production Aarch64 machine which supports SVE.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16015#discussion_r1343281110
PR Review Comment: https://git.openjdk.org/jdk/pull/16015#discussion_r1343475567