RFR: 8311087: PhiNode::wait_for_region_igvn should break early

[Christian Hagedorn]

On Thu, 29 Jun 2023 10:44:45 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi, please consider this PR.
> 
> Instead of continuing the loop we break after setting `delay = true`. I also flattened out the indentation by transforming `A && B` into `!A || !B` and `continue`ing if so. This makes the code a bit longer, but clearer imho.
> 
> I've run `TestCastIIAfterUnrollingInOuterLoop`, which is the test that was added along with this method. I'm also running tier1.
> 
> Thanks,
> Johan

Marked as reviewed by chagedorn (Reviewer).

src/hotspot/share/opto/cfgnode.cpp line 1944:

> 1942:     Node* n = in(j);
> 1943: 
> 1944:     if (rc == nullptr || !rc->is_Proj()) continue;

Maybe you could put braces around the `continue` statements.

src/hotspot/share/opto/cfgnode.cpp line 1966:

> 1964:       delay = true;
> 1965:       break;
> 1966:     }

Just an idea, how about putting this into a separate method `should_delay()` (or something like that) and replacing `continue` with `return false` and `break` with `return true`? If `should_delay()` is true at some point, we can push `this` to the worklist and return true.

But looks good either way.

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

PR Review: https://git.openjdk.org/jdk/pull/14707#pullrequestreview-1505271452
PR Review Comment: https://git.openjdk.org/jdk/pull/14707#discussion_r1247486085
PR Review Comment: https://git.openjdk.org/jdk/pull/14707#discussion_r1246605085