8269752: C2: assert(false) failed: Bad graph detected in build_loop_late

rwestrel · rwestrel · commit c1304519b56e · 2021-07-19T07:31:28.000Z
Reviewed-by: chagedorn, kvn
diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp
@@ -606,6 +606,15 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
           igvn->replace_input_of(outer, LoopNode::LoopBackControl, igvn->C->top());
         }
       }
+      if (is_CountedLoop()) {
+        Node* opaq = as_CountedLoop()->is_canonical_loop_entry();
+        if (opaq != NULL) {
+          // This is not a loop anymore. No need to keep the Opaque1 node on the test that guards the loop as it won't be
+          // subject to further loop opts.
+          assert(opaq->Opcode() == Op_Opaque1, "");
+          igvn->replace_node(opaq, opaq->in(1));
+        }
+      }
       Node *parent_ctrl;
       if( cnt == 0 ) {
         assert( req() == 1, "no inputs expected" );
diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp
@@ -1990,10 +1990,10 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
     // Check the shape of the graph at the loop entry. If an inappropriate
     // graph shape is encountered, the compiler bails out loop unrolling;
     // compilation of the method will still succeed.
-    if (!is_canonical_loop_entry(loop_head)) {
+    opaq = loop_head->is_canonical_loop_entry();
+    if (opaq == NULL) {
       return;
     }
-    opaq = loop_head->skip_predicates()->in(0)->in(1)->in(1)->in(2);
     // Zero-trip test uses an 'opaque' node which is not shared.
     assert(opaq->outcnt() == 1 && opaq->in(1) == limit, "");
   }
@@ -2608,7 +2608,7 @@ int PhaseIdealLoop::do_range_check(IdealLoopTree *loop, Node_List &old_new) {
   // Check graph shape. Cannot optimize a loop if zero-trip
   // Opaque1 node is optimized away and then another round
   // of loop opts attempted.
-  if (!is_canonical_loop_entry(cl)) {
+  if (cl->is_canonical_loop_entry() == NULL) {
     return closed_range_checks;
   }
 
@@ -2937,7 +2937,9 @@ bool PhaseIdealLoop::multi_version_post_loops(IdealLoopTree *rce_loop, IdealLoop
   }
 
   // Find RCE'd post loop so that we can stage its guard.
-  if (!is_canonical_loop_entry(legacy_cl)) return multi_version_succeeded;
+  if (legacy_cl->is_canonical_loop_entry() == NULL) {
+    return multi_version_succeeded;
+  }
   Node* ctrl = legacy_cl->in(LoopNode::EntryControl);
   Node* iffm = ctrl->in(0);
 
diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp
@@ -2139,9 +2139,10 @@ SafePointNode* CountedLoopNode::outer_safepoint() const {
 }
 
 Node* CountedLoopNode::skip_predicates_from_entry(Node* ctrl) {
-    while (ctrl != NULL && ctrl->is_Proj() && ctrl->in(0)->is_If() &&
-           ctrl->in(0)->as_If()->proj_out(1-ctrl->as_Proj()->_con)->outcnt() == 1 &&
-           ctrl->in(0)->as_If()->proj_out(1-ctrl->as_Proj()->_con)->unique_out()->Opcode() == Op_Halt) {
+    while (ctrl != NULL && ctrl->is_Proj() && ctrl->in(0) != NULL && ctrl->in(0)->is_If() &&
+            (ctrl->in(0)->as_If()->proj_out_or_null(1-ctrl->as_Proj()->_con) == NULL ||
+             (ctrl->in(0)->as_If()->proj_out(1-ctrl->as_Proj()->_con)->outcnt() == 1 &&
+              ctrl->in(0)->as_If()->proj_out(1-ctrl->as_Proj()->_con)->unique_out()->Opcode() == Op_Halt))) {
       ctrl = ctrl->in(0)->in(0);
     }
 
@@ -4920,28 +4921,34 @@ Node* PhaseIdealLoop::compute_lca_of_uses(Node* n, Node* early, bool verify) {
 // loop unswitching, and IGVN, or a combination of them) can freely change
 // the graph's shape. As a result, the graph shape outlined below cannot
 // be guaranteed anymore.
-bool PhaseIdealLoop::is_canonical_loop_entry(CountedLoopNode* cl) {
-  if (!cl->is_main_loop() && !cl->is_post_loop()) {
-    return false;
+Node* CountedLoopNode::is_canonical_loop_entry() {
+  if (!is_main_loop() && !is_post_loop()) {
+    return NULL;
   }
-  Node* ctrl = cl->skip_predicates();
+  Node* ctrl = skip_predicates();
 
   if (ctrl == NULL || (!ctrl->is_IfTrue() && !ctrl->is_IfFalse())) {
-    return false;
+    return NULL;
   }
   Node* iffm = ctrl->in(0);
   if (iffm == NULL || !iffm->is_If()) {
-    return false;
+    return NULL;
   }
   Node* bolzm = iffm->in(1);
   if (bolzm == NULL || !bolzm->is_Bool()) {
-    return false;
+    return NULL;
   }
   Node* cmpzm = bolzm->in(1);
   if (cmpzm == NULL || !cmpzm->is_Cmp()) {
-    return false;
+    return NULL;
   }
-  // compares can get conditionally flipped
+
+  uint input = is_main_loop() ? 2 : 1;
+  if (input >= cmpzm->req() || cmpzm->in(input) == NULL) {
+    return NULL;
+  }
+  bool res = cmpzm->in(input)->Opcode() == Op_Opaque1;
+#ifdef ASSERT
   bool found_opaque = false;
   for (uint i = 1; i < cmpzm->req(); i++) {
     Node* opnd = cmpzm->in(i);
@@ -4950,10 +4957,9 @@ bool PhaseIdealLoop::is_canonical_loop_entry(CountedLoopNode* cl) {
       break;
     }
   }
-  if (!found_opaque) {
-    return false;
-  }
-  return true;
+  assert(found_opaque == res, "wrong pattern");
+#endif
+  return res ? cmpzm->in(input) : NULL;
 }
 
 //------------------------------get_late_ctrl----------------------------------
diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp
@@ -350,6 +350,8 @@ class CountedLoopNode : public BaseCountedLoopNode {
     return T_INT;
   }
 
+  Node* is_canonical_loop_entry();
+
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif
@@ -931,8 +933,6 @@ class PhaseIdealLoop : public PhaseTransform {
 
   PhaseIterGVN &igvn() const { return _igvn; }
 
-  static bool is_canonical_loop_entry(CountedLoopNode* cl);
-
   bool has_node( Node* n ) const {
     guarantee(n != NULL, "No Node.");
     return _nodes[n->_idx] != NULL;
diff --git a/src/hotspot/share/opto/multnode.cpp b/src/hotspot/share/opto/multnode.cpp
@@ -46,7 +46,6 @@ Node *MultiNode::match( const ProjNode *proj, const Matcher *m ) { return proj->
 // Get a named projection or null if not found
 ProjNode* MultiNode::proj_out_or_null(uint which_proj) const {
   assert((Opcode() != Op_If && Opcode() != Op_RangeCheck) || which_proj == (uint)true || which_proj == (uint)false, "must be 1 or 0");
-  assert((Opcode() != Op_If && Opcode() != Op_RangeCheck) || outcnt() == 2, "bad if #1");
   for( DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++ ) {
     Node *p = fast_out(i);
     if (p->is_Proj()) {
@@ -75,6 +74,7 @@ ProjNode* MultiNode::proj_out_or_null(uint which_proj, bool is_io_use) const {
 
 // Get a named projection
 ProjNode* MultiNode::proj_out(uint which_proj) const {
+  assert((Opcode() != Op_If && Opcode() != Op_RangeCheck) || outcnt() == 2, "bad if #1");
   ProjNode* p = proj_out_or_null(which_proj);
   assert(p != NULL, "named projection %u not found", which_proj);
   return p;
diff --git a/src/hotspot/share/opto/superword.cpp b/src/hotspot/share/opto/superword.cpp
@@ -3604,7 +3604,7 @@ void SuperWord::align_initial_loop_index(MemNode* align_to_ref) {
 CountedLoopEndNode* SuperWord::find_pre_loop_end(CountedLoopNode* cl) const {
   // The loop cannot be optimized if the graph shape at
   // the loop entry is inappropriate.
-  if (!PhaseIdealLoop::is_canonical_loop_entry(cl)) {
+  if (cl->is_canonical_loop_entry() == NULL) {
     return NULL;
   }
 
diff --git a/test/hotspot/jtreg/compiler/loopopts/TestMainBodyExecutedOnce.java b/test/hotspot/jtreg/compiler/loopopts/TestMainBodyExecutedOnce.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2021, 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 8269752
+ * @summary C2: assert(false) failed: Bad graph detected in build_loop_late
+ *
+ * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileOnly=TestMainBodyExecutedOnce TestMainBodyExecutedOnce
+ *
+ */
+
+
+public class TestMainBodyExecutedOnce {
+    static int N;
+    static long vMeth_check_sum;
+
+    public static void main(String[] strArr) {
+        TestMainBodyExecutedOnce _instance = new TestMainBodyExecutedOnce();
+        for (int i = 0; i < 10; i++) {
+            _instance.test();
+        }
+    }
+
+    void test() {
+        vMeth(3);
+    }
+
+    void vMeth(int i2) {
+        double d = 1.74287;
+        int i3 = -36665, i4, iArr[] = new int[N];
+        short s;
+        long lArr[] = new long[N];
+        while (++i3 < 132) {
+            if (i2 != 0) {
+                vMeth_check_sum += i3;
+                return;
+            }
+            i4 = 1;
+            while (++i4 < 12) {
+                i2 += i4;
+            }
+        }
+        vMeth_check_sum += i3;
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -3604,7 +3604,7 @@ void SuperWord::align_initial_loop_index(MemNode* align_to_ref) {`
`3604`	`3604`	`CountedLoopEndNode* SuperWord::find_pre_loop_end(CountedLoopNode* cl) const {`
`3605`	`3605`	`// The loop cannot be optimized if the graph shape at`
`3606`	`3606`	`// the loop entry is inappropriate.`
`3607`		`- if (!PhaseIdealLoop::is_canonical_loop_entry(cl)) {`
	`3607`	`+ if (cl->is_canonical_loop_entry() == NULL) {`
`3608`	`3608`	`return NULL;`
`3609`	`3609`	`}`
`3610`	`3610`