RFR: 8300258: C2: vectorization fails on simple ByteBuffer loop [v2]

[Emanuel Peter]

On Tue, 21 Feb 2023 13:02:27 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Why are you not changing it here?
>> https://github.com/openjdk/jdk/blob/6751978122afc5102db84c7f040a0b0ab76e70b6/src/hotspot/share/opto/superword.cpp#L728-L739
>> 
>> If we just removed the memory slice where `best_align_to_mem_ref` was in, then should we not find a new `best`?
>> 
>> I fear there is now a way to trigger the assert:
>> https://github.com/openjdk/jdk/blob/6751978122afc5102db84c7f040a0b0ab76e70b6/src/hotspot/share/opto/superword.cpp#L736
>> How can we be sure that none of the other packs do not contain a memop with the same type as `mem_ref`?
>
>> Why are you not changing it here?
>> 
>> https://github.com/openjdk/jdk/blob/6751978122afc5102db84c7f040a0b0ab76e70b6/src/hotspot/share/opto/superword.cpp#L728-L739
>> 
>> If we just removed the memory slice where `best_align_to_mem_ref` was in, then should we not find a new `best`?
> 
> I didn't because for alignment heuristics what matters AFAIU is operand size so the same memory slice check doesn't seem correct. But you're right that there's a problem if `best_align_to_mem_ref` is not part of any pack.
> 
>> I fear there is now a way to trigger the assert:
> 
> Yes there must be.
> 
> I find the code quite confusing because it mixes correctness issues with best alignment heuristics. @vnkozlov I saw in the history that you made the changes that are discussed here. What do you think of this change?

@rwestrel I think we can refactor this code in SuperWord.
I hope this change will also go into that direction: https://github.com/openjdk/jdk/pull/12350

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

PR: https://git.openjdk.org/jdk/pull/12440