RFR: 8314024: SIGSEGV in PhaseIdealLoop::build_loop_late_post_work due to bad immediate dominator info

[Christian Hagedorn]

On Wed, 23 Aug 2023 09:15:38 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> A node is sunk from the pre loop into the main loop. That node, in the
> main loop, feeds into a test. When the node is sunk it is pinned
> between the main and pre loop. The test it feeds into is then
> eliminated by range check elimination: the sunk node becomes input to
> an expression that computes the new bound of the pre loop. The
> resulting graph is broken because the sunk node is pinned below the
> pre loop but used by the exit test of the pre loop.
> 
> The fix I propose is in `PhaseIdealLoop::try_sink_out_of_loop()`, to
> skip nodes in pre loops that have a use in the companion main loop.

Looks good but I'm wondering if we could also bail out in Range Check Elimination instead, if we find that `get_ctrl()` of one of the involved data nodes does not dominate the pre loop exit test. What do you think?

test/hotspot/jtreg/compiler/loopopts/TestNodeSunkFromPreLoop.java line 28:

> 26:  * @bug 8314024
> 27:  * @summary Node used in check in main loop sunk from pre loop before RC elimination
> 28:  * @run main/othervm -XX:-BackgroundCompilation -XX:-UseLoopPredicate TestNodeSunkFromPreLoop

You should add a `@requires vm.compiler2.enabled` since `UseLoopPredicate` is a C2 only flag.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15399#pullrequestreview-1593102702
PR Review Comment: https://git.openjdk.org/jdk/pull/15399#discussion_r1304014502