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

[Roland Westrelin]

> 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.

Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains nine additional commits since the last revision:

 - remove useless PhaseIdealLoop::only_has_infinite_loops()
 - Merge branch 'master' into JDK-8338100
 - Update src/hotspot/share/opto/loopnode.hpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/loopnode.cpp
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - Update src/hotspot/share/opto/loopnode.cpp
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - comment
 - test fix
 - remove verification code
 - test & fix

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/20797/files
  - new: https://git.openjdk.org/jdk/pull/20797/files/ceb241a8..a1bfc79e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=20797&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20797&range=01-02

  Stats: 11045 lines in 484 files changed: 6572 ins; 1805 del; 2668 mod
  Patch: https://git.openjdk.org/jdk/pull/20797.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20797/head:pull/20797

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