RFR: 8347459: C2: missing transformation for chain of shifts/multiplications by constants [v4]

[Marc Chevalier]

On Fri, 21 Feb 2025 18:39:23 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   comment
>
> src/hotspot/share/opto/mulnode.cpp line 996:
> 
>> 994: 
>> 995:   if (con0 + con1 >= nbits) {
>> 996:     return ConNode::make(TypeInteger::zero(bt));
> 
> It'd be clearer to do this, which is more equivalent but more concise:
> Suggestion:
> 
>     return phase->zerocon(bt);

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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23728#discussion_r1969744526