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

[Matthias Ernst]

On Fri, 14 Feb 2025 07:18:52 GMT, Matthias Ernst <duke at openjdk.org> wrote:

>> Fixes [JDK-8346664](https://bugs.openjdk.org/browse/JDK-8346664): extends the optimization of masked sums introduced in #6697 to cover constant values, which currently break the optimization.
>> 
>> Such constant values arise in an expression of the following form, for example from `MemorySegmentImpl#isAlignedForElement`:
>> 
>> 
>> (base + (index + 1) << 8) & 255
>> => MulNode
>> (base + (index << 8 + 256)) & 255
>> => AddNode
>> ((base + index << 8) + 256) & 255
>> 
>> 
>> Currently, `256` is not being recognized as a shifted value. This PR enables further reduction:
>> 
>> 
>> ((base + index << 8) + 256) & 255
>> => MulNode (this PR)
>> (base + index << 8) & 255
>> => MulNode (PR #6697)
>> base & 255 (loop invariant)
>> 
>> 
>> Implementation notes:
>> * I verified that the originating issue "scaled varhandle indexed with i+1"  (https://mail.openjdk.org/pipermail/panama-dev/2024-December/020835.html) is resolved with this PR.
>> * ~in order to stay with the flow of the current implementation, I refrained from solving general (const & mask)==0 cases, but only those where const == _ << shift.~
>> * ~I modified existing test cases adding/subtracting from the index var (which would fail with current C2). Let me know if would like to see separate cases for these.~
>
> Matthias Ernst has updated the pull request incrementally with one additional commit since the last revision:
> 
>   incorporate @eme64's comment suggestions

Concretely, my understanding of what's breaking here is the following:

`AndI ( CastII ( ConI ) )` : `AndI` reaches through the cast and, since 8346664, also recognizes constants in addition to shifts and can eliminate. This currently gets stuck on the Cast node in CCP.

Pushing the And node fixes the crash and looks straightforward, but it makes me wonder why it is necessary, and why the change doesn't bubble through the Cast already: shouldn't the cast node be re-pushed when its input changes,`CastII(Con).Value()` now return the constant, and then bubble further to the And? It appears to me that either there's something off in Cast's Value handling (there's more to it than expected ;-), OR - if it is somehow necessary to preserve this cast - it is not clear to me why And (and others) reaching _through_ the Cast is legal in the first place. Does this make sense?

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

PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2694307169