RFR: 8323582: C2 SuperWord AlignVector: misaligned vector memory access with unaligned native memory [v3]

[Vladimir Kozlov]

On Thu, 20 Feb 2025 07:21:45 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Note: the approach with Predicates and Multiversioning prepares us well for Runtime Checks for Aliasing Analysis, see more below.
>> 
>> **Background**
>> 
>> With `-XX:+AlignVector`, all vector loads/stores must be aligned. We try to statically determine if we can always align the vectors. One condition is that the address `base` is already aligned. For arrays, we know that this always holds, because they are `ObjectAlignmentInBytes` aligned. But with native memory, the `base` is just some arbitrarily aligned pointer.
>> 
>> **Problem**
>> 
>> So far, we have just naively assumed that the `base` is always `ObjectAlignmentInBytes` aligned. But that does not hold for `native` memory segments: the `base` can also be unaligned. I had constructed such an example, and with `-XX:+AlignVector -XX:+VerifyAlignVector` this example hits the verification code.
>> 
>> 
>> MemorySegment nativeAligned = Arena.ofAuto().allocate(RANGE * 4 + 1);
>> MemorySegment nativeUnaligned = nativeAligned.asSlice(1);
>> test3(nativeUnaligned);
>> 
>> 
>> When compiling the test method, we assume that the `nativeUnaligned.address()` is aligned - but it is not!
>> 
>>     static void test3(MemorySegment ms) {
>>         for (int i = 0; i < RANGE; i++) {
>>             long adr = i * 4L;
>>             int v = ms.get(ELEMENT_LAYOUT, adr);
>>             ms.set(ELEMENT_LAYOUT, adr, (int)(v + 1));
>>         }
>>     }
>> 
>> 
>> **Solution: Runtime Checks - Predicate and Multiversioning**
>> 
>> Of course we could just forbid cases where we have a `native` base from vectorizing. But that would lead to regressions currently - in most cases we do get aligned `base`s, and we currently vectorize those. We cannot statically determine if the `base` is aligned, we need a runtime check.
>> 
>> I came up with 2 options where to place the runtime checks:
>> - A new "auto vectorization" Parse Predicate:
>>   - This only works when predicates are available.
>>   - If we fail the predicate, then we recompile without the predicate. That means we cannot add a check to the predicate any more, and we would have to do multiversioning at that point if we still want to have a vectorized loop.
>> - Multiversion the loop:
>>   - Create 2 copies of the loop (fast and slow loops).
>>   - The `fast_loop` can make speculative alignment assumptions, and add the corresponding check to the `multiversion_if` which decides which loop we take
>>   - In the `slow_loop`, we make no assumption which means we can not vectorize, but we still compile - so even ...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   adjust selector if probability

How profitable (performance wise) to optimize slow path loop? Can we skip any optimizations for it - treat it as not-Counted?

src/hotspot/share/opto/loopTransform.cpp line 3363:

> 3361:   if (cl->is_pre_loop() || cl->is_post_loop()) return true;
> 3362: 
> 3363:   // If we are stalled, check if we can get unstalled.

Can you expand comment explaining cases when we "stall" and what it means?

src/hotspot/share/opto/loopopts.cpp line 4514:

> 4512: // and then rejecting the slow_loop by constant folding the multiversion_if.
> 4513: //
> 4514: // Therefore, we "stall" the optimization of the slow_loop until we add

We don't use "stall" term. We use "delay" - this is what happens here if I understand it correctly.

src/hotspot/share/opto/loopopts.cpp line 4520:

> 4518: // multiversion_if folds away the "stalled" slow_loop. If we add any
> 4519: // speculative assumption, then we mark the OpaqueMultiversioningNode
> 4520: // with "unstall_slow_loop", so that the slow_loop can be optimized.

"unstall_slow_loop" - > "optimize_slow_loop"

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

PR Review: https://git.openjdk.org/jdk/pull/22016#pullrequestreview-2633960596
PR Review Comment: https://git.openjdk.org/jdk/pull/22016#discussion_r1966019182
PR Review Comment: https://git.openjdk.org/jdk/pull/22016#discussion_r1966028103
PR Review Comment: https://git.openjdk.org/jdk/pull/22016#discussion_r1966032230