RFR: 8347459: C2: missing transformation for chain of shifts/multiplications by constants [v4]
Marc Chevalier
duke at openjdk.org
Wed Feb 26 15:33:53 UTC 2025
On Wed, 26 Feb 2025 15:28:03 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:
>> Actually, this is not equivalent and incorrect. I've did this exact mistake in an earlier version. The problem is that `zerocon` caches the nodes:
>> https://github.com/openjdk/jdk/blob/8cfebc41dc8ec7b0d24d9c467b91de82d28b73fc/src/hotspot/share/opto/phaseX.cpp#L654-L656
>> So, then, we likely (or at least may) return an old node, which is not legal: `Ideal` is only allowed to return `this`, `nullptr` or a new node. But yes, it's unfortunate because it'd be much lighter to read.
>
> Ah, that's tricky! But that makes a lot of sense to me. Maybe it could be worth adding a comment there to mention the fact.
You're right, it's tricky enough to be worth it. I will add a comment, good idea.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23728#discussion_r1971829030
More information about the hotspot-compiler-dev
mailing list