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

[Roland Westrelin]

On Tue, 25 Feb 2025 09:27:13 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 with a new target base due to a merge or a rebase. The pull request now contains 66 commits:
> 
>  - Merge branch 'master' into JDK-8323582-SW-native-alignment
>  - stall -> delay, plus some more comments
>  - adjust selector if probability
>  - Merge branch 'master' into JDK-8323582-SW-native-alignment
>  - remove multiversion mark if we break the structure
>  - register opaque with igvn
>  - copyright and rm CFG check
>  - IR rules for all cases
>  - 3 test versions
>  - test changed to unaligned ints
>  - ... and 56 more: https://git.openjdk.org/jdk/compare/d551daca...8eb52292

Would it be possible and make sense to remove useless slow path loops the way it's done for predicates or zero trip guards? In `PhaseIdealLoop::build_loop_late_post_work()`, collect all `OpaqueMultiversioningNode` in a list. Then iterate over all loops the way it's done in `PhaseIdealLoop::eliminate_useless_zero_trip_guard()`, find loops marked as multi version, check we can get from the loop to the `OpaqueMultiversioningNode` and mark that one as useful. Eliminate all `OpaqueMultiversioningNode` not marked as useful. That way if some transformation such as peeling makes the loop non multi version or if the expected shape breaks for some reason, the slow loop is eliminated on next loop opts pass.

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

PR Comment: https://git.openjdk.org/jdk/pull/22016#issuecomment-2684365921