RFR: 8372302: C2: IGVN verification fails because ModXNode::Ideal creates unused intermediate nodes

[Emanuel Peter]

On Tue, 25 Nov 2025 10:35:36 GMT, Benoît Maillard <bmaillard at openjdk.org> wrote:

> This PR addresses a failure in IGVN verification with `ModI` and `ModL` nodes.
> 
> In `ModeXNode::Ideal`, we have code to optimize a modulo expression by expressing it in terms of other operations. There are actually two distinct cases, one where the divisor is a constant and is equal to `modulo 2^k-1` for some integer `k`, and a more general case where other transformations do not succeed. Because these transformations involve creating several new nodes (sometimes in a loop) and calling `phase->transform(...)` on them, we want to avoid accidentally triggering optimizations on the "unfinished" state of the subgraph. For this, we create a temporary dummy node and add edges to the nodes being constructed.
> 
> There are some execution paths where the node is not destroyed before `Ideal` returns, and this creates issues during IGVN verification, as the verification code checks if the number of nodes has changed after having called `Ideal` on a given node and not expecting changes.
> 
> The path in question is when we exit because the divisor is a constant and is the minimum value:
> https://github.com/openjdk/jdk/blob/c19b12927d2ac901ec8ccaa2de5897ee4c47af56/src/hotspot/share/opto/divnode.cpp#L1146-L1147
> 
> The zero case does not cause problems (this seems to be because it would hide behind a `div0_check` anyway).
> 
> The fix is simply to only create the temporary node when it is needed, and thus avoiding returning without destroying it.
> 
> ### Testing
> - [x] [GitHub Actions](TODO)
> - [x] tier1-4, plus some internal testing
> 
> Thank you for reviewing!

@benoitmaillard Thanks for fixing this! Fix looks good, I just have a few nits below ;)

test/hotspot/jtreg/compiler/c2/TestModIdealCreatesUselessNode.java line 24:

> 22:  */
> 23: 
> 24: package compiler.c2;

Could we find some more specific igvn directory?

test/hotspot/jtreg/compiler/c2/TestModIdealCreatesUselessNode.java line 39:

> 37:  *      -XX:VerifyIterativeGVN=1110
> 38:  *      compiler.c2.TestModIdealCreatesUselessNode
> 39:  * @run main compiler.c2.TestModIdealCreatesUselessNode

Suggestion:

 * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
 *      -Xcomp -XX:-TieredCompilation
 *      -XX:CompileCommand=compileonly,${test.main.class}::test*
 *      -XX:VerifyIterativeGVN=1110
 *      ${test.main.class}
 * @run main ${test.main.class}

You would have to verify if the template works also in the `CompileCommand`, but I think it should.

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28488#pullrequestreview-3562528839
PR Review Comment: https://git.openjdk.org/jdk/pull/28488#discussion_r2606650901
PR Review Comment: https://git.openjdk.org/jdk/pull/28488#discussion_r2606646816