8330004: Refactor cloning down code in Split If for Template Assertio…

…n 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 @@ class PhaseIdealLoop : public PhaseTransform {
                                                  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 @@ class PhaseIdealLoop : public PhaseTransform {
 
   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 @@ class Unique_Node_List : public Node_List {
     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");