8351889: C2 crash: assertion failed: Base pointers must match (addp 344)

src/hotspot/share/opto/addnode.hpp

@@ -246,6 +246,13 @@ class AddPNode : public Node {
 
   // Do not match base-ptr edge
   virtual uint match_edge(uint idx) const;
+
+#ifdef ASSERT
+  bool address_input_has_same_base() const {
+    Node *addp = in(Address);
+    return !addp->is_AddP() || addp->in(Base)->is_top() || addp->in(Base) == in(Base);
+  }
+#endif
 };
 
 //------------------------------OrINode----------------------------------------

src/hotspot/share/opto/c2_globals.hpp

@@ -697,7 +697,8 @@
           "Print progress during Iterative Global Value Numbering")         \
                                                                             \
   develop(uint, VerifyIterativeGVN, 0,                                      \
-          "Verify Iterative Global Value Numbering =DCBA, with:"            \
+          "Verify Iterative Global Value Numbering =EDCBA, with:"           \
+          "  E: verify node specific invariants"                            \
           "  D: verify Node::Identity did not miss opportunities"           \
           "  C: verify Node::Ideal did not miss opportunities"              \
           "  B: verify that type(n) == n->Value() after IGVN"               \

src/hotspot/share/opto/cfgnode.cpp

@@ -2096,6 +2096,20 @@ bool PhiNode::is_split_through_mergemem_terminating() const {
   return true;
 }
 
+// Is one of the inputs a Cast that has not been processed by igvn yet?
+bool PhiNode::wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const {
+  for (uint i = 1, cnt = req(); i < cnt; ++i) {
+    Node* n = in(i);
+    while (n != nullptr && n->is_ConstraintCast()) {
+      if (igvn->_worklist.member(n)) {
+        return true;
+      }
+      n = n->in(1);
+    }
+  }
+  return false;
+}
+
 //------------------------------Ideal------------------------------------------
 // Return a node which is more "ideal" than the current node.  Must preserve
 // the CFG, but we can still strip out dead paths.
@@ -2154,6 +2168,28 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
       // If there is a chance that the region can be optimized out do
       // not add a cast node that we can't remove yet.
       !wait_for_region_igvn(phase)) {
+    // If one of the inputs is a cast that has yet to be processed by igvn, delay processing of this node to give the
+    // inputs a chance to optimize and possibly end up with identical inputs (casts included).
+    // Say we have:
+    // (Phi region (Cast#1 c uin) (Cast#2 c uin))
+    // and Cast#1 and Cast#2 have not had a chance to common yet
+    // if the unique_input() transformation below proceeds, then PhiNode::Ideal returns:
+    // (Cast#3 region uin) (1)
+    // If PhiNode::Ideal is delayed until Cast#1 and Cast#2 common, then it returns:
+    // (Cast#1 c uin) (2)
+    //
+    // In (1) the resulting cast is conservatively pinned at a later control and while Cast#3 and Cast#1/Cast#2 still
+    // have a chance to common, that requires proving that c dominates region in ConstraintCastNode::dominating_cast()
+    // which may not happen if control flow is too complicated and another pass of loop opts doesn't run. Delaying the
+    // transformation here should allow a more optimal result.
+    // Beyond the efficiency concern, there is a risk, if the casts are CastPPs, to end up with a chain of AddPs with
+    // different base inputs (but a unique uncasted base input). This breaks an invariant in the shape of address
+    // subtrees.
+    PhaseIterGVN* igvn = phase->is_IterGVN();
+    if (wait_for_cast_input_igvn(igvn)) {
+      igvn->_worklist.push(this);
+      return nullptr;
+    }
     uncasted = true;
     uin = unique_input(phase, true);
   }

src/hotspot/share/opto/cfgnode.hpp

