RFR: 8348572: C2 compilation asserts due to unexpected irreducible loop [v2]

Tobias Hartmann thartmann at openjdk.org
Wed Feb 5 06:26:16 UTC 2025


On Fri, 31 Jan 2025 06:11:41 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> A quick summary:
>> - In [JDK-8280126](https://bugs.openjdk.org/browse/JDK-8280126), we decided that we are only going to allow irreducible loops that were detected at parsing, and we can thus restrict optimizations to reducible loops which would be difficult to do correct with irreducible loops. That's why we added that assert that checks that no new irreducible loop shows up during compilation.
>> - Problem: we use `split_if` for `IfNode::Ideal_common` to split through a Region that is loop-head, and the splitting of the Region introduces a second loop entry -> irreducible loop.
>> 
>> Before `split_if`:
>> ![image](https://github.com/user-attachments/assets/01bc78fa-7fed-4a8f-b6f4-078dac9b5dc4)
>> 
>> After `split_if`:
>> ![image](https://github.com/user-attachments/assets/1e3bd08e-b76d-4e7f-813e-27a5a22cb2bd)
>> 
>> 
>> - We have the `split_if` for `IfNode::Ideal_common` to do split-if on straight-line code. But we currently execute this before loop-opts, and so we don't know if the region we split through is actually a loop head. We guard against LoopNode, but a Region only becomes a LoopNode in loop-opts.
>> - We also have split-if in loop-opts, which is more careful about splitting through loop-heads.
>> - Just removing the straight-line split-if probably leads to a regression, as the loop-opts version only executes if there are loops for example.
>> - We could consider delaying the straight-line split-if until after loop-opts. But I don't know if that could lead to regressions in any way.
>> 
>> I discussed this temporary solution with @TobiHartmann :
>> - We would like [JDK-8348570](https://bugs.openjdk.org/browse/JDK-8348570) to be unblocked for @shipilev .
>> - Convert the assert into a bailout-check, so we are sure we behave correctly in product. Compiling with irreducible loops behaves correctly in almost all cases, but there could be exceptions.
>> - For now, have the assert behind a Verify flag, so that [JDK-8348570](https://bugs.openjdk.org/browse/JDK-8348570) is unblocked. Later, we can remove the Verify flag and alway enable the assert again.
>> - This fix also looks easier to backport.
>> 
>> -----------------------
>> 
>> The attached regression test now does **NOT** fail by default, but rather silently bails out of compilation.
>> 
>> With the new debug flag `-XX:+VerifyNoNewIrreducibleLoops`, we still hit the assert, as expected:
>> 
>> #  Internal Error (/oracle-work/jdk-fork0/open/src/hotspot/share/opto/loopnode.cpp:5636), pid=3698055, tid=3698072
>> #  asser...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   update for Vladimir

Nice analysis! Thanks for quickly jumping on this, Emanuel! Looks good to me.

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

> 5646:         if (!head->can_be_irreducible_entry()) {
> 5647:           assert(!VerifyNoNewIrreducibleLoops, "A new irreducible loop was created after parsing.");
> 5648:           C->record_method_not_compilable("A new irreducible loop was created after parsing.");

If you haven't done that yet, I would suggest to hardcode these bailouts to "always bail" out and run testing to check if the bailout always works. You'll of course get all kinds of test failures but the VM should not crash/assert (you can filter for these in the test results and ignore anything else).

test/hotspot/jtreg/compiler/loopopts/TestSplitIfNewIrreducibleLoop.java line 47:

> 45: 
> 46:     public static void main(String[] args) {
> 47:         // Instanciate one each: classes are loaded.

Suggestion:

        // Instantiate one each: classes are loaded.

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23363#pullrequestreview-2594724717
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1942305325
PR Review Comment: https://git.openjdk.org/jdk/pull/23363#discussion_r1942302878


More information about the hotspot-compiler-dev mailing list