8330004: Refactor cloning down code in Split If for Template Assertion Predicates

Reviewed-by: epeter, kvn

src/hotspot/share/opto/loopTransform.cpp

@@ -1430,40 +1430,6 @@ void PhaseIdealLoop::copy_assertion_predicates_to_main_loop_helper(const Predica
   }
 }
 
-// Is 'n' a node that can be found on the input chain of a Template Assertion Predicate bool (i.e. between a Template
-// Assertion Predicate If node and the OpaqueLoop* nodes)?
-static bool is_part_of_template_assertion_predicate_bool(Node* n) {
-  int op = n->Opcode();
-  return (n->is_Bool() ||
-          n->is_Cmp() ||
-          op == Op_AndL ||
-          op == Op_OrL ||
-          op == Op_RShiftL ||
-          op == Op_LShiftL ||
-          op == Op_LShiftI ||
-          op == Op_AddL ||
-          op == Op_AddI ||
-          op == Op_MulL ||
-          op == Op_MulI ||
-          op == Op_SubL ||
-          op == Op_SubI ||
-          op == Op_ConvI2L ||
-          op == Op_CastII);
-}
-
-bool PhaseIdealLoop::subgraph_has_opaque(Node* n) {
-  if (n->Opcode() == Op_OpaqueLoopInit || n->Opcode() == Op_OpaqueLoopStride) {
-    return true;
-  }
-  if (!is_part_of_template_assertion_predicate_bool(n)) {
-    return false;
-  }
-  uint init;
-  uint stride;
-  count_opaque_loop_nodes(n, init, stride);
-  return init != 0 || stride != 0;
-}
-
 bool PhaseIdealLoop::assertion_predicate_has_loop_opaque_node(IfNode* iff) {
   uint init;
   uint stride;
@@ -1507,19 +1473,19 @@ void PhaseIdealLoop::count_opaque_loop_nodes(Node* n, uint& init, uint& stride)
   wq.push(n);
   for (uint i = 0; i < wq.size(); i++) {
     Node* n = wq.at(i);
-    if (is_part_of_template_assertion_predicate_bool(n)) {
-      for (uint j = 1; j < n->req(); j++) {
-        Node* m = n->in(j);
-        if (m != nullptr) {
-          wq.push(m);
+    if (TemplateAssertionPredicateExpressionNode::is_maybe_in_expression(n)) {
+      if (n->is_OpaqueLoopInit()) {
+        init++;
+      } else if (n->is_OpaqueLoopStride()) {
+        stride++;
+      } else {
+        for (uint j = 1; j < n->req(); j++) {
+          Node* m = n->in(j);
+          if (m != nullptr) {
+            wq.push(m);
+          }
         }
       }
-      continue;
-    }
-    if (n->Opcode() == Op_OpaqueLoopInit) {
-      init++;
-    } else if (n->Opcode() == Op_OpaqueLoopStride) {
-      stride++;
     }
   }
 }

src/hotspot/share/opto/loopnode.hpp

@@ -951,7 +951,6 @@ private:
                                                  Node* uncommon_proj, Node* control, IdealLoopTree* outer_loop,
                                                  Node* input_proj);
   static void count_opaque_loop_nodes(Node* n, uint& init, uint& stride);
-  static bool subgraph_has_opaque(Node* n);
   static bool assertion_predicate_has_loop_opaque_node(IfNode* iff);
   static void get_assertion_predicates(Node* predicate, Unique_Node_List& list, bool get_opaque = false);
   void update_main_loop_assertion_predicates(Node* ctrl, CountedLoopNode* loop_head, Node* init, int stride_con);
@@ -1759,14 +1758,12 @@ public:
 
   void finish_clone_loop(Node_List* split_if_set, Node_List* split_bool_set, Node_List* split_cex_set);
 
-  bool clone_cmp_down(Node* n, const Node* blk1, const Node* blk2);
-
-  void clone_loadklass_nodes_at_cmp_index(const Node* n, Node* cmp, int i);
-
-  bool clone_cmp_loadklass_down(Node* n, const Node* blk1, const Node* blk2);
-
   bool at_relevant_ctrl(Node* n, const Node* blk1, const Node* blk2);
 
+  bool clone_cmp_loadklass_down(Node* n, const Node* blk1, const Node* blk2);
+  void clone_loadklass_nodes_at_cmp_index(const Node* n, Node* cmp, int i);
+  bool clone_cmp_down(Node* n, const Node* blk1, const Node* blk2);
+  void clone_template_assertion_predicate_expression_down(Node* node);
 
   Node* similar_subtype_check(const Node* x, Node* r_in);
 

src/hotspot/share/opto/node.hpp

@@ -1708,6 +1708,22 @@ public:
     if( !_in_worklist.test_set(b->_idx) )
       Node_List::push(b);
   }
+  void push_non_cfg_inputs_of(const Node* node) {
+    for (uint i = 1; i < node->req(); i++) {
+      Node* input = node->in(i);
+      if (input != nullptr && !input->is_CFG()) {
+        push(input);
+      }
+    }
+  }
+
+  void push_outputs_of(const Node* node) {
+    for (DUIterator_Fast imax, i = node->fast_outs(imax); i < imax; i++) {
+      Node* output = node->fast_out(i);
+      push(output);
+    }
+  }
+
   Node *pop() {
     if( _clock_index >= size() ) _clock_index = 0;
     Node *b = at(_clock_index);

src/hotspot/share/opto/predicates.cpp

@@ -323,7 +323,7 @@ Opaque4Node* TemplateAssertionPredicateExpression::clone(const TransformStrategy
   auto is_opaque_loop_node = [](const Node* node) {
     return node->is_Opaque1();
   };
-  DataNodesOnPathsToTargets data_nodes_on_path_to_targets(TemplateAssertionPredicateExpression::maybe_contains,
+  DataNodesOnPathsToTargets data_nodes_on_path_to_targets(TemplateAssertionPredicateExpressionNode::is_maybe_in_expression,
                                                           is_opaque_loop_node);
   const Unique_Node_List& collected_nodes = data_nodes_on_path_to_targets.collect(_opaque4_node);
   DataNodeGraph data_node_graph(collected_nodes, phase);
@@ -332,3 +332,25 @@ Opaque4Node* TemplateAssertionPredicateExpression::clone(const TransformStrategy
   Node* opaque4_clone = *orig_to_new.get(_opaque4_node);
   return opaque4_clone->as_Opaque4();
 }
+
+// Check if this node belongs a Template Assertion Predicate Expression (including OpaqueLoop* nodes).
+bool TemplateAssertionPredicateExpressionNode::is_in_expression(Node* node) {
+  if (is_maybe_in_expression(node)) {
+    ResourceMark rm;
+    Unique_Node_List list;
+    list.push(node);
+    for (uint i = 0; i < list.size(); i++) {
+      Node* next = list.at(i);
+      if (next->is_OpaqueLoopInit() || next->is_OpaqueLoopStride()) {
+        return true;
+      } else if (is_maybe_in_expression(next)) {
+        list.push_non_cfg_inputs_of(next);
+      }
+    }
+  }
+  return false;
+}
+
+bool TemplateAssertionPredicateExpressionNode::is_template_assertion_predicate(Node* node) {
+  return node->is_If() && node->in(1)->is_Opaque4();
+}

src/hotspot/share/opto/predicates.hpp

@@ -283,14 +283,41 @@ class TemplateAssertionPredicateExpression : public StackObj {
   Opaque4Node* clone(const TransformStrategyForOpaqueLoopNodes& transform_strategy, Node* new_ctrl, PhaseIdealLoop* phase);
 
  public:
-  // Is 'n' a node that could be part of a Template Assertion Predicate Expression (i.e. could be found on the input
-  // chain of a Template Assertion Predicate Opaque4Node up to and including the OpaqueLoop* nodes)?
-  static bool maybe_contains(const Node* n) {
-    const int opcode = n->Opcode();
-    return (opcode == Op_OpaqueLoopInit ||
-            opcode == Op_OpaqueLoopStride ||
-            n->is_Bool() ||
-            n->is_Cmp() ||
+  Opaque4Node* clone(Node* new_ctrl, PhaseIdealLoop* phase);
+  Opaque4Node* clone_and_replace_init(Node* new_init, Node* new_ctrl,PhaseIdealLoop* phase);
+  Opaque4Node* clone_and_replace_init_and_stride(Node* new_init, Node* new_stride, Node* new_ctrl, PhaseIdealLoop* phase);
+};
+
+// Class to represent a node being part of a Template Assertion Predicate Expression.
+//
+// The expression itself can belong to no, one, or two Template Assertion Predicates:
+// - None: This node is already dead (i.e. we replaced the Bool condition of the Template Assertion Predicate).
+// - Two: A OpaqueLoopInitNode could be part of two Template Assertion Predicates.
+// - One: In all other cases.
+class TemplateAssertionPredicateExpressionNode : public StackObj {
+  Node* const _node;
+
+ public:
+  explicit TemplateAssertionPredicateExpressionNode(Node* node) : _node(node) {
+    assert(is_in_expression(node), "must be valid");
+  }
+  NONCOPYABLE(TemplateAssertionPredicateExpressionNode);
+
+ private:
+  static bool is_template_assertion_predicate(Node* node);
+
+ public:
+  // Check whether the provided node is part of a Template Assertion Predicate Expression or not.
+  static bool is_in_expression(Node* node);
+
+  // Check if the opcode of node could be found in a Template Assertion Predicate Expression.
+  // This also provides a fast check whether a node is unrelated.
+  static bool is_maybe_in_expression(const Node* node) {
+    const int opcode = node->Opcode();
+    return (node->is_OpaqueLoopInit() ||
+            node->is_OpaqueLoopStride() ||
+            node->is_Bool() ||
+            node->is_Cmp() ||
             opcode == Op_AndL ||
             opcode == Op_OrL ||
             opcode == Op_RShiftL ||
@@ -306,9 +333,33 @@ class TemplateAssertionPredicateExpression : public StackObj {
             opcode == Op_CastII);
   }
 
-  Opaque4Node* clone(Node* new_ctrl, PhaseIdealLoop* phase);
-  Opaque4Node* clone_and_replace_init(Node* new_init, Node* new_ctrl,PhaseIdealLoop* phase);
-  Opaque4Node* clone_and_replace_init_and_stride(Node* new_init, Node* new_stride, Node* new_ctrl, PhaseIdealLoop* phase);
+  // Apply the given function to all Template Assertion Predicates (if any) to which this Template Assertion Predicate
+  // Expression Node belongs to.
+  template <class Callback>
+  void for_each_template_assertion_predicate(Callback callback) {
+    ResourceMark rm;
+    Unique_Node_List list;
+    list.push(_node);
+    DEBUG_ONLY(int template_counter = 0;)
+    for (uint i = 0; i < list.size(); i++) {
+      Node* next = list.at(i);
+      if (is_template_assertion_predicate(next)) {
+        callback(next->as_If());
+        DEBUG_ONLY(template_counter++;)
+      } else {
+        assert(!next->is_CFG(), "no CFG expected in Template Assertion Predicate Expression");
+        list.push_outputs_of(next);
+      }
+    }
+
+    // Each node inside a Template Assertion Predicate Expression is in between a Template Assertion Predicate and
+    // its OpaqueLoop* nodes (or an OpaqueLoop* node itself). The OpaqueLoop* nodes do not common up. Therefore, each
+    // Template Assertion Predicate Expression node belongs to a single expression - except for OpaqueLoopInitNodes.
+    // An OpaqueLoopInitNode is shared between the init and last value Template Assertion Predicate at creation.
+    // Later, when cloning the expressions, they are no longer shared.
+    assert(template_counter <= 2, "a node cannot be part of more than two templates");
+    assert(template_counter <= 1 || _node->is_OpaqueLoopInit(), "only OpaqueLoopInit nodes can be part of two templates");
+  }
 };
 
 // This class represents a Predicate Block (i.e. either a Loop Predicate Block, a Profiled Loop Predicate Block,

src/hotspot/share/opto/split_if.cpp

@@ -95,25 +95,7 @@ bool PhaseIdealLoop::split_up( Node *n, Node *blk1, Node *blk2 ) {
     return true;
   }
 
-  if (subgraph_has_opaque(n)) {
-    Unique_Node_List wq;
-    wq.push(n);
-    for (uint i = 0; i < wq.size(); i++) {
-      Node* m = wq.at(i);
-      if (m->is_If()) {
-        assert(assertion_predicate_has_loop_opaque_node(m->as_If()), "opaque node not reachable from if?");
-        TemplateAssertionPredicateExpression template_assertion_predicate_expression(m->in(1)->as_Opaque4());
-        Opaque4Node* cloned_opaque4_node = template_assertion_predicate_expression.clone(m->in(0), this);
-        _igvn.replace_input_of(m, 1, cloned_opaque4_node);
-      } else {
-        assert(!m->is_CFG(), "not CFG expected");
-        for (DUIterator_Fast jmax, j = m->fast_outs(jmax); j < jmax; j++) {
-          Node* u = m->fast_out(j);
-          wq.push(u);
-        }
-      }
-    }
-  }
+  clone_template_assertion_predicate_expression_down(n);
 
   if (n->Opcode() == Op_OpaqueZeroTripGuard) {
     // If this Opaque1 is part of the zero trip guard for a loop:
@@ -427,6 +409,27 @@ bool PhaseIdealLoop::clone_cmp_down(Node* n, const Node* blk1, const Node* blk2)
   return false;
 }
 
+// 'n' could be a node belonging to a Template Assertion Predicate Expression (i.e. any node between a Template
+// Assertion Predicate and its OpaqueLoop* nodes (included)). We cannot simply split this node up since this would
+// create a phi node inside the Template Assertion Predicate Expression - making it unrecognizable as such. Therefore,
+// we completely clone the entire Template Assertion Predicate Expression "down". This ensures that we have an
+// untouched copy that is still recognized by the Template Assertion Predicate matching code.
+void PhaseIdealLoop::clone_template_assertion_predicate_expression_down(Node* node) {
+  if (!TemplateAssertionPredicateExpressionNode::is_in_expression(node)) {
+    return;
+  }
+
+  TemplateAssertionPredicateExpressionNode template_assertion_predicate_expression_node(node);
+  auto clone_expression = [&](IfNode* template_assertion_predicate) {
+    Opaque4Node* opaque4_node = template_assertion_predicate->in(1)->as_Opaque4();
+    TemplateAssertionPredicateExpression template_assertion_predicate_expression(opaque4_node);
+    Node* new_ctrl = template_assertion_predicate->in(0);
+    Opaque4Node* cloned_opaque4_node = template_assertion_predicate_expression.clone(new_ctrl, this);
+    igvn().replace_input_of(template_assertion_predicate, 1, cloned_opaque4_node);
+  };
+  template_assertion_predicate_expression_node.for_each_template_assertion_predicate(clone_expression);
+}
+
 //------------------------------register_new_node------------------------------
 void PhaseIdealLoop::register_new_node( Node *n, Node *blk ) {
   assert(!n->is_CFG(), "must be data node");

test/hotspot/jtreg/compiler/predicates/assertion/TestSplitIfCloningDown.java

@@ -0,0 +1,117 @@
+/*
+ * 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 8330004
+ * @summary Sanity test to exercise code to clone a Template Assertion Predicate down in Split If.
+ * @requires vm.compiler2.enabled
+ * @run main/othervm -Xcomp -XX:LoopMaxUnroll=0
+ *                   -XX:CompileCommand=compileonly,compiler.predicates.assertion.TestSplitIfCloningDown::*
+ *                   compiler.predicates.assertion.TestSplitIfCloningDown
+ */
+
+/*
+ * @test id=no-flags
+ * @bug 8330004
+ * @summary Sanity test to exercise code to clone a Template Assertion Predicate down in Split If.
+ * @run main compiler.predicates.assertion.TestSplitIfCloningDown
+ */
+
+package compiler.predicates.assertion;
+
+public class TestSplitIfCloningDown {
+    static int[] iArr = new int[100];
+    static boolean flag;
+    static int iFld;
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 10000; i++) {
+            testPhiIntoNonOpaqueLoopExpressionNode();
+            testPhiIntoOpaqueLoopExpressionNode();
+        }
+    }
+
+
+    static void testPhiIntoNonOpaqueLoopExpressionNode() {
+        int zero = 34;
+        int limit = 2;
+        for (; limit < 4; limit *= 2);
+        for (int i = 2; i < limit; i++) {
+            zero = 0;
+        }
+
+
+        for (int t = 0; t < 100; t++) { // Use outer loop such that OpaqueLoop* will get an earlier ctrl.
+            iArr = new int[1000];
+
+            // Replaced by CMove which is an input into Template Assertion Predicate Expression which
+            // is not an OpaqueLoop* node. Split If will create a phi and tries to split a Template
+            // Assertion Predicate Expression node -> Need to clone template down.
+            int a;
+            if (flag) {
+                a = 4;
+            } else {
+                a = 3;
+            }
+
+            for (int i = 0; i < 100; i++) {
+                iArr[i+a] = 34; // Hoisted with Hoisted Check Predicate and Template Assertion Predicate
+                if (i * zero < iFld) { // Unswitched after Split If to check further template cloning.
+                    return;
+                }
+            }
+        }
+    }
+
+    // Same as test above but this time the phi inputs into an OpaqueLoop* node and not a node in between.
+    static void testPhiIntoOpaqueLoopExpressionNode() {
+        int zero = 34;
+        int limit = 2;
+        for (; limit < 4; limit *= 2);
+        for (int i = 2; i < limit; i++) {
+            zero = 0;
+        }
+
+        iArr = new int[1000];
+
+        // Replaced by CMove which is an input into Template Assertion Predicate Expression which
+        // is not an OpaqueLoop* node. Split If will create a phi and tries to split a Template
+        // Assertion Predicate Expression node -> Need to clone template down.
+        int a;
+        if (flag) {
+            a = 4;
+        } else {
+            a = 3;
+        }
+
+        for (int i = 0; i < 100; i++) {
+            iArr[i+a] = 34; // Hoisted with Hoisted Check Predicate and Template Assertion Predicate
+            if (i * zero < iFld) { // Unswitched after Split If to check further template cloning.
+                return;
+            }
+        }
+    }
+}
+