8298176: remove OpaqueZeroTripGuardPostLoop once main-loop disappears

Reviewed-by: thartmann, chagedorn, kvn

Author: rwestrel

src/hotspot/share/opto/loopTransform.cpp

@@ -1705,7 +1705,7 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
   // pre-loop, the main-loop may not execute at all.  Later in life this
   // zero-trip guard will become the minimum-trip guard when we unroll
   // the main-loop.
-  Node *min_opaq = new OpaqueZeroTripGuardNode(C, limit);
+  Node *min_opaq = new OpaqueZeroTripGuardNode(C, limit, b_test);
   Node *min_cmp  = new CmpINode(pre_incr, min_opaq);
   Node *min_bol  = new BoolNode(min_cmp, b_test);
   register_new_node(min_opaq, new_pre_exit);
@@ -1994,7 +1994,7 @@ Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new,
   // (the previous loop trip-counter exit value) because we will be changing
   // the exit value (via additional unrolling) so we cannot constant-fold away the zero
   // trip guard until all unrolling is done.
-  Node *zer_opaq = new OpaqueZeroTripGuardNode(C, incr);
+  Node *zer_opaq = new OpaqueZeroTripGuardNode(C, incr, main_end->test_trip());
   Node *zer_cmp = new CmpINode(zer_opaq, limit);
   Node *zer_bol = new BoolNode(zer_cmp, main_end->test_trip());
   register_new_node(zer_opaq, new_main_exit);

src/hotspot/share/opto/opaquenode.hpp

@@ -27,6 +27,7 @@
 
 #include "opto/node.hpp"
 #include "opto/opcodes.hpp"
+#include "subnode.hpp"
 
 //------------------------------Opaque1Node------------------------------------
 // A node to prevent unwanted optimizations.  Allows constant folding.
@@ -72,9 +73,16 @@ class OpaqueLoopStrideNode : public Opaque1Node {
 
 class OpaqueZeroTripGuardNode : public Opaque1Node {
 public:
-  OpaqueZeroTripGuardNode(Compile* C, Node *n) : Opaque1Node(C, n) {
+  // This captures the test that returns true when the loop is entered. It depends on whether the loop goes up or down.
+  // This is used by CmpINode::Value.
+  BoolTest::mask _loop_entered_mask;
+  OpaqueZeroTripGuardNode(Compile* C, Node* n, BoolTest::mask loop_entered_test) :
+          Opaque1Node(C, n), _loop_entered_mask(loop_entered_test) {
   }
   virtual int Opcode() const;
+  virtual uint size_of() const {
+    return sizeof(*this);
+  }
 };
 
 //------------------------------Opaque3Node------------------------------------

src/hotspot/share/opto/phaseX.cpp

@@ -1678,6 +1678,13 @@ void PhaseIterGVN::add_users_to_worklist( Node *n ) {
         }
       }
     }
+    if (use->Opcode() == Op_OpaqueZeroTripGuard) {
+      assert(use->outcnt() <= 1, "OpaqueZeroTripGuard can't be shared");
+      if (use->outcnt() == 1) {
+        Node* cmp = use->unique_out();
+        _worklist.push(cmp);
+      }
+    }
   }
 }
 
@@ -1848,6 +1855,7 @@ void PhaseCCP::push_more_uses(Unique_Node_List& worklist, Node* parent, const No
   push_loadp(worklist, use);
   push_and(worklist, parent, use);
   push_cast_ii(worklist, parent, use);
+  push_opaque_zero_trip_guard(worklist, use);
 }
 
 
@@ -1968,6 +1976,12 @@ void PhaseCCP::push_cast_ii(Unique_Node_List& worklist, const Node* parent, cons
   }
 }
 
