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

[Marc Chevalier]

On Fri, 21 Feb 2025 23:05:26 GMT, Dean Long <dlong at openjdk.org> wrote:

>> This collapses double shift lefts by constants in a single constant: (x << con1) << con2 => x << (con1 + con2). Care must be taken in the case con1 + con2 is bigger than the number of bits in the integer type. In this case, we must simplify to 0.
>> 
>> Moreover, the simplification logic of the sign extension trick had to be improved. For instance, we use `(x << 16) >> 16` to convert a 32 bits into a 16 bits integer, with sign extension. When storing this into a 16-bit field, this can be simplified into simple `x`. But in the case where `x` is itself a left-shift expression, say `y << 3`, this PR makes the IR looks like `(y << 19) >> 16` instead of the old `((y << 3) << 16) >> 16`. The former logic didn't handle the case where the left and the right shift have different magnitude. In this PR, I generalize this simplification to cases where the left shift has a larger magnitude than the right shift. This improvement was needed not to miss vectorization opportunities: without the simplification, we have a left shift and a right shift instead of a single left shift, which confuses the type inference.
>> 
>> This also works for multiplications by powers of 2 since they are already translated into shifts.
>> 
>> Thanks,
>> Marc
>
>> Care must be taken in the case con1 + con2 is bigger than the number of bits in the integer type. In this case, we must simplify to 0.
> 
> So `1 << 33` and `1 << 30 << 3` are still treated differently, according to the JVM spec?

@dean-long Yes! 1 << 33 was already, and is still, transformed into 1 << 1, while 1 << 30 << 3 is NOT transformed into 1 << 33 but directly into 0. The second part is exhibited by this test:

    @Test
    @IR(failOn = {IRNode.LSHIFT})
    // Checks (x << 31) << 1 => 0
    public int testDoubleShift3(int x) {
        return (x << 31) << 1;
    }

(and a few similar other). I didn't add a test for the simple `1 << 33` since my code doesn't kick in unless there are 2 shifts, so nothing should have changed here. I can add such a test if you think it's needed.

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

PR Comment: https://git.openjdk.org/jdk/pull/23728#issuecomment-2678032006