RFR: 8294217: Assertion failure: parsing found no loops but there are some

[Christian Hagedorn]

On Fri, 28 Oct 2022 14:34:42 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> This was reported on 11 and is not reproducible with the current
> jdk. The reason is that the PhaseIdealLoop invocation before EA was
> changed from LoopOptsNone to LoopOptsMaxUnroll. In the absence of
> loops, LoopOptsMaxUnroll exits earlier than LoopOptsNone. That wasn't
> intended and this patch makes sure they behave the same. Once that's
> changed, the crash reproduces with the current jdk.
> 
> The assert fires because PhaseIdealLoop::only_has_infinite_loops()
> returns false even though the IR only has infinite loops. There's a
> single loop nest and the inner most loop is an infinite loop. The
> current logic only looks at loops that are direct children of the root
> of the loop tree. It's not the first bug where
> PhaseIdealLoop::only_has_infinite_loops() fails to catch an infinite
> loop (8257574 was the previous one) and it's proving challenging to
> have PhaseIdealLoop::only_has_infinite_loops() handle corner cases
> robustly. I reworked PhaseIdealLoop::only_has_infinite_loops() once
> more. This time it goes over all children of the root of the loop
> tree, collects all controls for the loop and its inner loop. It then
> checks whether any control is a branch out of the loop and if it is
> whether it's not a NeverBranch.

That looks reasonable to me.

src/hotspot/share/opto/loopnode.cpp line 4183:

> 4181: 
> 4182: #ifdef ASSERT
> 4183: bool PhaseIdealLoop::only_has_infinite_loops() {

> This time it goes over all children of the root of the loop
tree, collects all controls for the loop and its inner loop. It then
checks whether any control is a branch out of the loop and if it is
whether it's not a NeverBranch.

Maybe you can add this summary as a comment here.

src/hotspot/share/opto/loopnode.cpp line 4209:

> 4207:     for (uint i = 0; i < wq.size(); ++i) {
> 4208:       Node* c = wq.at(i);
> 4209:       if (c->isa_MultiBranch()) {

Can be changed to `is_MultiBranch()` as you do not need the casted type.

test/hotspot/jtreg/compiler/loopopts/TestInfiniteLoopNest.java line 55:

> 53:     public static void main(String[] p) throws Exception {
> 54:         Thread thread = new Thread() {
> 55:                 public void run() {

Indentation is wrong here.

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

Marked as reviewed by chagedorn (Reviewer).

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