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

Marc Chevalier duke at openjdk.org
Fri Mar 7 13:52:01 UTC 2025


On Fri, 7 Mar 2025 13:12:18 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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/24483157...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.

I can also do that, but I think using the constants is more valuable than only random numbers: most of random int or long won't have some corner properties I try to cover here. On top of that, with the comments, they help enumerating clearly different cases to look at. So, I'd be happy to add some randomization, but I strongly feel I should keep the constant ones as well.

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

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


More information about the hotspot-compiler-dev mailing list