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

[Roland Westrelin]

On Thu, 2 Mar 2023 08:29:47 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Maybe we should first collect all legal packs and then try to find the best alignment, dropping packs that are unaligned on those platforms that support it.
>
> Hi @rwestrel @vnkozlov .
> 
> I have another general question. Should this really vectorize?
> 
>     @Test
>     @IR(counts = { IRNode.LOAD_VECTOR, ">=1", IRNode.STORE_VECTOR, ">=1" })
>     public static void testOffHeapLong1(long dest, long[] src) {
>         for (int i = 0; i < src.length; i++) {
>             UNSAFE.putLongUnaligned(null, dest + 8 * i, src[i]);
>         }
>     }
> 
> 
> I talked with @TobiHartmann about this. It is apparently possible to get the address of an array, and store in a `long`. In that case, we could play with that rawptr a bit, and create a cyclic dependency.
> 
> Pseudocode:
> 
> long[] arr = new long[1000];
> long ptr = unsafe.getTheArrayAddress(arr); // does not exist directly, but it is possible
> ptr += 8; // shift it one long forward
> testOffHeapLong1(ptr, arr);
> 
> 
> This should have different behavior if it is vectorized or not.
> 
> But maybe this is expected, and just a bad use of `Unsafe`. Probably we want this to vectorize, for the extra performance, and people who use `Unsafe` just have to be careful not to do such strange things.

Thanks for the comments @eme64 @vnkozlov 
Looking at the code again, if `vectors_should_be_aligned()` is true, if `create_pack` is false, the current code removes every memops and already created packset with `same_velt_type()` true: That can't be motivated by a correctness issue. So I suppose we want to preserve that behavior. Wouldn't we need the change of the last commit I pushed then?

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

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