8287284: C2: loop optimization performs split_thru_phi infinitely many times

Reviewed-by: thartmann, epeter, chagedorn, roland

src/hotspot/share/opto/loopnode.hpp

@@ -1735,6 +1735,8 @@ public:
   Node* similar_subtype_check(const Node* x, Node* r_in);
 
   void update_addp_chain_base(Node* x, Node* old_base, Node* new_base);
+
+  bool can_move_to_inner_loop(Node* n, LoopNode* n_loop, Node* x);
 };
 
 

src/hotspot/share/opto/loopopts.cpp

@@ -152,9 +152,22 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) {
         }
       }
     }
-    if (x != the_clone && the_clone != nullptr)
-      _igvn.remove_dead_node(the_clone);
+
     phi->set_req( i, x );
+
+    if (the_clone == nullptr) {
+      continue;
+    }
+
+    if (the_clone != x) {
+      _igvn.remove_dead_node(the_clone);
+    } else if (region->is_Loop() && i == LoopNode::LoopBackControl &&
+               n->is_Load() && can_move_to_inner_loop(n, region->as_Loop(), x)) {
+      // it is not a win if 'x' moved from an outer to an inner loop
+      // this edge case can only happen for Load nodes
+      wins = 0;
+      break;
+    }
   }
   // Too few wins?
   if (wins <= policy) {
@@ -218,6 +231,16 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) {
   return phi;
 }
 
+// Test whether node 'x' can move into an inner loop relative to node 'n'.
+// Note: The test is not exact. Returns true if 'x' COULD end up in an inner loop,
+// BUT it can also return true and 'x' is in the outer loop
+bool PhaseIdealLoop::can_move_to_inner_loop(Node* n, LoopNode* n_loop, Node* x) {
+  IdealLoopTree* n_loop_tree = get_loop(n_loop);
+  IdealLoopTree* x_loop_tree = get_loop(get_early_ctrl(x));
+  // x_loop_tree should be outer or same loop as n_loop_tree
+  return !x_loop_tree->is_member(n_loop_tree);
+}
+
 // Subtype checks that carry profile data don't common so look for a replacement by following edges
 Node* PhaseIdealLoop::similar_subtype_check(const Node* x, Node* r_in) {
   if (x->is_SubTypeCheck()) {

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

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2023, 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 8287284
+ * @summary The phi of cnt is split from the inner to the outer loop,
+ *          and then from outer loop to the inner loop again.
+ *          This ended in a endless optimization cycle.
+ * @library /test/lib /
+ * @run driver compiler.c2.loopopts.TestSplitThruPhiInfinitely
+ */
+
+package compiler.c2.loopopts;
+
+import compiler.lib.ir_framework.*;
+
+public class TestSplitThruPhiInfinitely {
+
+    public static int cnt = 1;
+
+    @Test
+    @IR(counts = {IRNode.PHI, " <= 10"})
+    public static void test() {
+        int j = 0;
+        do {
+            j = cnt;
+            for (int k = 0; k < 20000; k++) {
+                cnt += 2;
+            }
+        } while (++j < 10);
+    }
+
+    public static void main(String[] args) {
+        TestFramework.runWithFlags("-XX:-PartialPeelLoop");
+    }
+}