RFR: 8349361: C2: RShiftL should support all applicable transformations that RShiftI does [v9]

[Emanuel Peter]

On Fri, 14 Feb 2025 15:57:54 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> This change refactors `RShiftI`/`RshiftL` `Ideal`, `Identity` and
>> `Value` because the `int` and `long` versions are very similar and so
>> there's no logic duplication. In the process, support for some extra
>> transformations is added to `RShiftL`. I also added some new test
>> cases.
>
> Roland Westrelin 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 15 additional commits since the last revision:
> 
>  - review
>  - review
>  - review
>  - Merge branch 'master' into JDK-8349361
>  - Update src/hotspot/share/opto/mulnode.cpp
>    
>    Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>
>  - Update src/hotspot/share/opto/mulnode.cpp
>    
>    Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>
>  - review
>  - Update src/hotspot/share/opto/mulnode.hpp
>    
>    Co-authored-by: Jasmine Karthikeyan <25208576+jaskarth at users.noreply.github.com>
>  - Update src/hotspot/share/opto/mulnode.cpp
>    
>    Co-authored-by: Jasmine Karthikeyan <25208576+jaskarth at users.noreply.github.com>
>  - Update src/hotspot/share/opto/mulnode.cpp
>    
>    Co-authored-by: Jasmine Karthikeyan <25208576+jaskarth at users.noreply.github.com>
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/bdb3f04d...5b05d222

test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java line 125:

> 123:     final static int test7Shift = RunInfo.getRandom().nextInt(32) + 32;
> 124:     final static long test7Min = -1L << (64 - test7Shift -1);
> 125:     final static long test7Max = ~test7Min;

Would you mind adding a quick comment about why you chose the values the way you do?

test/hotspot/jtreg/compiler/c2/irTests/RShiftLNodeIdealizationTests.java line 145:

> 143:     public long test9(long x) {
> 144:         x = Integer.max(Integer.min((int)x, (int)test7Max), (int)(test7Min-1));
> 145:         return ((x << test7Shift) >> test7Shift);

It could be nice to have some test cases where both shift values are completely randomized.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23438#discussion_r1967923729
PR Review Comment: https://git.openjdk.org/jdk/pull/23438#discussion_r1967922247