Integrated: 8338100: C2: assert(!n_loop->is_member(get_loop(lca))) failed: control must not be back in the loop

[Roland Westrelin]

On Fri, 30 Aug 2024 15:58:30 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> The crash occurs because a `Store` is sunk out of a loop that's an
> inner loop of an infinite loop. The infinite loop was just found to be
> infinite in the current round of loop opts. When that happens the
> infinite loop is not properly attached to the rest of the loop tree. As
> a consequence, the `IdealLoopTree` instance for the infinite loop and
> its children are only partially initialized (`_nest` is not set) and
> the structure is an inconsistent state.
> 
> When the `Store` is sunk it's reported as belonging to a loop but the
> `IdealLoopTree` for that loop is only half populated. As a consequence
> a call to `is_dominator` for that loop hits an inconsistency, returns
> an incorrect result and the assert fires.
> 
> A possible fix would be a point fix that skips that optimization for a
> loop that's part of an infinite loop nest. But given basic methods of
> loop opts can't be trusted to work in the infinite loop nest, I
> suppose similar issues can surface elsewhere.
> 
> It's not the first time, we have issues with an infinite loop that's
> not properly attached to the loop tree the first time it is
> encountered (a NeverBranch is then added and on the next loop passes,
> the infinite loop is properly attached to the loop tree). For instance
> on a loop opts round, C2 can see that it has no loops and on the next
> that it has some.
> 
> I propose fixing this by properly attaching the infinite loop to the
> loop tree when it's first discovered. A comment in the code seems to
> hint that it requires going over the graph again after the
> `NeverBranch` is added but I don't think that's case.
> 
> I changed the assert in `loopnode.cpp` because it was there to work
> around the inconsistency I mentioned above (no loop in a round, some
> loops on the next one).
> 
> The change in `parse1.cpp` fixes an issue I ran into when testing the
> fix. The existing logic doesn't properly detect an exception backedge.
> 
> I added the test case from 8336478 to this. The problem there is that
> an infinite loop contains a long counted loop. The long counted loop
> is transformed into a loop nest which is a 2 step process that
> requires 2 rounds of loop opts. But c2 finds an infinite loop in the
> middle of the process which causes it to see no more loops and to not
> attempt another round of loop opts. The assert fires because it finds
> a long counted loop nest that's half transformed. The change I propose
> here fixes this too. If we go with this fix, I'll close 8336478 as
> duplicate of this one.

This pull request has now been integrated.

Changeset: e203df46
Author:    Roland Westrelin <roland at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/e203df46faf610e35e2c2510271ad68199f4fa3f
Stats:     308 lines in 7 files changed: 257 ins; 32 del; 19 mod

8338100: C2: assert(!n_loop->is_member(get_loop(lca))) failed: control must not be back in the loop

Co-authored-by: Emanuel Peter <epeter at openjdk.org>
Reviewed-by: chagedorn, thartmann

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

PR: https://git.openjdk.org/jdk/pull/20797