RFR: 8332442: C2: refactor Mod cases in Compile::final_graph_reshaping_main_switch() [v6]

[Jasmine Karthikeyan]

On Wed, 11 Sep 2024 15:57:29 GMT, Kangcheng Xu <kxu at openjdk.org> wrote:

>> Hello all. This patch addresses [JDK-8332442](https://bugs.openjdk.org/browse/JDK-8332442) and refactors `Op_ModI`/`Op_ModL`/`Op_UModI`/`Op_UModL` cases in `DIVMOD` transforamtions. The purpose of the transformation to convert adjacent div `/`  and mod `%` operations of the same operands into one should the platform support this feature (e.g., x86-64).
>> 
>> I took the liberty adding _signed_ DIVMOD nodes (i.e., `DIV_MOD_I` and `DIV_MOD_L`) to the `IRNode` class constants as they are previously missing. Please let me know if they were left intentionally and if there are any other concerns. Thanks!
>> 
>> ~This will be a draft PR before GHA tests are confirmed passing.~
>
> Kangcheng Xu 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 12 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into refactor-mod-cases
>  - improve formatting, nullptr comparison, update test flags, use custom @Run tests
>  - allow one div on platforms without hardware divmod
>  - remove platform restriction
>  - Merge branch 'openjdk:master' into refactor-mod-cases
>  - include aarch64 in tests, add more configuration combinations
>  - remove redundant arguments, test with -XX:-UseDivMod
>  - Merge branch 'master' into refactor-mod-cases
>  - Add test and IRNode for signed int/long divmod
>  - created IR tests
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/bf2d8c7b...fe7b82ef

src/hotspot/share/opto/compile.cpp line 3181:

> 3179:     n->subsume_by(divmod->mod_proj(), this);
> 3180:   } else {
> 3181:     // replace "a % b" with "a - ((a / b) *b)"

Suggestion:

    // Replace "a % b" with "a - ((a / b) * b)"

test/hotspot/jtreg/compiler/c2/TestDivModNodes.java line 33:

> 31: /*
> 32:  * @test
> 33:  * @summary Test that DIV and MOD nodes are converted into DIVMOD where possible

I think you're missing `@bug 8332442` here.

test/hotspot/jtreg/compiler/c2/TestDivModNodes.java line 68:

> 66:     private static void runSignedIntDivMod() {
> 67:         int dividend = RANDOM.nextInt();
> 68:         int divisor = RANDOM.nextInt();

Since `nextInt()` can return 0, this might lead to a very rare test failure since div/mod by 0 results in an `ArithmeticException`. You could do `divisor = (divisor == 0) ? 1 : divisor` to avoid that, like we do in the other division idealization tests:
https://github.com/openjdk/jdk/blob/51b85a1f692fed7a66bdc0fae21438a60aafe7c2/test/hotspot/jtreg/compiler/c2/irTests/DivINodeIdealizationTests.java#L45-L46

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20877#discussion_r1755363063
PR Review Comment: https://git.openjdk.org/jdk/pull/20877#discussion_r1755371635
PR Review Comment: https://git.openjdk.org/jdk/pull/20877#discussion_r1755392758