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