RFR: 8324751: C2 SuperWord: Aliasing Analysis runtime check [v6]

[Emanuel Peter]

On Mon, 11 Aug 2025 09:08:49 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update test/hotspot/jtreg/compiler/loopopts/superword/TestAliasingFuzzer.java
>>   
>>   Co-authored-by: Manuel Hässig <manuel at haessig.org>
>
> src/hotspot/share/opto/vectorization.cpp line 598:
> 
>> 596: //   If iv_stride <= 0, i.e. last <= iv <= init:
>> 597: //     (iv - init) * scale_1 >= (iv - init) * iv_scale
>> 598: //     (iv - last) * scale_1 <= (iv - last) * iv_scale                   (NEG-STRIDE)
> 
> Suggestion:
> 
> //   If iv_stride >= 0, i.e. init <= iv <= last:
> //     (iv - init) * iv_scale_1 <= (iv - init) * iv_scale2
> //     (iv - last) * iv_scale_1 >= (iv - last) * iv_scale2                   (POS-STRIDE)
> //   If iv_stride <= 0, i.e. last <= iv <= init:
> //     (iv - init) * iv_scale_1 >= (iv - init) * iv_scale2
> //     (iv - last) * iv_scale_1 <= (iv - last) * iv_scale2                   (NEG-STRIDE)
> 
> If I am not massively confused, the `iv_scale`s should be like this.

Correct! except that it should be `iv_scale_1` -> `iv_scale1` ;)
Together we can do it eventually 🙈 😆

> src/hotspot/share/opto/vectorization.cpp line 604:
> 
>> 602: //       p1(init) + size1 <= p2(init)       (if iv_stride >= 0)  |    p2(last) + size2 <= p1(last)      (if iv_stride >= 0)   |
>> 603: //       p1(last) + size1 <= p2(last)       (if iv_stride <= 0)  |    p2(init) + size2 <= p1(init)      (if iv_stride <= 0)   |
>> 604: //       ----- is equivalent to -----                            |    ----- is equivalent to -----                            |
> 
> Suggestion:
> 
> //       ---- are equivalent to -----                            |    ---- are equivalent to -----                            |
> 
> This confused me a bit 😅

Sure, sounds good :)

> src/hotspot/share/opto/vectorization.cpp line 625:
> 
>> 623: //       <= size1 + p1(init) - init * iv_scale2 + iv * iv_scale2 |    <= size2 + p2(last) - init * iv_scale1 + iv * iv_scale1 |
>> 624: //          -- assumption --                                     |       -- assumption --                                     |
>> 625: //       <=         p2(init) - init * iv_scale2 + iv * iv_scale2 |    <=         p1(last) - init * iv_scale1 + iv * iv_scale1 |
> 
> Suggestion:
> 
> //        = size1 + p1(init) - init * iv_scale1 + iv * iv_scale1 |     = size2 + p2(last) - last * iv_scale2 + iv * iv_scale2 |
> //                           ------ apply (POS-STRIDE) --------- |                        ------ apply (POS-STRIDE) --------- |
> //       <= size1 + p1(init) - init * iv_scale2 + iv * iv_scale2 |    <= size2 + p2(last) - last * iv_scale1 + iv * iv_scale1 |
> //          -- assumption --                                     |       -- assumption --                                     |
> //       <=         p2(init) - init * iv_scale2 + iv * iv_scale2 |    <=         p1(last) - last * iv_scale1 + iv * iv_scale1 |
> 
> 
> `LINEAR-FORM-LAST: p1(iv) = p1(last) - last * iv_scale1 + iv * iv_scale1`

Nice catch!

> src/hotspot/share/opto/vectorization.cpp line 639:
> 
>> 637: //       <= size1 + p1(last) - init * iv_scale2 + iv * iv_scale2 |    <= size2 + p2(init) - init * iv_scale1 + iv * iv_scale1 |
>> 638: //          -- assumption --                                     |       -- assumption --                                     |
>> 639: //       <=         p2(last) - init * iv_scale2 + iv * iv_scale2 |    <=         p1(init) - init * iv_scale1 + iv * iv_scale1 |
> 
> Suggestion:
> 
> //        = size1 + p1(last) - last * iv_scale1 + iv * iv_scale1 |     = size2 + p2(init) - init * iv_scale2 + iv * iv_scale2 |
> //                           ------ apply (NEG-STRIDE) --------- |                        ------ apply (NEG-STRIDE) --------- |
> //       <= size1 + p1(last) - last * iv_scale2 + iv * iv_scale2 |    <= size2 + p2(init) - init * iv_scale1 + iv * iv_scale1 |
> //          -- assumption --                                     |       -- assumption --                                     |
> //       <=         p2(last) - last * iv_scale2 + iv * iv_scale2 |    <=         p1(init) - init * iv_scale1 + iv * iv_scale1 |

Same case, thanks!

> src/hotspot/share/opto/vectorization.cpp line 742:
> 
>> 740: //              a solution that also works when the loop is not entered:
>> 741: //
>> 742: //              k = (init - stride - 1) / abs(stride)
> 
> Suggestion:
> 
> //              k = (init - limit - 1) / abs(stride)
> 
> Where does `stride` come from? If I did not miss anything, this should be `limit`.

>From a blip in the brain I suppose :face_with_peeking_eye: 
Thanks for spotting it!

> src/hotspot/share/opto/vectorization.cpp line 895:
> 
>> 893:   Node* diffL = (stride > 0) ? new SubLNode(limitL, initL)
>> 894:                              : new SubLNode(initL, limitL);
>> 895:   Node* diffL_m1 = new AddLNode(diffL, igvn.longcon(-1));
> 
> Out of curiosity, why did you choose `AddL(diff, -1)` over `SubL(diff, 1)`?

I think it would otherwise just get cannonicalized to `AddL` anyway. I'm saving IGVN from doing that additional work. For me this is a detail, some may call it premature optimization ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270284362
PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270288117
PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270294118
PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270294865
PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270300217
PR Review Comment: https://git.openjdk.org/jdk/pull/24278#discussion_r2270306868