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

[Quan Anh Mai]

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

The issue is interesting. The code shape looks like this `AndI(CastII(Phi), ConI)`. `Phi` is a constant 0 while `CastII` is `top`. `CastII` is `top` because its control input is dead. And, as a result, `CastII` does not change after CCP and we do not push its inputs (in this case the `AndI`) to the worklist, resulting in the `AndI` remains to be a `top`. On the other hand, `AndINode::Value` looks through `CastIINode`s (in `AndIL_min_trailing_zeros` we do `expr = expr->uncast()`). Which means that it sees the `Phi` being a constant 0, and returns `TypeInt::ZERO`.

Of course, in this particular case, a solution is to check for top inputs before proceeding with any action. However, I think that it is not sufficient. Given that we often look through `CastNode`s when doing inference, I think it is suitable we push nodes to the worklist through `CastNode`s, too.

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

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