RFR: 8300258: C2: vectorization fails on simple ByteBuffer loop

[Roland Westrelin]

On Wed, 8 Feb 2023 11:45:02 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> The loop that doesn't vectorize is:
>> 
>> 
>> public static void testByteLong4(byte[] dest, long[] src, int start, int stop) {
>>     for (int i = start; i < stop; i++) {
>>         UNSAFE.putLongUnaligned(dest, 8 * i + baseOffset, src[i]);
>>     }
>> }
>> 
>> 
>> It's from a micro-benchmark in the panama
>> repo. `SuperWord::find_adjacent_refs() `prevents it from vectorizing
>> because it finds it cannot properly align the loop and, from the
>> comment in the code, that:
>> 
>> 
>> // Can't allow vectorization of unaligned memory accesses with the
>> // same type since it could be overlapped accesses to the same array.
>> 
>> 
>> The test for "same type" is implemented by looking at the memory
>> operation type which in this case is overly conservative as the loop
>> above is reading and writing with long loads/stores but from and to
>> arrays of different types that can't overlap. Actually, with such
>> mismatched accesses, it's also likely an incorrect test (reading and
>> writing could be to the same array with loads/stores that use
>> different operand size) eventhough I couldn't write a test case that
>> would trigger an incorrect execution.
>> 
>> As a fix, I propose implementing the "same type" test by looking at
>> memory aliases instead.
>
> test/hotspot/jtreg/compiler/c2/irTests/TestVectorizationMismatchedAccess.java line 39:
> 
>> 37:  */
>> 38: 
>> 39: public class TestVectorizationMismatchedAccess {
> 
> Could you have a much simpler example, just with two int-arrays, where one allows vectorization and one does not. We still want there to be some vectorization. Maybe like this, maybe you have to play around with it a bit:
> 
> // make sure arr1 and arr2 are in different slices
> for (int i ...) {
>   arr1[i] = arr1[i-2] + 5; // do not vectorize, cyclic dependency
>   arr2[i] = arr2[i] + 5; // should vectorize
> }

I agree that we should have such a test if we don't have one already but does it belong to that bug? That bug changes the behavior with mismatched accesses so it feels strange to add an unrelated test.` testByteByte1() `is a mismatched access test that vectorizes and that's unaffected by my change. I could add another one that doesn't vectorize because of a dependency with a similar mismatched access. What do you think?

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

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