RFR: 8346664: C2: Optimize mask check with constant offset [v9]

[Emanuel Peter]

On Thu, 30 Jan 2025 14:40:50 GMT, Matthias Ernst <duke at openjdk.org> wrote:

>> Matthias Ernst has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   TestEquivalentInvariants: split scenarios that differ by 'AlignVector'
>>   
>>   Three variants (3d 3d2 3e) start vectorizing with -XX:-AlignVector, whereas they remain unvectorized under -XX:+AlignVector. Constrain these to AlignVector=false, and add 3 new variants (suffix 'a') with AlignVector=true.
>>   
>>   Special attention to 3e: it says
>>       // Should never vectorize, since i1 and i2 are not guaranteed to be adjacent
>>       // invar2 + invar3 could overflow, and the address be valid with and without overflow.
>>       // So both addresses are valid, and not adjacent.
>>   but now it does vectorize.
>
> test/hotspot/jtreg/compiler/loopopts/superword/TestEquivalentInvariants.java line 831:
> 
>> 829:     // Should never vectorize, since i1 and i2 are not guaranteed to be adjacent
>> 830:     // invar2 + invar3 could overflow, and the address be valid with and without overflow.
>> 831:     // So both addresses are valid, and not adjacent.
> 
> This needs extra attention: it _does_ vectorize with `applyIf = {"AlignVector", "false"}`, so something is off.

Ah, this indeed sounds scary. Hmm let me try to remember what I was thinking here... this was rather complicated.

If you are interested to read up more on this (no need), see here:
`src/hotspot/share/opto/mempointer.hpp`
Look for `overflow` and the definition of `SAFE` decompisition, and the `MemPointer Lemma`.

We are doing int accesses with int stride (`getAtIndex` for `int` layout). So if there was a different overflow, we would overflow by a factor of `2^32` indices, which would put us outside the range of an `int[]` size which is maximally about `2^31`.

Note that we feed the method only `int[]`.

If we were to feed in `long[]`, then we could possibly overflow the index, and end up still inside the array, as the jump would be `2^32 * 4` bytes, which is inside close to the maximal capacity of an `long[]`, i.e. `2^31 * 8` bytes.

Why don't you make an exact copy of the method, but instead feed it `long[]`. Then, I think it would not vectorize. This would be helpful to confirm my theory. That would make me feel ok to just remove the comment, or rather move the comment to the `long[]` case ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22856#discussion_r1935787123