8297724: Loop strip mining prevents some empty loops from being eliminated

Reviewed-by: kvn, thartmann

src/hotspot/share/opto/loopTransform.cpp

@@ -3587,19 +3587,27 @@ void IdealLoopTree::remove_main_post_loops(CountedLoopNode *cl, PhaseIdealLoop *
 // counter with the value it will have on the last iteration.  This will break
 // the loop.
 bool IdealLoopTree::do_remove_empty_loop(PhaseIdealLoop *phase) {
-  // Minimum size must be empty loop
-  if (_body.size() > EMPTY_LOOP_SIZE) {
-    return false;
-  }
   if (!_head->is_CountedLoop()) {
     return false;   // Dead loop
   }
-  CountedLoopNode *cl = _head->as_CountedLoop();
-  if (!cl->is_valid_counted_loop(T_INT)) {
-    return false;   // Malformed loop
+  if (!empty_loop_candidate(phase)) {
+    return false;
   }
-  if (!phase->is_member(this, phase->get_ctrl(cl->loopexit()->in(CountedLoopEndNode::TestValue)))) {
-    return false;   // Infinite loop
+  CountedLoopNode *cl = _head->as_CountedLoop();
+#ifdef ASSERT
+  // Call collect_loop_core_nodes to exercise the assert that checks that it finds the right number of nodes
+  if (empty_loop_with_extra_nodes_candidate(phase)) {
+    Unique_Node_List wq;
+    collect_loop_core_nodes(phase, wq);
+  }
+#endif
+  // Minimum size must be empty loop
+  if (_body.size() > EMPTY_LOOP_SIZE) {
+    // This loop has more nodes than an empty loop but, maybe they are only kept alive by the outer strip mined loop's
+    // safepoint. If they go away once the safepoint is removed, that loop is empty.
+    if (!empty_loop_with_data_nodes(phase)) {
+      return false;
+    }
   }
   if (cl->is_pre_loop()) {
     // If the loop we are removing is a pre-loop then the main and post loop
@@ -3697,6 +3705,137 @@ bool IdealLoopTree::do_remove_empty_loop(PhaseIdealLoop *phase) {
   return true;
 }
 
+bool IdealLoopTree::empty_loop_candidate(PhaseIdealLoop* phase) const {
+  CountedLoopNode *cl = _head->as_CountedLoop();
+  if (!cl->is_valid_counted_loop(T_INT)) {
+    return false;   // Malformed loop
+  }
+  if (!phase->is_member(this, phase->get_ctrl(cl->loopexit()->in(CountedLoopEndNode::TestValue)))) {
+    return false;   // Infinite loop
+  }
+  return true;
+}
+
+bool IdealLoopTree::empty_loop_with_data_nodes(PhaseIdealLoop* phase) const {
+  CountedLoopNode* cl = _head->as_CountedLoop();
+  if (!cl->is_strip_mined() || !empty_loop_with_extra_nodes_candidate(phase)) {
+    return false;
+  }
+  Unique_Node_List empty_loop_nodes;
+  Unique_Node_List wq;
+
+  // Start from all data nodes in the loop body that are not one of the EMPTY_LOOP_SIZE nodes expected in an empty body
+  enqueue_data_nodes(phase, empty_loop_nodes, wq);
+  // and now follow uses
+  for (uint i = 0; i < wq.size(); ++i) {
+    Node* n = wq.at(i);
+    for (DUIterator_Fast jmax, j = n->fast_outs(jmax); j < jmax; j++) {
+      Node* u = n->fast_out(j);
+      if (u->Opcode() == Op_SafePoint) {
+        // found a safepoint. Maybe this loop's safepoint or another loop safepoint.
+        if (!process_safepoint(phase, empty_loop_nodes, wq, u)) {
+          return false;
+        }
+      } else {
+        const Type* u_t = phase->_igvn.type(u);
+        if (u_t == Type::CONTROL || u_t == Type::MEMORY || u_t == Type::ABIO) {
+          // found a side effect
+          return false;
+        }
+        wq.push(u);
+      }
+    }
+  }
+  // Nodes (ignoring the EMPTY_LOOP_SIZE nodes of the "core" of the loop) are kept alive by otherwise empty loops'
+  // safepoints: kill them.
+  for (uint i = 0; i < wq.size(); ++i) {
+    Node* n = wq.at(i);
+    phase->_igvn.replace_node(n, phase->C->top());
+  }
+
+#ifdef ASSERT
+  for (uint i = 0; i < _body.size(); ++i) {
+    Node* n = _body.at(i);
+    assert(wq.member(n) || empty_loop_nodes.member(n), "missed a node in the body?");
+  }
+#endif
+
+  return true;
+}
+
+bool IdealLoopTree::process_safepoint(PhaseIdealLoop* phase, Unique_Node_List& empty_loop_nodes, Unique_Node_List& wq,
+                                      Node* sfpt) const {
+  CountedLoopNode* cl = _head->as_CountedLoop();
+  if (cl->outer_safepoint() == sfpt) {
+    // the current loop's safepoint
+    return true;
+  }
+
+  // Some other loop's safepoint. Maybe that loop is empty too.
+  IdealLoopTree* sfpt_loop = phase->get_loop(sfpt);
+  if (!sfpt_loop->_head->is_OuterStripMinedLoop()) {
+    return false;
+  }
+  IdealLoopTree* sfpt_inner_loop = sfpt_loop->_child;
+  CountedLoopNode* sfpt_cl = sfpt_inner_loop->_head->as_CountedLoop();
+  assert(sfpt_cl->is_strip_mined(), "inconsistent");
+
+  if (empty_loop_nodes.member(sfpt_cl)) {
+    // already taken care of
+    return true;
+  }
+
+  if (!sfpt_inner_loop->empty_loop_candidate(phase) || !sfpt_inner_loop->empty_loop_with_extra_nodes_candidate(phase)) {
+    return false;
+  }
+
+  // Enqueue the nodes of that loop for processing too
+  sfpt_inner_loop->enqueue_data_nodes(phase, empty_loop_nodes, wq);
+  return true;
+}
+
+bool IdealLoopTree::empty_loop_with_extra_nodes_candidate(PhaseIdealLoop* phase) const {
+  CountedLoopNode *cl = _head->as_CountedLoop();
+  // No other control flow node in the loop body
+  if (cl->loopexit()->in(0) != cl) {
+    return false;
+  }
+
+  if (phase->is_member(this, phase->get_ctrl(cl->limit()))) {
+    return false;
+  }
+  return true;
+}
+
+void IdealLoopTree::enqueue_data_nodes(PhaseIdealLoop* phase, Unique_Node_List& empty_loop_nodes,
+                                       Unique_Node_List& wq) const {
+  collect_loop_core_nodes(phase, empty_loop_nodes);
+  for (uint i = 0; i < _body.size(); ++i) {
+    Node* n = _body.at(i);
+    if (!empty_loop_nodes.member(n)) {
+      wq.push(n);
+    }
+  }
+}
+
+// This collects the node that would be left if this body was empty
+void IdealLoopTree::collect_loop_core_nodes(PhaseIdealLoop* phase, Unique_Node_List& wq) const {
+  uint before = wq.size();
+  wq.push(_head->in(LoopNode::LoopBackControl));
+  for (uint i = 0; i < wq.size(); ++i) {
+    Node* n = wq.at(i);
+    for (uint j = 0; j < n->req(); ++j) {
+      Node* in = n->in(j);
+      if (in != NULL) {
+        if (phase->get_loop(phase->ctrl_or_self(in)) == this) {
+          wq.push(in);
+        }
+      }
+    }
+  }
+  assert(wq.size() - before == EMPTY_LOOP_SIZE, "expect the EMPTY_LOOP_SIZE nodes of this body if empty");
+}
+
 //------------------------------do_one_iteration_loop--------------------------
 // Convert one iteration loop into normal code.
 bool IdealLoopTree::do_one_iteration_loop(PhaseIdealLoop *phase) {

src/hotspot/share/opto/loopnode.hpp

@@ -797,6 +797,19 @@ public:
   bool is_residual_iters_large(int unroll_cnt, CountedLoopNode *cl) const {
     return (unroll_cnt - 1) * (100.0 / LoopPercentProfileLimit) > cl->profile_trip_cnt();
   }
+
+  void collect_loop_core_nodes(PhaseIdealLoop* phase, Unique_Node_List& wq) const;
+
+  bool empty_loop_with_data_nodes(PhaseIdealLoop* phase) const;
+
+  void enqueue_data_nodes(PhaseIdealLoop* phase, Unique_Node_List& empty_loop_nodes, Unique_Node_List& wq) const;
+
+  bool process_safepoint(PhaseIdealLoop* phase, Unique_Node_List& empty_loop_nodes, Unique_Node_List& wq,
+                         Node* sfpt) const;
+
+  bool empty_loop_candidate(PhaseIdealLoop* phase) const;
+
+  bool empty_loop_with_extra_nodes_candidate(PhaseIdealLoop* phase) const;
 };
 
 // -----------------------------PhaseIdealLoop---------------------------------

test/hotspot/jtreg/compiler/c2/irTests/TestLSMMissedEmptyLoop.java

@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2022, 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.
+ */
+
+package compiler.c2.irTests;
+
+import compiler.lib.ir_framework.*;
+
+/*
+ * @test
+ * @bug 8297724
+ * @library /test/lib /
+ * @requires vm.compiler2.enabled
+ * @run driver compiler.c2.irTests.TestLSMMissedEmptyLoop
+ */
+
+public class TestLSMMissedEmptyLoop {
+    public static void main(String[] args) {
+        TestFramework.runWithFlags("-XX:LoopMaxUnroll=0");
+        TestFramework.runWithFlags("-XX:-UseCountedLoopSafepoints", "-XX:LoopMaxUnroll=0");
+        TestFramework.run();
+        TestFramework.runWithFlags("-XX:-UseCountedLoopSafepoints");
+    }
+
+    static double doubleField;
+
+    @ForceInline
+    public static void testHelper(int i, double d) {
+        if (i != 42) {
+            doubleField = d;
+        }
+    }
+
+    @Test
+    @IR(failOn = {IRNode.COUNTED_LOOP, IRNode.LOOP })
+    public static void test1() {
+        double d = 1;
+        for (int i = 0; i < 1000; i++) {
+            d = d * 2;
+        }
+        int i = 0;
+        for (i = 0; i < 42; i++) {
+        }
+        testHelper(i, d);
+    }
+
+    @Run(test = "test1")
+    private void test1_runner() {
+        testHelper(-42, 42);
+        test1();
+    }
+
+    @Test
+    @IR(applyIf = { "LoopStripMiningIter", "0" }, failOn = {IRNode.COUNTED_LOOP, IRNode.LOOP })
+    @IR(applyIf = { "LoopStripMiningIter", "> 0" }, counts = {IRNode.COUNTED_LOOP, ">= 2" })
+    public static void test2() {
+        double d = 1;
+        for (int j = 0; j < 10; j++) {
+            for (int i = 0; i < 1000; i++) {
+                d = d * 2;
+            }
+        }
+        int i = 0;
+        for (i = 0; i < 42; i++) {
+        }
+        testHelper(i, d);
+    }
+
+    @Run(test = "test2")
+    private void test2_runner() {
+        testHelper(-42, 42);
+        test2();
+    }
+
+    @Test
+    @IR(failOn = {IRNode.COUNTED_LOOP, IRNode.LOOP })
+    public static void test3() {
+        double d = 1;
+        for (int i = 0; i < 1000; i++) {
+            d = d * 2;
+        }
+        for (int i = 0; i < 1000; i++) {
+            d = d * 2;
+        }
+        int i = 0;
+        for (i = 0; i < 42; i++) {
+        }
+        testHelper(i, d);
+    }
+
+    @Run(test = "test3")
+    private void test3_runner() {
+        testHelper(-42, 42);
+        test1();
+    }
+
+    @Test
+    @IR(applyIf = { "LoopStripMiningIter", "0" }, failOn = {IRNode.COUNTED_LOOP, IRNode.LOOP })
+    @IR(applyIf = { "LoopStripMiningIter", "> 0" }, counts = {IRNode.COUNTED_LOOP, ">= 3" })
+    public static void test4() {
+        double d = 1;
+        for (int j = 0; j < 10; j++) {
+            for (int i = 0; i < 1000; i++) {
+                d = d * 2;
+            }
+            for (int i = 0; i < 1000; i++) {
+                d = d * 2;
+            }
+        }
+        int i = 0;
+        for (i = 0; i < 42; i++) {
+        }
+        testHelper(i, d);
+    }
+
+    @Run(test = "test4")
+    private void test4_runner() {
+        testHelper(-42, 42);
+        test4();
+    }
+}