@@ -182,6 +182,8 @@ class PhiNode : public TypeNode {
 
   bool is_split_through_mergemem_terminating() const;
 
+  bool wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const;
+
 public:
   // Node layout (parallels RegionNode):
   enum { Region,                // Control input is the Phi's region.

src/hotspot/share/opto/compile.cpp

@@ -3414,10 +3414,7 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
 
   case Op_AddP: {               // Assert sane base pointers
     Node *addp = n->in(AddPNode::Address);
-    assert( !addp->is_AddP() ||
-            addp->in(AddPNode::Base)->is_top() || // Top OK for allocation
-            addp->in(AddPNode::Base) == n->in(AddPNode::Base),
-            "Base pointers must match (addp %u)", addp->_idx );
+    assert(n->as_AddP()->address_input_has_same_base(), "Base pointers must match (addp %u)", addp->_idx );
 #ifdef _LP64
     if ((UseCompressedOops || UseCompressedClassPointers) &&
         addp->Opcode() == Op_ConP &&

src/hotspot/share/opto/phaseX.cpp

@@ -1076,7 +1076,8 @@ void PhaseIterGVN::verify_optimize() {
 
   if (is_verify_Value() ||
       is_verify_Ideal() ||
-      is_verify_Identity()) {
+      is_verify_Identity() ||
+      is_verify_invariants()) {
     ResourceMark rm;
     Unique_Node_List worklist;
     bool failure = false;
@@ -1088,6 +1089,7 @@ void PhaseIterGVN::verify_optimize() {
       if (is_verify_Ideal())    { failure |= verify_Ideal_for(n, false); }
       if (is_verify_Ideal())    { failure |= verify_Ideal_for(n, true); }
       if (is_verify_Identity()) { failure |= verify_Identity_for(n); }
+      if (is_verify_invariants()) { failure |= verify_node_invariants_for(n); }
       // traverse all inputs and outputs
       for (uint i = 0; i < n->req(); i++) {
         if (n->in(i) != nullptr) {
@@ -1102,7 +1104,7 @@ void PhaseIterGVN::verify_optimize() {
     // We should either make sure that these nodes are properly added back to the IGVN worklist
     // in PhaseIterGVN::add_users_to_worklist to update them again or add an exception
     // in the verification code above if that is not possible for some reason (like Load nodes).
-    assert(!failure, "Missed optimization opportunity in PhaseIterGVN");
+    assert(!failure, "Missed optimization opportunity/broken graph in PhaseIterGVN");
   }
 
   verify_empty_worklist(nullptr);
@@ -2058,6 +2060,21 @@ bool PhaseIterGVN::verify_Identity_for(Node* n) {
   tty->print_cr("%s", ss.as_string());
   return true;
 }
+
+// Some other verifications that are not specific to a particular transformation.
+bool PhaseIterGVN::verify_node_invariants_for(const Node* n) {
+  if (n->is_AddP()) {
+    if (!n->as_AddP()->address_input_has_same_base()) {
+      stringStream ss; // Print as a block without tty lock.
+      ss.cr();
+      ss.print_cr("Base pointers must match for AddP chain:");
+      n->dump_bfs(2, nullptr, "", &ss);
+      tty->print_cr("%s", ss.as_string());
+      return true;
+    }
+  }
+  return false;
+}
 #endif
 
 /**

src/hotspot/share/opto/phaseX.hpp

@@ -493,6 +493,7 @@ class PhaseIterGVN : public PhaseGVN {
   bool verify_Value_for(Node* n, bool strict = false);
   bool verify_Ideal_for(Node* n, bool can_reshape);
   bool verify_Identity_for(Node* n);
+  bool verify_node_invariants_for(const Node* n);
   void verify_empty_worklist(Node* n);
 #endif
 
@@ -616,6 +617,10 @@ class PhaseIterGVN : public PhaseGVN {
     // '-XX:VerifyIterativeGVN=1000'
     return ((VerifyIterativeGVN % 10000) / 1000) == 1;
   }
+  static bool is_verify_invariants() {
+    // '-XX:VerifyIterativeGVN=10000'
+    return ((VerifyIterativeGVN % 100000) / 10000) == 1;
+  }
 protected:
   // Sub-quadratic implementation of '-XX:VerifyIterativeGVN=1' (Use-Def verification).
   julong _verify_counter;

src/hotspot/share/runtime/flags/jvmFlagConstraintsCompiler.cpp

@@ -306,7 +306,7 @@ JVMFlag::Error TypeProfileLevelConstraintFunc(uint value, bool verbose) {
 }
 
 JVMFlag::Error VerifyIterativeGVNConstraintFunc(uint value, bool verbose) {
-  const int max_modes = 4;
+  const int max_modes = 5;
   uint original_value = value;
   for (int i = 0; i < max_modes; i++) {
     if (value % 10 > 1) {

test/hotspot/jtreg/compiler/c2/TestMismatchedAddPAfterMaxUnroll.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2025, Red Hat, Inc. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8351889
+ * @summary C2 crash: assertion failed: Base pointers must match (addp 344)
+ * @run main/othervm -XX:-BackgroundCompilation -XX:CompileOnly=TestMismatchedAddPAfterMaxUnroll::test1
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+IgnoreUnrecognizedVMOptions -XX:-UseLoopPredicate
+ *                   -XX:+StressIGVN -XX:StressSeed=383593806 -XX:VerifyIterativeGVN=10000
+ *                   TestMismatchedAddPAfterMaxUnroll
+ * @run main/othervm -XX:-BackgroundCompilation -XX:CompileOnly=TestMismatchedAddPAfterMaxUnroll::test1
+ *                   -XX:+UnlockDiagnosticVMOptions -XX:+IgnoreUnrecognizedVMOptions -XX:-UseLoopPredicate
+ *                   -XX:+StressIGVN  -XX:VerifyIterativeGVN=10000 TestMismatchedAddPAfterMaxUnroll
+ * @run main/othervm TestMismatchedAddPAfterMaxUnroll
+ */
+
+public class TestMismatchedAddPAfterMaxUnroll {
+    private static C[] arrayField = new C[4];
+
+    public static void main(String[] args) {
+        C c = new C();
+        Object lock = new Object();
+        for (int i = 0; i < 20_000; i++) {
+            arrayField[3] = null;
+            test1(3, c, arrayField, true, true, lock);
+            arrayField[3] = null;
+            test1(3, c, arrayField, true, false, lock);
+            arrayField[3] = null;
+            test1(3, c, arrayField, false, false, lock);
+            arrayField[3] = c;
+            test1(3, c, arrayField, false, false, lock);
+        }
+    }
+
+    static class C {
+
+    }
+
+    private static void test1(int j, C c, C[] otherArray, boolean flag, boolean flag2, Object lock) {
+        C[] array = arrayField;
+        int i = 0;
+        for (;;) {
+            synchronized (lock) {}
+            if (array[j] == null) {
+                break;
+            }
+            otherArray[i] = c;
+            i++;
+            if (i >= 3) {
+                return;
+            }
+        }
+        if (flag) {
+            if (flag2) {
+            }
+        }
+        array[j] = c;
+    }
+}

test/hotspot/jtreg/compiler/c2/TestVerifyIterativeGVN.java

@@ -23,11 +23,11 @@
 
 /*
  * @test
- * @bug 8238756
+ * @bug 8238756 8351889
  * @requires vm.debug == true & vm.flavor == "server"
- * @summary Run with -Xcomp to test -XX:VerifyIterativeGVN=1111 in debug builds.
+ * @summary Run with -Xcomp to test -XX:VerifyIterativeGVN=11111 in debug builds.
  *
- * @run main/othervm/timeout=300 -Xcomp -XX:VerifyIterativeGVN=1111 compiler.c2.TestVerifyIterativeGVN
+ * @run main/othervm/timeout=300 -Xcomp -XX:VerifyIterativeGVN=11111 compiler.c2.TestVerifyIterativeGVN
  */
 package compiler.c2;