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

[Fei Gao]

On Wed, 6 Dec 2023 09:49:14 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/superword.cpp line 3916:
>> 
>>> 3914: 
>>> 3915:   // We chose an aw that is the maximal possible vector width for the type of
>>> 3916:   // align_to_ref.
>> 
>> I have a question here: Could we always get benefit from aligning address to maximal possible vector width for small-size types, as vector width becomes large? E.g., for `byte` type on `512-bit` platform, will the pre-loop limit become very large and cost much more to execute the whole loop?
>
> I agree, this is a concern. But this was already like that before my fix:
> `int vw = vector_width_in_bytes(p.mem());`
> 
> If we want to relax this, I suggest we do that in an a future RFE.
> Some thoughts:
> - I did not want to change this now, since it may affect performance, and this is a bug fix here.
> - We may relax the `aw` to be the maximal maximal vector size used in the vectorization. Or we can make it even smaller, i.e. `MIN2(max_vw, ObjectAlignmentInBytes);', which would then be analogue to `AlignmentSolution SuperWord::pack_alignment_solution`.
> - We may even want to completely remove pre-loop adjustment if alignment is neither a strict requirement, and actually on average a performance cost that outweighs the performance gains of alignment. It is for example questionable to align one of the mem_refs if there are multiple, and hence we cannot guarantee alignment of all anyway.

Make sense to me. Thanks for your reply!

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

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