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

[Christian Hagedorn]

On Tue, 29 Aug 2023 08:28:50 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.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test fix

> > 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?
> 
> We could, but it seems unfortunate to bail out of a major optimization when it's fairly straightforward to avoid it. I added asserts in RCE to catch that issue earlier instead.

I agree with that. Thanks for adding additional asserts to catch such cases earlier. Looks good!

I'll run some more testing with the new asserts in place.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15399#pullrequestreview-1599848081