JDK-8348572 : C2 compilation asserts due to unexpected irreducible loop
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 21,22,23,24,25
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2025-01-24
  • Updated: 2025-02-10
  • Resolved: 2025-02-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 25
25 b09Fixed
Related Reports
Blocks :  
JDK-8348570 - CTW: Expose the code hidden by uncommon traps
Relates :  
JDK-8280126 - C2: detect and remove dead irreducible loops
Description
A quick summary:
- 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.
- 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 a possible temporary solution with [~thartmann]:
- We would like JDK-8348570 to be unblocked for [~shade].
- 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 is unblocked. Later, we can remove the Verify flag and alway enable the assert again.

-------------------- Original Description ----------------------

(synopsis is provisional, change as you see fit)

I have been testing various compilation modes in the context of JDK-8348570, and noticed this failure:

$ CONF=linux-x86_64-server-fastdebug make images test TEST=applications/ctw/modules/java_desktop_2.java TEST_VM_OPTS="-XX:PerMethodTrapLimit=0"

java.lang.Error: modules_java_desktop_2685: failed during compilation of class #2885 : javax/swing/plaf/basic/BasicScrollBarUI
	at sun.hotspot.tools.ctw.CtwRunner.startCtwforAllClasses(CtwRunner.java:231)
	at sun.hotspot.tools.ctw.CtwRunner.run(CtwRunner.java:129)
	at sun.hotspot.tools.ctw.CtwRunner.main(CtwRunner.java:75)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:333)
	at java.base/java.lang.Thread.run(Thread.java:1447)

# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/shade/trunks/jdk/src/hotspot/share/opto/cfgnode.cpp:445), pid=2894539, tid=2894609
#  assert(loop_status() == RegionNode::LoopStatus::MaybeIrreducibleEntry) failed: must be marked irreducible
#

Current CompileTask:
C2:7425 7243    b  4       javax.swing.plaf.basic.BasicScrollBarUI$ScrollListener::actionPerformed (351 bytes)

