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

[Emanuel Peter]

On Mon, 10 Mar 2025 15:31:29 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Random testing, trying...
>
> src/hotspot/share/opto/memnode.cpp line 3541:
> 
>> 3539: // +------------------------+---------+
>> 3540: //  31                     8 7        0
>> 3541: // v[0..7] is meaningful, but v[8..31] is not. In this case, num_rejected_bits == 24.
> 
> Would this example not be nice with the original case above?
> 
> // Check for useless sign-extension before a partial-word store
> // (StoreB ... (RShiftI _ (LShiftI _ valIn conIL ) conIR) )
> // If (conIL == conIR && conIR <= num_bits)  this simplifies to
> // (StoreB ... (valIn) )
> 
> Because it seems you are assuming here that `conIL == conIR`, right? And then below you ask what if they are not equal.

It could also be nice to introduce the `num_rejected_bits` somewhere.

> src/hotspot/share/opto/memnode.cpp line 3579:
> 
>> 3577: // The non-rejected bits are the 8 lower one of (v << conIL - conIR).
>> 3578: // The bits 6 and 7 of v have been thrown away after the shift left.
>> 3579: // The simplification is still fine.
> 
> Suggestion for everywhere: `fine` -> `valid`.

Or correct.

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

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