RFR: 8361702: C2: assert(is_dominator(compute_early_ctrl(limit, limit_ctrl), pre_end)) failed: node pinned on loop exit test?

[Christian Hagedorn]

On Tue, 22 Jul 2025 08:25:08 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> A node in a pre loop only has uses out of the loop dominated by the
> loop exit. `PhaseIdealLoop::try_sink_out_of_loop()` sets its control
> to the loop exit projection. A range check in the main loop has this
> node as input (through a chain of some other nodes). Range check
> elimination needs to update the exit condition of the pre loop with an
> expression that depends on the node pinned on its exit: that's
> impossible and the assert fires. This is a variant of 8314024 (this
> one was for a node with uses out of the pre loop on multiple paths). I
> propose the same fix: leave the node with control in the pre loop in
> this case.

Some small comments, otherwise, looks good! I launched some testing.

Somehow forgot to submit my review from 2 days ago...

src/hotspot/share/opto/loopopts.cpp line 1929:

> 1927:   // Sinking a node from a pre loop to its main loop pins the node between the pre and main loops. If that node is input
> 1928:   // to a check that's eliminated by range check elimination, it becomes input to an expression that feeds into the exit
> 1929:   // test of the pre loop above the point in the graph where it's pinned.

I suggest to move it up as a method comment:
Suggestion:

 // Sinking a node from a pre loop to its main loop pins the node between the pre and main loops. If that node is input
// to a check that's eliminated by range check elimination, it becomes input to an expression that feeds into the exit
// test of the pre loop above the point in the graph where it's pinned.
bool PhaseIdealLoop::would_sink_below_pre_loop_exit(IdealLoopTree* n_loop, Node* ctrl) {

test/hotspot/jtreg/compiler/rangechecks/TestSunkRangeFromPreLoopRCE2.java line 30:

> 28:   *
> 29:  * @run main/othervm -XX:CompileCommand=compileonly,*TestSunkRangeFromPreLoopRCE2*::* -Xbatch TestSunkRangeFromPreLoopRCE2
> 30:  *

Suggestion:

test/hotspot/jtreg/compiler/rangechecks/TestSunkRangeFromPreLoopRCE2.java line 63:

> 61:         }
> 62:     }
> 63: 

Suggestion:

test/hotspot/jtreg/compiler/rangechecks/TestSunkRangeFromPreLoopRCE3.java line 31:

> 29:  *
> 30:  * @run main/othervm -XX:-BackgroundCompilation -XX:LoopUnrollLimit=100 -XX:-UseLoopPredicate -XX:-UseProfiledLoopPredicate TestSunkRangeFromPreLoopRCE3
> 31:  *

Suggestion:

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26424#pullrequestreview-3042110108
PR Review Comment: https://git.openjdk.org/jdk/pull/26424#discussion_r2221904998
PR Review Comment: https://git.openjdk.org/jdk/pull/26424#discussion_r2221907804
PR Review Comment: https://git.openjdk.org/jdk/pull/26424#discussion_r2221901405
PR Review Comment: https://git.openjdk.org/jdk/pull/26424#discussion_r2221909155