8350577: Fix missing Assertion Predicates when splitting loops

chhagedorn · chhagedorn · commit 25925138b0a7 · 2025-03-31T07:38:53.000Z
Reviewed-by: epeter, kvn
diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp
@@ -812,7 +812,7 @@ void PhaseIdealLoop::do_peeling(IdealLoopTree *loop, Node_List &old_new) {
   }
 
   // Step 5: Assertion Predicates initialization
-  if (counted_loop && UseLoopPredicate) {
+  if (counted_loop) {
     initialize_assertion_predicates_for_peeled_loop(new_head->as_CountedLoop(), head->as_CountedLoop(),
                                                     first_node_index_in_post_loop_body, old_new);
  }
@@ -1450,11 +1450,9 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
   DEBUG_ONLY(const uint last_node_index_from_backedge_goo = Compile::current()->unique() - 1);
 
   DEBUG_ONLY(ensure_zero_trip_guard_proj(outer_main_head->in(LoopNode::EntryControl), true);)
-  if (UseLoopPredicate) {
-    initialize_assertion_predicates_for_main_loop(pre_head, main_head, first_node_index_in_pre_loop_body,
-                                                  last_node_index_in_pre_loop_body,
-                                                  DEBUG_ONLY(last_node_index_from_backedge_goo COMMA) old_new);
-  }
+  initialize_assertion_predicates_for_main_loop(pre_head, main_head, first_node_index_in_pre_loop_body,
+                                                last_node_index_in_pre_loop_body,
+                                                DEBUG_ONLY(last_node_index_from_backedge_goo COMMA) old_new);
 
   // Step B4: Shorten the pre-loop to run only 1 iteration (for now).
   // RCE and alignment may change this later.
@@ -1683,9 +1681,7 @@ Node *PhaseIdealLoop::insert_post_loop(IdealLoopTree* loop, Node_List& old_new,
   }
 
   DEBUG_ONLY(ensure_zero_trip_guard_proj(post_head->in(LoopNode::EntryControl), false);)
-  if (UseLoopPredicate) {
-    initialize_assertion_predicates_for_post_loop(main_head, post_head, first_node_index_in_cloned_loop_body);
-  }
+  initialize_assertion_predicates_for_post_loop(main_head, post_head, first_node_index_in_cloned_loop_body);
   return new_main_exit;
 }
 
@@ -1719,7 +1715,7 @@ void PhaseIdealLoop::initialize_assertion_predicates_for_peeled_loop(CountedLoop
                                                                      const uint first_node_index_in_cloned_loop_body,
                                                                      const Node_List& old_new) {
   const NodeInOriginalLoopBody node_in_original_loop_body(first_node_index_in_cloned_loop_body, old_new);
-  create_assertion_predicates_at_loop(peeled_loop_head, remaining_loop_head, node_in_original_loop_body, false);
+  create_assertion_predicates_at_loop(peeled_loop_head, remaining_loop_head, node_in_original_loop_body, true);
 }
 
 // Source Loop: Cloned   - pre_loop_head
