RFR: 8310190: C2 SuperWord: AlignVector is broken, generates misaligned packs

Emanuel Peter epeter at openjdk.org
Tue Nov 14 08:45:36 UTC 2023


On Tue, 14 Nov 2023 06:39:35 GMT, Fei Gao <fgao at openjdk.org> wrote:

>>> Would you mind re-running testing on your platforms? (I ran on x64 and aarch64 without SVE)
>> 
>> Hi @eme64 , I've submitted testing on aarch64 (sve) and arm32 separately. Let's wait for results.
>> 
>>> @fg1417 I don't know aarch64 very well. Would you want to create a matching-rule for aarch64 for `VerifyVectorAlignment`, similar to what I did in `x86.ad`? It is not necessary, but could be nice.
>> 
>> Of course, I will create the matching rule for aarch64. BTW, what kind of testing have you done for `VerifyVectorAlignment` to see if it works as expected? Then I can verify it as well on aarch64.
>
>> @fg1417 thanks for the help! I ran tier1-6 with `-XX:+AlignVector -XX:+IgnoreUnrecognizedVMOptions -XX:+VerifyAlignVector`. Though `VerifyAlignVector` only has an effect if there is a matching rule in the `ad` files. And I ran quite a few repetitions of `compiler/loopopts/superword/TestAlignVectorFuzzer.java`.
> 
> Hi @eme64, we have [ `tst`](https://developer.arm.com/documentation/ddi0602/2023-09/Base-Instructions/TST--immediate---Test-bits--immediate---an-alias-of-ANDS--immediate--?lang=en) on aarch64.
> 
> instruct verify_vector_alignment(iRegP r, immL_positive_bitmaskI mask, rFlagsReg cr) %{
>   match(Set r (VerifyVectorAlignment r mask));
>   effect(KILL cr);
>   format %{ "verify_vector_alignment $r $mask \t! verify alignment" %}
>   ins_encode %{
>     Label Lskip;
>     // check if masked bits of r are zero
>     __ tst($r$$Register, $mask$$constant);
>     __ br(Assembler::EQ, Lskip);
>     __ stop("verify_vector_alignment found a misaligned vector memory access");
>     __ bind(Lskip);
>   %}
>   ins_pipe( pipe_slow );
> %}
> 
> 
> I tested tier1-tier3 with `-XX:+AlignVector -XX:+IgnoreUnrecognizedVMOptions -XX:+VerifyAlignVector` on aarch64(neon and 128-bit sve) platforms, and several repetitions of `compiler/loopopts/superword/TestAlignVectorFuzzer.java`. No new failures found. Hope it works for you. Thanks.

@fg1417 @zifeihan @reinrich  thanks for the testing!

@fg1417 thanks for the snippet! Where do I add it? I don't know how the `ad` and `m4` work. Is there some script that converts them?

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

PR Comment: https://git.openjdk.org/jdk/pull/14785#issuecomment-1809761367


More information about the hotspot-compiler-dev mailing list