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

Roland Westrelin roland at openjdk.org
Tue Sep 10 11:54:06 UTC 2024


On Mon, 9 Sep 2024 20:46:24 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 incrementally with one additional commit since the last revision:
> 
>   remove redundant arguments, test with -XX:-UseDivMod

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

> 32:  * @test
> 33:  * @summary Test DIV and MOD nodes are converted into DIVMOD where possible
> 34:  * @requires os.arch=="amd64" | os.arch=="x86_64"

So this can run on aarch64 now?

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

> 45:     @Arguments(values = {Argument.RANDOM_EACH, Argument.RANDOM_EACH})
> 46:     @IR(counts = {IRNode.DIV_MOD_I, "1" }, applyIf = {"UseDivMod", "true"})
> 47:     @IR(failOn = {IRNode.DIV_MOD_I}, applyIf = {"UseDivMod", "false"})

Rather than check that there's no `DivMod` node, you could check that there are the expected nodes (1 `Mul`, 1 `Sub`).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20877#discussion_r1751801808
PR Review Comment: https://git.openjdk.org/jdk/pull/20877#discussion_r1751801516


More information about the hotspot-compiler-dev mailing list