Stack: [0x000077ddab66b000,0x000077ddab76b000],  sp=0x000077ddab765c80,  free space=1003k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x9326a4]  RegionNode::verify_can_be_irreducible_entry() const+0x54  (cfgnode.cpp:445)
V  [libjvm.so+0x1387cd0]  PhaseIdealLoop::build_loop_tree_impl(Node*, int)+0x190  (loopnode.cpp:5635)
V  [libjvm.so+0x1397817]  PhaseIdealLoop::build_loop_tree()+0x507  (loopnode.cpp:5509)
V  [libjvm.so+0x139b7bb]  PhaseIdealLoop::build_and_optimize()+0x16b  (loopnode.cpp:4663)
V  [libjvm.so+0xa9b1dc]  PhaseIdealLoop::optimize(PhaseIterGVN&, LoopOptsMode)+0x3ac  (loopnode.hpp:1114)
V  [libjvm.so+0xa93ffc]  Compile::Optimize()+0x53c  (compile.cpp:2379)
V  [libjvm.so+0xa98d53]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x1bc3  (compile.cpp:852)
V  [libjvm.so+0x8e2047]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x1b7  (c2compiler.cpp:142)
V  [libjvm.so+0xaa5b20]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0xb60  (compileBroker.cpp:2319)
V  [libjvm.so+0xaa6978]  CompileBroker::compiler_thread_loop()+0x5c8  (compileBroker.cpp:1977)
V  [libjvm.so+0xf8d1de]  JavaThread::thread_main_inner()+0xee  (javaThread.cpp:777)
V  [libjvm.so+0x1aa47be]  Thread::call_run()+0xbe  (thread.cpp:232)
V  [libjvm.so+0x15bdd1b]  thread_native_entry(Thread*)+0x12b  (os_linux.cpp:860)
Registers:
Comments
Changeset: 19399d27 Branch: master Author: Emanuel Peter <epeter@openjdk.org> Date: 2025-02-05 12:58:43 +0000 URL: https://git.openjdk.org/jdk/commit/19399d271ef00f925232fbbe9087b5772f2fca01
05-02-2025
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/23363 Date: 2025-01-30 10:03:49 +0000
30-01-2025
A quick summary: - 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. - 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 a possible temporary solution with [~thartmann]: - We would like JDK-8348570 to be unblocked for [~shade]. - 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 is unblocked. Later, we can remove the Verify flag and alway enable the assert again.
30-01-2025
ILW = Same as JDK-8280126 = P3
28-01-2025
If split_if introduces second loop entries (irreducible loops), then we may delay split_if until loop opts, where we could know if we have a Loop head or just a regular Region.
27-01-2025
I played with it a little more, and have some more explanation now. However, I'm not sure what we should do about it. There are bailouts to the split_if like this: if (!r->is_Region() || r->is_Loop() || phi_region != r || r->as_Region()->is_copy()) { return nullptr } We do not split_if through a Loop. But at this point, we have not yet recognized the Region as a Loop head. We have no dominator information at this point, so how should we know that the Region is a loop head, and that splitting the if may cause additional loop entries, i.e. an irreducible loop? public class Test { static class A { public A parent; } static class B extends A {} public static void main(String[] args) { // Instanciate one each: classes are loaded. A a = new A(); B b = new B(); test(b); } static int test(A parent) { do { if (parent instanceof B b) { return 1; } if (parent != null) { parent = parent.parent; } if (parent == null) { return 0; } } while (true); } // Reproducing it like this: // // java -XX:PerMethodTrapLimit=0 -Xbatch -XX:+UseG1GC -Xcomp -XX:CompileCommand=printcompilation,Test::* -XX:CompileCommand=compileonly,Test::test Test.java // // Before split_if it looks like this (the instanceof check has already been partial peeled): // // if (parent instanceof B b) { return 1; } // do { // if (parent != null) { parent = parent.parent; } // if (parent == null) { return 0; } // if (parent instanceof B b) { return 1; } // } while (true); // // // Now, we want to split_if the first if in the loop body, like this: // // if (parent instanceof B b) { return 1; } // if (parent != null) { goto LOOP2; } else { goto LOOP1; } // do { // :LOOP1 // parent = parent.parent; // :LOOP2 // if (parent == null) { return 0; } // if (parent instanceof B b) { return 1; } // if (parent != null) { goto LOOP2; } else { goto LOOP1; } // } while (true); // // As the comment in ifnode.cpp / split_if says: we know that on the backedge // "parent" cannot be null, and so we would be able to split the if through // the region, and on the backedge it would constant fold away, and we would // only have to check the split if in the loop entry. // // Problem: we have introduced an irreducible loop! }
27-01-2025
Assert is from JDK-8280126.
27-01-2025
Reproduces from JDK21 (assert added in JDK-8280126) - JDK25 (current).
27-01-2025
Oh, I have a simple reproducer now: /oracle-work/jdk-fork0/build/linux-x64-debug/jdk/bin/java -XX:PerMethodTrapLimit=0 -Xbatch -XX:+UseG1GC -Xcomp -XX:CompileCommand=printcompilation,Test::* -XX:CompileCommand=compileonly,Test::test Test.java
27-01-2025
358 If -> nullcheck in else block: if (parent != null) { parent = parent.getParent(); } 149 If -> loop exit check (null-check): } while (parent != null); 368 If -> instanceof / subtype check: if (parent instanceof JFrame par) { --------------------------------------------- Ok, so the loop got a little rotated. Let's now to reproduce it in a separate Java file.
27-01-2025
Looks like we are folding around the null-checks in this code / loop: public void actionPerformed(ActionEvent e) { // If frame is disabled and timer is started in mousePressed // and mouseReleased is not called, then timer will not be stopped // Stop the timer if frame is disabled Component parent = scrollbar.getParent(); do { if (parent instanceof JFrame par) { if (!par.isEnabled()) { ((Timer)e.getSource()).stop(); buttonListener.handledEvent = false; scrollbar.setValueIsAdjusting(false); return; } break; } else { if (parent != null) { parent = parent.getParent(); } } } while (parent != null); If I'm not mistaken, the thing is that on the backedge, we already know that "parent != null", and so the null-check at the beginning of the loop-body could be split through the region. And that gives us 2 entries into the loop... yikes.
27-01-2025
Looking into it. Looks like we are creating a new irreducible loop by splitting an if through a region. See img_001.png and img_002.png. Back in JDK-8280126 we had decided to disable the new creation of irreducible loops. So we'll have to see what if we want to just disable this optimization here, and how to do that. Maybe the rewiring of Phi nodes is just happening the wrong way around... let's see.
27-01-2025