@@ -1739,6 +1735,9 @@ void PhaseIdealLoop::initialize_assertion_predicates_for_main_loop(CountedLoopNo
 
 // Source Loop: Original - main_loop_head
 // Target Loop: Cloned   - post_loop_head
+//
+// The post loop is cloned before the pre loop. Do not kill the old Template Assertion Predicates, yet. We need to clone
+// from them when creating the pre loop. Only then we can kill them.
 void PhaseIdealLoop::initialize_assertion_predicates_for_post_loop(CountedLoopNode* main_loop_head,
                                                                    CountedLoopNode* post_loop_head,
                                                                    const uint first_node_index_in_cloned_loop_body) {
@@ -1749,9 +1748,9 @@ void PhaseIdealLoop::initialize_assertion_predicates_for_post_loop(CountedLoopNo
 void PhaseIdealLoop::create_assertion_predicates_at_loop(CountedLoopNode* source_loop_head,
                                                          CountedLoopNode* target_loop_head,
                                                          const NodeInLoopBody& _node_in_loop_body,
-                                                         const bool clone_template) {
+                                                         const bool kill_old_template) {
   CreateAssertionPredicatesVisitor create_assertion_predicates_visitor(target_loop_head, this, _node_in_loop_body,
-                                                                       clone_template);
+                                                                       kill_old_template);
   Node* source_loop_entry = source_loop_head->skip_strip_mined()->in(LoopNode::EntryControl);
   PredicateIterator predicate_iterator(source_loop_entry);
   predicate_iterator.for_each(create_assertion_predicates_visitor);
@@ -1760,11 +1759,11 @@ void PhaseIdealLoop::create_assertion_predicates_at_loop(CountedLoopNode* source
 void PhaseIdealLoop::create_assertion_predicates_at_main_or_post_loop(CountedLoopNode* source_loop_head,
                                                                       CountedLoopNode* target_loop_head,
                                                                       const NodeInLoopBody& _node_in_loop_body,
-                                                                      bool clone_template) {
+                                                                      const bool kill_old_template) {
   Node* old_target_loop_head_entry = target_loop_head->skip_strip_mined()->in(LoopNode::EntryControl);
   const uint node_index_before_new_assertion_predicate_nodes = C->unique();
   const bool need_to_rewire_old_target_loop_entry_dependencies = old_target_loop_head_entry->outcnt() > 1;
-  create_assertion_predicates_at_loop(source_loop_head, target_loop_head, _node_in_loop_body, clone_template);
+  create_assertion_predicates_at_loop(source_loop_head, target_loop_head, _node_in_loop_body, kill_old_template);
   if (need_to_rewire_old_target_loop_entry_dependencies) {
     rewire_old_target_loop_entry_dependency_to_new_entry(target_loop_head, old_target_loop_head_entry,
                                                          node_index_before_new_assertion_predicate_nodes);
diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp
@@ -993,10 +993,11 @@ class PhaseIdealLoop : public PhaseTransform {
   void initialize_assertion_predicates_for_post_loop(CountedLoopNode* main_loop_head, CountedLoopNode* post_loop_head,
                                                      uint first_node_index_in_cloned_loop_body);
   void create_assertion_predicates_at_loop(CountedLoopNode* source_loop_head, CountedLoopNode* target_loop_head,
-                                           const NodeInLoopBody& _node_in_loop_body, bool clone_template);
+                                           const NodeInLoopBody& _node_in_loop_body, bool kill_old_template);
   void create_assertion_predicates_at_main_or_post_loop(CountedLoopNode* source_loop_head,
                                                         CountedLoopNode* target_loop_head,
-                                                        const NodeInLoopBody& _node_in_loop_body, bool clone_template);
+                                                        const NodeInLoopBody& _node_in_loop_body,
+                                                        bool kill_old_template);
   void rewire_old_target_loop_entry_dependency_to_new_entry(LoopNode* target_loop_head,
                                                             const Node* old_target_loop_entry,
                                                             uint node_index_before_new_assertion_predicate_nodes);
diff --git a/src/hotspot/share/opto/predicates.cpp b/src/hotspot/share/opto/predicates.cpp
@@ -960,41 +960,24 @@ void Predicates::dump_for_loop(LoopNode* loop_node) {
 CreateAssertionPredicatesVisitor::CreateAssertionPredicatesVisitor(CountedLoopNode* target_loop_head,
                                                                    PhaseIdealLoop* phase,
                                                                    const NodeInLoopBody& node_in_loop_body,
-                                                                   const bool clone_template)
+                                                                   bool kill_old_template)
     : _init(target_loop_head->init_trip()),
       _stride(target_loop_head->stride()),
       _target_loop_head(target_loop_head),
       _old_target_loop_entry(target_loop_head->skip_strip_mined()->in(LoopNode::EntryControl)),
       _current_predicate_chain_head(target_loop_head->skip_strip_mined()), // Initially no predicates, yet.
       _phase(phase),
-      _has_hoisted_check_parse_predicates(false),
       _node_in_loop_body(node_in_loop_body),
-      _clone_template(clone_template) {}
-
-// Keep track of whether we are in the correct Predicate Block where Template Assertion Predicates can be found.
-// The PredicateIterator will always start at the loop entry and first visits the Loop Limit Check Predicate Block.
-void CreateAssertionPredicatesVisitor::visit(const ParsePredicate& parse_predicate) {
-  Deoptimization::DeoptReason deopt_reason = parse_predicate.head()->deopt_reason();
-  if (deopt_reason == Deoptimization::Reason_predicate ||
-      deopt_reason == Deoptimization::Reason_profile_predicate) {
-    _has_hoisted_check_parse_predicates = true;
-  }
-}
+      _kill_old_template(kill_old_template) {}
 
+// Clone Template Assertion Predicate and create an Initialized Assertion Predicate from it.
 void CreateAssertionPredicatesVisitor::visit(const TemplateAssertionPredicate& template_assertion_predicate) {
-  if (!_has_hoisted_check_parse_predicates) {
-    // Only process if we are in the correct Predicate Block.
-    return;
-  }
-  if (_clone_template) {
-    TemplateAssertionPredicate cloned_template_assertion_predicate =
-        clone_template_and_replace_init_input(template_assertion_predicate);
-    initialize_from_template(template_assertion_predicate, cloned_template_assertion_predicate.tail());
-    _current_predicate_chain_head = cloned_template_assertion_predicate.head();
-  } else {
-    InitializedAssertionPredicate initialized_assertion_predicate =
-        initialize_from_template(template_assertion_predicate, _old_target_loop_entry);
-    _current_predicate_chain_head = initialized_assertion_predicate.head();
+  TemplateAssertionPredicate cloned_template_assertion_predicate =
+      clone_template_and_replace_init_input(template_assertion_predicate);
+  initialize_from_template(template_assertion_predicate, cloned_template_assertion_predicate.tail());
+  _current_predicate_chain_head = cloned_template_assertion_predicate.head();
+  if (_kill_old_template) {
+    template_assertion_predicate.kill(_phase->igvn());
   }
 }
 
@@ -1110,18 +1093,11 @@ CloneUnswitchedLoopPredicatesVisitor::CloneUnswitchedLoopPredicatesVisitor(
     PhaseIdealLoop* phase)
     : _clone_predicate_to_true_path_loop(true_path_loop_head, node_in_true_path_loop_body, phase),
       _clone_predicate_to_false_path_loop(false_path_loop_head, node_in_false_path_loop_body, phase),
-      _phase(phase),
-      _has_hoisted_check_parse_predicates(false) {}
+      _phase(phase) {}
 
 // Keep track of whether we are in the correct Predicate Block where Template Assertion Predicates can be found.
 // The PredicateIterator will always start at the loop entry and first visits the Loop Limit Check Predicate Block.
 void CloneUnswitchedLoopPredicatesVisitor::visit(const ParsePredicate& parse_predicate) {
-  Deoptimization::DeoptReason deopt_reason = parse_predicate.head()->deopt_reason();
-  if (deopt_reason == Deoptimization::Reason_predicate ||
-      deopt_reason == Deoptimization::Reason_profile_predicate) {
-    _has_hoisted_check_parse_predicates = true;
-  }
-
   _clone_predicate_to_true_path_loop.clone_parse_predicate(parse_predicate, false);
   _clone_predicate_to_false_path_loop.clone_parse_predicate(parse_predicate, true);
   parse_predicate.kill(_phase->igvn());
@@ -1130,11 +1106,6 @@ void CloneUnswitchedLoopPredicatesVisitor::visit(const ParsePredicate& parse_pre
 // Clone the Template Assertion Predicate, which is currently found before the newly added unswitched loop selector,
 // to the true path and false path loop.
 void CloneUnswitchedLoopPredicatesVisitor::visit(const TemplateAssertionPredicate& template_assertion_predicate) {
-  if (!_has_hoisted_check_parse_predicates) {
-    // Only process if we are in the correct Predicate Block.
-    return;
-  }
-
   _clone_predicate_to_true_path_loop.clone_template_assertion_predicate(template_assertion_predicate);
   _clone_predicate_to_false_path_loop.clone_template_assertion_predicate(template_assertion_predicate);
   template_assertion_predicate.kill(_phase->igvn());
diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp
@@ -1091,9 +1091,8 @@ class CreateAssertionPredicatesVisitor : public PredicateVisitor {
   Node* const _old_target_loop_entry;
   Node* _current_predicate_chain_head;
   PhaseIdealLoop* const _phase;
-  bool _has_hoisted_check_parse_predicates;
   const NodeInLoopBody& _node_in_loop_body;
-  const bool _clone_template;
+  const bool _kill_old_template;
 
   TemplateAssertionPredicate
   clone_template_and_replace_init_input(const TemplateAssertionPredicate& template_assertion_predicate) const;
@@ -1104,12 +1103,11 @@ class CreateAssertionPredicatesVisitor : public PredicateVisitor {
 
  public:
   CreateAssertionPredicatesVisitor(CountedLoopNode* target_loop_head, PhaseIdealLoop* phase,
-                                   const NodeInLoopBody& node_in_loop_body, bool clone_template);
+                                   const NodeInLoopBody& node_in_loop_body, bool kill_old_template);
   NONCOPYABLE(CreateAssertionPredicatesVisitor);
 
   using PredicateVisitor::visit;
 
-  void visit(const ParsePredicate& parse_predicate) override;
   void visit(const TemplateAssertionPredicate& template_assertion_predicate) override;
 };
 
@@ -1178,7 +1176,6 @@ class CloneUnswitchedLoopPredicatesVisitor : public PredicateVisitor {
   ClonePredicateToTargetLoop _clone_predicate_to_false_path_loop;
 
   PhaseIdealLoop* const _phase;
-  bool _has_hoisted_check_parse_predicates;
 
  public:
   CloneUnswitchedLoopPredicatesVisitor(LoopNode* true_path_loop_head,
diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java
