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

Emanuel Peter epeter at openjdk.org
Fri Mar 7 13:15:01 UTC 2025


On Fri, 7 Mar 2025 13:11:39 GMT, Marc Chevalier <duke 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
>
> Marc Chevalier has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 16 additional commits since the last revision:
> 
>  - use a random number in testing
>  - Merge branch 'master' into fix/missing-transformation-for-chain-of-shifts-multiplications-by-constants
>  - + comment on why not zerocon
>  - comment
>  - Fix style in the few lines I haven't touched yet
>  - Remove useless local, with especially helpful name
>  - rename
>  - Add test suggested by @dean-long exhibiting the difference between (x << 30) << 3 and x << 33
>  - improve simplification of double shifts in stores
>  - actually return a new node
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/fa1b30c2...8e93a12f

test/hotspot/jtreg/compiler/c2/irTests/LShiftLNodeIdealizationTests.java line 222:

> 220:     public long testDoubleShift9(long x) {
> 221:         return (x << 62L) << 3L;
> 222:     }

I see that you have quite a few examples here with fixed constants. It would be good to extend this with random constants. You can do that with `static final` fields, as they are constant by the time we JIT compile.


private static final int CON0 = RANDOM.nextInt();
private static final int CON1 = RANDOM.nextInt();

    @Test
    public long test(int x) {
        return (x << CON0) << CON1;
    }

I would give you "bonus points" if you use `Generators.java`, because that produces more interesting constants.

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

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


More information about the hotspot-compiler-dev mailing list