RFR: 8346664: C2: Optimize mask check with constant offset [v24]
Emanuel Peter
epeter at openjdk.org
Tue Feb 25 07:26:08 UTC 2025
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
About severity: As long as we find and integrate a fix during `JDK25` it is fine (the issue does not break the CI that badly at the moment). If we get close to rampdown, then we have to consider if we want to backout 8346664 or if we defer the bug to `JDK26`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2680920491
More information about the hotspot-compiler-dev
mailing list