RFR: 8346664: C2: Optimize mask check with constant offset [v22]
Matthias Ernst
duke at openjdk.org
Thu Feb 13 15:44:17 UTC 2025
On Sat, 8 Feb 2025 18:30:56 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:
>
> Reword correctness (fixes).
Hey, at this point I'm happy to paste in anything you tell me, I'm not sure this is an efficient process for me to iterate on the language until it passes the bar.
Bare in mind the logic here isn't mine, I've just renamed and reworded @rwestrel 's "is_always_zero" logic. The only addition being to recognize that a CONST node can be an LSHIFT node in spirit.
If you want to decouple this, I can wait for you to add the proof to https://github.com/openjdk/jdk/commit/8af3b27ce98bcb9cf0c155c98d6b9a9bc159aafe#diff-b1bd52f0743843e15452764f48ff43c15dd3192a28bfb684b34149f0e964996eR1749 and then I'll rebase my PR?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2656999057
More information about the hotspot-compiler-dev
mailing list