+void PhaseCCP::push_opaque_zero_trip_guard(Unique_Node_List& worklist, const Node* use) const {
+  if (use->Opcode() == Op_OpaqueZeroTripGuard) {
+    push_if_not_bottom_type(worklist, use->unique_out());
+  }
+}
+
 //------------------------------do_transform-----------------------------------
 // Top level driver for the recursive transformer
 void PhaseCCP::do_transform() {

src/hotspot/share/opto/phaseX.hpp

@@ -596,6 +596,7 @@ class PhaseCCP : public PhaseIterGVN {
   static void push_load_barrier(Unique_Node_List& worklist, const BarrierSetC2* barrier_set, const Node* use);
   void push_and(Unique_Node_List& worklist, const Node* parent, const Node* use) const;
   void push_cast_ii(Unique_Node_List& worklist, const Node* parent, const Node* use) const;
+  void push_opaque_zero_trip_guard(Unique_Node_List& worklist, const Node* use) const;
 
  public:
   PhaseCCP( PhaseIterGVN *igvn ); // Compute conditional constants

src/hotspot/share/opto/subnode.cpp

@@ -34,6 +34,7 @@
 #include "opto/matcher.hpp"
 #include "opto/movenode.hpp"
 #include "opto/mulnode.hpp"
+#include "opto/opaquenode.hpp"
 #include "opto/opcodes.hpp"
 #include "opto/phaseX.hpp"
 #include "opto/subnode.hpp"
@@ -661,6 +662,47 @@ const Type *CmpINode::sub( const Type *t1, const Type *t2 ) const {
   return TypeInt::CC;           // else use worst case results
 }
 
+const Type* CmpINode::Value(PhaseGVN* phase) const {
+  Node* in1 = in(1);
+  Node* in2 = in(2);
+  // If this test is the zero trip guard for a main or post loop, check whether, with the opaque node removed, the test
+  // would constant fold so the loop is never entered. If so return the type of the test without the opaque node removed:
+  // make the loop unreachable.
+  // The reason for this is that the iv phi captures the bounds of the loop and if the loop becomes unreachable, it can
+  // become top. In that case, the loop must be removed.
+  // This is safe because:
+  // - as optimizations proceed, the range of iterations executed by the main loop narrows. If no iterations remain, then
+  // we're done with optimizations for that loop.
+  // - the post loop is initially not reachable but as long as there's a main loop, the zero trip guard for the post
+  // loop takes a phi that merges the pre and main loop's iv and can't constant fold the zero trip guard. Once, the main
+  // loop is removed, there's no need to preserve the zero trip guard for the post loop anymore.
+  if (in1 != NULL && in2 != NULL) {
+    uint input = 0;
+    Node* cmp = NULL;
+    BoolTest::mask test;
+    if (in1->Opcode() == Op_OpaqueZeroTripGuard && phase->type(in1) != Type::TOP) {
+      cmp = new CmpINode(in1->in(1), in2);
+      test = ((OpaqueZeroTripGuardNode*)in1)->_loop_entered_mask;
+    }
+    if (in2->Opcode() == Op_OpaqueZeroTripGuard && phase->type(in2) != Type::TOP) {
+      assert(cmp == NULL, "A cmp with 2 OpaqueZeroTripGuard inputs");
+      cmp = new CmpINode(in1, in2->in(1));
+      test = ((OpaqueZeroTripGuardNode*)in2)->_loop_entered_mask;
+    }
+    if (cmp != NULL) {
+      const Type* cmp_t = cmp->Value(phase);
+      const Type* t = BoolTest(test).cc2logical(cmp_t);
+      cmp->destruct(phase);
+      if (t == TypeInt::ZERO) {
+        return cmp_t;
+      }
+    }
+  }
+
+  return SubNode::Value(phase);
+}
+
+
 // Simplify a CmpU (compare 2 integers) node, based on local information.
 // If both inputs are constants, compare them.
 const Type *CmpUNode::sub( const Type *t1, const Type *t2 ) const {

src/hotspot/share/opto/subnode.hpp

@@ -153,6 +153,7 @@ class CmpINode : public CmpNode {
   virtual int Opcode() const;
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
   virtual const Type *sub( const Type *, const Type * ) const;
+  virtual const Type* Value(PhaseGVN* phase) const;
 };
 
 //------------------------------CmpUNode---------------------------------------

test/hotspot/jtreg/compiler/loopopts/TestOpaqueZeroTripGuardPostLoopRemoval.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2022, 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 8298176
+ * @summary Must remove OpaqueZeroTripGuardPostLoop after main loop disappears else
+ *          the zero-trip-guard of the post loop cannot die and leaves an inconsistent
+ *          graph behind.
+ * @run main/othervm -Xcomp -XX:-TieredCompilation
+ *      -XX:CompileCommand=compileonly,TestOpaqueZeroTripGuardPostLoopRemoval::test*
+ *      -XX:CompileCommand=dontinline,TestOpaqueZeroTripGuardPostLoopRemoval::*
+ *      TestOpaqueZeroTripGuardPostLoopRemoval
+ */
+
+public class TestOpaqueZeroTripGuardPostLoopRemoval {
+    static long x;
+
+    public static void main(String[] strArr) {
+        test_001();
+        test_002();
+        try {
+            test_003();
+        } catch (Exception e) {
+            // Expected
+        }
+        test_004();
+        test_005();
+    }
+
+    static void test_001() {
+        int b = 6;
+        for (long l = 1; l < 9; l++) {
+            b++;
+        }
+        for (int i = 1; i < 1000; i*=2) {
+            for (int j = 1; j < 2; j++) {
+                x = b + 1;
+            }
+        }
+    }
+
+    static void test_002() {
+        int b = 6;
+        for (long l = 60; l < 3000; l+=3) {
+            // bounds of loop: no work for post loop
+            b += 33; // any multiple of iv step
+        }
+        for (int i = 1; i < 1000; i*=2) {
+            for (int j = 1; j < 2; j++) {
+                x = b + 1;
+            }
+        }
+    }
+
+    static void dontInline() {
+        throw new RuntimeException();
+    }
+
+    static int test_003() {
+        int y = 3;
+        for (int i = 0; i < 9; ) {
+            for (long l = 1; l < 5; l++) {
+                y *= 2;
+            }
+            while (true) {
+                dontInline();
+            }
+        }
+        return y;
+    }
+
+    static void test_004() {
+        for (int i2 = 4; i2 < 13; i2++) {
+            double d = 56;
+            for (long l = 1; l < 5; l++) {
+                d = d + 3;
+            }
+            for (int i = 0; i < 10; i++) {
+                for (int d2 = i2; d2 < 2; d2 = 3) {
+                }
+            }
+        }
+    }
+
+    public static int test_005() {
+        long arr[]=new long[400];
+        for (int i = 3; i < 177; i++) {
+            for (int j = 0; j < 10; j++){}
+        }
+        int y = 0;
+        for (int i = 15; i < 356; i++) {
+            // Inner loop prevents strip-mining of outer loop
+            // later, inner loop is removed, so outer does pre-main-post without strip-mining
+            for (int j = 0; j < 10; j++){
+                y |= 1;
+            }
+        }
+        return y;
+    }
+}
+