8340454: C2 EA asserts with "previous reducible Phi is no longer reducible before SUT"

Reviewed-by: thartmann, kvn

src/hotspot/share/opto/escape.cpp

@@ -317,7 +317,7 @@ bool ConnectionGraph::compute_escape() {
 
   // Propagate NSR (Not Scalar Replaceable) state.
   if (found_nsr_alloc) {
-    find_scalar_replaceable_allocs(jobj_worklist);
+    find_scalar_replaceable_allocs(jobj_worklist, reducible_merges);
   }
 
   // alloc_worklist will be processed in reverse push order.
@@ -3051,8 +3051,43 @@ bool ConnectionGraph::has_non_reducible_merge(FieldNode* field, Unique_Node_List
   return false;
 }
 
+void ConnectionGraph::revisit_reducible_phi_status(JavaObjectNode* jobj, Unique_Node_List& reducible_merges) {
+  assert(jobj != nullptr && !jobj->scalar_replaceable(), "jobj should be set as NSR before calling this function.");
+
+  // Look for 'phis' that refer to 'jobj' as the last
+  // remaining scalar replaceable input.
+  uint reducible_merges_cnt = reducible_merges.size();
+  for (uint i = 0; i < reducible_merges_cnt; i++) {
+    Node* phi = reducible_merges.at(i);
+
+    // This 'Phi' will be a 'good' if it still points to
+    // at least one scalar replaceable object. Note that 'obj'
+    // was/should be marked as NSR before calling this function.
+    bool good_phi = false;
+
+    for (uint j = 1; j < phi->req(); j++) {
+      JavaObjectNode* phi_in_obj = unique_java_object(phi->in(j));
+      if (phi_in_obj != nullptr && phi_in_obj->scalar_replaceable()) {
+        good_phi = true;
+        break;
+      }
+    }
+
+    if (!good_phi) {
+      NOT_PRODUCT(if (TraceReduceAllocationMerges) tty->print_cr("Phi %d became non-reducible after node %d became NSR.", phi->_idx, jobj->ideal_node()->_idx);)
+      reducible_merges.remove(i);
+
+      // Decrement the index because the 'remove' call above actually
+      // moves the last entry of the list to position 'i'.
+      i--;
+
+      reducible_merges_cnt--;
+    }
+  }
+}
+
 // Propagate NSR (Not scalar replaceable) state.
-void ConnectionGraph::find_scalar_replaceable_allocs(GrowableArray<JavaObjectNode*>& jobj_worklist) {
+void ConnectionGraph::find_scalar_replaceable_allocs(GrowableArray<JavaObjectNode*>& jobj_worklist, Unique_Node_List &reducible_merges) {
   int jobj_length = jobj_worklist.length();
   bool found_nsr_alloc = true;
   while (found_nsr_alloc) {
@@ -3071,6 +3106,10 @@ void ConnectionGraph::find_scalar_replaceable_allocs(GrowableArray<JavaObjectNod
             // it is stored has NSR base.
             if ((base != null_obj) && !base->scalar_replaceable()) {
               set_not_scalar_replaceable(jobj NOT_PRODUCT(COMMA "is stored into field with NSR base"));
+              // Any merge that had only 'jobj' as scalar-replaceable will now be non-reducible,
+              // because there is no point in reducing a Phi that won't improve the number of SR
+              // objects.
+              revisit_reducible_phi_status(jobj, reducible_merges);
               found_nsr_alloc = true;
               break;
             }

src/hotspot/share/opto/escape.hpp

@@ -472,8 +472,11 @@ private:
   // Adjust scalar_replaceable state after Connection Graph is built.
   void adjust_scalar_replaceable_state(JavaObjectNode* jobj, Unique_Node_List &reducible_merges);
 
+  // Reevaluate Phis reducible status after 'obj' became NSR.
+  void revisit_reducible_phi_status(JavaObjectNode* jobj, Unique_Node_List& reducible_merges);
+
   // Propagate NSR (Not scalar replaceable) state.
-  void find_scalar_replaceable_allocs(GrowableArray<JavaObjectNode*>& jobj_worklist);
+  void find_scalar_replaceable_allocs(GrowableArray<JavaObjectNode*>& jobj_worklist, Unique_Node_List &reducible_merges);
 
   // Optimize ideal graph.
   void optimize_ideal_graph(GrowableArray<Node*>& ptr_cmp_worklist,

test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndNonReduciblePhi.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. 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 8340454
+ * @summary Check that Reduce Allocation Merges doesn't crash when
+ *          a reducible Phi becomes irreducible after the last of
+ *          its SR inputs is flagged as NSR.
+ * @run main/othervm -XX:CompileCommand=dontinline,*TestReduceAllocationAndNonReduciblePhi*::test
+ *                   -XX:CompileCommand=compileonly,*TestReduceAllocationAndNonReduciblePhi*::test
+ *                   -XX:CompileCommand=compileonly,*Picture*::*
+ *                   -XX:CompileCommand=compileonly,*Point*::*
+ *                   -XX:CompileCommand=inline,*Picture*::*
+ *                   -XX:CompileCommand=inline,*Point*::*
+ *                   -XX:CompileCommand=exclude,*::dummy*
+ *                   -Xbatch
+ *                   -server
+ *                   compiler.escapeAnalysis.TestReduceAllocationAndNonReduciblePhi
+ *
+ * @run main compiler.escapeAnalysis.TestReduceAllocationAndNonReduciblePhi
+ */
+
+package compiler.escapeAnalysis;
+
+public class TestReduceAllocationAndNonReduciblePhi {
+    public static void main(String args[]) {
+        int result = 0;
+
+        for (int i = 0; i < 20000; i++) {
+            result += test(i % 2 == 0, i % 3);
+        }
+
+        System.out.println("Result is = " + result);
+    }
+
+    public static int test(boolean flag1, int pos) {
+        Point p0 = new Point();
+        Point p1 = flag1 ? null : p0;
+
+        Picture pic = new Picture();
+        pic.p = p0;
+
+        Picture[] ps = new Picture[5];
+        ps[pos] = pic;
+
+        return p1 != null ? dummy1() : dummy2();
+    }
+
+    public static int dummy1() { return 1; }
+
+    public static int dummy2() { return 2; }
+
+    private static class Picture {
+        public Point p;
+    }
+
+    private static class Point { }
+}