From 7ef283129388413b362942fb45af48d1f7393b67 Mon Sep 17 00:00:00 2001
From: Christian Hagedorn <chagedorn@openjdk.org>
Date: Thu, 6 Jun 2024 06:58:05 +0000
Subject: [PATCH] 8333644: C2: assert(is_Bool()) failed: invalid node class:
 Phi

Reviewed-by: thartmann, kvn
---
 src/hotspot/share/opto/loopopts.cpp           | 17 ++++----
 ...aqueInitializedAssertionPredicateNode.java | 42 +++++++++++++++++++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/src/hotspot/share/opto/loopopts.cpp b/src/hotspot/share/opto/loopopts.cpp
index 200adc34494..a3227d47832 100644
--- a/src/hotspot/share/opto/loopopts.cpp
+++ b/src/hotspot/share/opto/loopopts.cpp
@@ -2154,18 +2154,19 @@ void PhaseIdealLoop::clone_loop_handle_data_uses(Node* old, Node_List &old_new,
 #endif
     if (!loop->is_member(use_loop) && !outer_loop->is_member(use_loop) && (!old->is_CFG() || !use->is_CFG())) {
 
-      // If the Data use is an IF, that means we have an IF outside of the
-      // loop that is switching on a condition that is set inside of the
+      // If the Data use is an IF, that means we have an IF outside the
+      // loop that is switching on a condition that is set inside the
       // loop.  Happens if people set a loop-exit flag; then test the flag
-      // in the loop to break the loop, then test is again outside of the
+      // in the loop to break the loop, then test is again outside the
       // loop to determine which way the loop exited.
-      // Loop predicate If node connects to Bool node through Opaque1 node.
       //
-      // If the use is an AllocateArray through its ValidLengthTest input,
-      // make sure the Bool/Cmp input is cloned down to avoid a Phi between
-      // the AllocateArray node and its ValidLengthTest input that could cause
+      // For several uses we need to make sure that there is no phi between,
+      // the use and the Bool/Cmp. We therefore clone the Bool/Cmp down here
+      // to avoid such a phi in between.
+      // For example, it is unexpected that there is a Phi between an
+      // AllocateArray node and its ValidLengthTest input that could cause
       // split if to break.
-      if (use->is_If() || use->is_CMove() || use->is_Opaque4() ||
+      if (use->is_If() || use->is_CMove() || use->is_Opaque4() || use->is_OpaqueInitializedAssertionPredicate() ||
           (use->Opcode() == Op_AllocateArray && use->in(AllocateNode::ValidLengthTest) == old)) {
         // Since this code is highly unlikely, we lazily build the worklist
         // of such Nodes to go split.
diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java
index 8fb9e693eb6..5860fcb45b6 100644
--- a/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java
+++ b/test/hotspot/jtreg/compiler/predicates/assertion/TestOpaqueInitializedAssertionPredicateNode.java
@@ -54,6 +54,17 @@
  * @run main compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode
  */
 
+/*
+ * @test id=clone_loop_handle_data_uses
+ * @bug 8333644
+ * @modules java.base/jdk.internal.misc:+open
+ * @summary Test that using OpaqueInitializedAssertionPredicate for Initialized Assertion Predicates instead of Opaque4
+ *          nodes also works with clone_loop_handle_data_uses() which missed a case before.
+ * @run main/othervm -Xcomp -XX:CompileCommand=compileonly,*TestOpaqueInitializedAssertionPredicateNode::test*
+ *                   -XX:CompileCommand=dontinline,*TestOpaqueInitializedAssertionPredicateNode::dontInline
+ *                   compiler.predicates.assertion.TestOpaqueInitializedAssertionPredicateNode
+ */
+
 package compiler.predicates.assertion;
 
 import jdk.internal.misc.Unsafe;
@@ -63,6 +74,7 @@ public class TestOpaqueInitializedAssertionPredicateNode {
 
     static boolean flag, flag2;
     static int iFld;
+    static long lFld;
     static int x;
     static int y = 51;
     static int iArrLength;
@@ -95,6 +107,7 @@ public class TestOpaqueInitializedAssertionPredicateNode {
             testPolicyRangeCheck(a);
             testUnsafeAccess(a);
             testOpaqueOutsideLoop();
+            testOpaqueOutsideLoop8333644();
             testOpaqueInsideIfOutsideLoop();
         }
     }
@@ -350,6 +363,35 @@ public class TestOpaqueInitializedAssertionPredicateNode {
         }
     }
 
+    // Same as testOpaqueOutsideLoop() but we crash later when generating the Mach graph due to wrongly having an If
+    // with a Phi input instead of: If <- Bool <- CmpU <- [x, Phi]. Found by fuzzing.
+    static void testOpaqueOutsideLoop8333644() {
+        int a = 3, b = 7;
+        boolean bArr[] = new boolean[1];
+        for (int i = 1; i < 122; i++) {
+            float f = 1.729F;
+            while (++a < 7) {
+                iArr[a] *= lFld;
+                switch (i) {
+                    case 26:
+                        for (; b < 1; ) {}
+                    case 27:
+                        iArr[1] = 9;
+                    case 28:
+                        break;
+                    case 33:
+                        iArr[1] = a;
+                        break;
+                    case 35:
+                        lFld = b;
+                        break;
+                    default:
+                        ;
+                }
+            }
+        }
+    }
+
     // Similar to testOpaqueOutside loop but Opaque is now also inside loop.
     // [If]->OpaqueInitializedAssertionPredicate->Bool->Cmp, []: Inside Loop, other nodes outside.
     static void testOpaqueInsideIfOutsideLoop() {