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

[Vladimir Kozlov]

On Wed, 11 Oct 2023 14:31:08 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/compile.cpp line 3668:
>> 
>>> 3666: 
>>> 3667:   case Op_LoadVector:
>>> 3668:   case Op_StoreVector:
>> 
>> What about Masked Load and Store vectors?
>
> @vnkozlov for now, I only mark the vectors produced from SuperWord. And SuperWord only produces LoadVector/StoreVector, and nothing masked.
> The masked vector memops come from the VectorAPI. But the VectorAPI does not respect AlignVector, and may generate misaligned vector ops - and then the verification fails.

ok

>> src/hotspot/share/opto/compile.cpp line 3672:
>> 
>>> 3670: #ifdef AMD64
>>> 3671:     // Add VerifyAlignment node between adr and load / store.
>>> 3672:     if (AlignVector && VerifyAlignVector) {
>> 
>> You should check only VerifyAlignVector here. In `CompilerConfig::check_args_consistency()` you need to check that VerifyAlignVector could be set to true only if AlignVector is true. See PostLoopMultiversioning as example there. Don't forget #ifdef ASSERT there.
>
> `CompilerConfig::check_args_consistency()` seems to be too late. The reason is that `AlignVector` can still be changed by `VM_Version::get_processor_features` because of `UseUnalignedLoadStores`.
> @vnkozlov do you have any other suggestion where to do this - ie when all the flags are final?

You are right about. I surprise we did not have such issue before (where to put some C2 flags setting).
I think `C2Compiler::init_c2_runtime()` could be good place for that. I don't want to do that in `Compile::Init()` or other Compile's methods which are called for each compilation.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14785#discussion_r1355299483
PR Review Comment: https://git.openjdk.org/jdk/pull/14785#discussion_r1355334673