8255665: C2 should aggressively remove temporary hook nodes

TobiHartmann · TobiHartmann · commit eb85b8da3283 · 2020-11-05T08:02:47.000Z
Reviewed-by: chagedorn, kvn
diff --git a/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp b/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp
@@ -2377,7 +2377,7 @@ void MemoryGraphFixer::collect_memory_nodes() {
   while (dead_phis.size() > 0) {
     Node* n = dead_phis.pop();
     n->replace_by(_phase->C->top());
-    n->destruct();
+    n->destruct(&_phase->igvn());
   }
   for (int i = rpo_list.size() - 1; i >= 0; i--) {
     Node* c = rpo_list.at(i);
diff --git a/src/hotspot/share/opto/arraycopynode.cpp b/src/hotspot/share/opto/arraycopynode.cpp
@@ -183,7 +183,7 @@ Node* ArrayCopyNode::try_clone_instance(PhaseGVN *phase, bool can_reshape, int c
 
   const Type* src_type = phase->type(base_src);
 
-  MergeMemNode* mem = MergeMemNode::make(in_mem);
+  MergeMemNode* mem = phase->transform(MergeMemNode::make(in_mem))->as_MergeMem();
 
   const TypeInstPtr* inst_src = src_type->isa_instptr();
 
@@ -367,7 +367,7 @@ void ArrayCopyNode::array_copy_test_overlap(PhaseGVN *phase, bool can_reshape, b
 Node* ArrayCopyNode::array_copy_forward(PhaseGVN *phase,
                                         bool can_reshape,
                                         Node*& forward_ctl,
-                                        MergeMemNode* mm,
+                                        Node* mem,
                                         const TypePtr* atp_src,
                                         const TypePtr* atp_dest,
                                         Node* adr_src,
@@ -379,7 +379,7 @@ Node* ArrayCopyNode::array_copy_forward(PhaseGVN *phase,
                                         int count) {
   if (!forward_ctl->is_top()) {
     // copy forward
-    mm = mm->clone()->as_MergeMem();
+    MergeMemNode* mm = MergeMemNode::make(mem);
 
     if (count > 0) {
       BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
@@ -392,7 +392,7 @@ Node* ArrayCopyNode::array_copy_forward(PhaseGVN *phase,
         v = load(bs, phase, forward_ctl, mm, next_src, atp_src, value_type, copy_type);
         store(bs, phase, forward_ctl, mm, next_dest, atp_dest, v, value_type, copy_type);
       }
-    } else if(can_reshape) {
+    } else if (can_reshape) {
       PhaseIterGVN* igvn = phase->is_IterGVN();
       igvn->_worklist.push(adr_src);
       igvn->_worklist.push(adr_dest);
@@ -405,7 +405,7 @@ Node* ArrayCopyNode::array_copy_forward(PhaseGVN *phase,
 Node* ArrayCopyNode::array_copy_backward(PhaseGVN *phase,
                                          bool can_reshape,
                                          Node*& backward_ctl,
-                                         MergeMemNode* mm,
+                                         Node* mem,
                                          const TypePtr* atp_src,
                                          const TypePtr* atp_dest,
                                          Node* adr_src,
@@ -417,7 +417,7 @@ Node* ArrayCopyNode::array_copy_backward(PhaseGVN *phase,
                                          int count) {
   if (!backward_ctl->is_top()) {
     // copy backward
-    mm = mm->clone()->as_MergeMem();
+    MergeMemNode* mm = MergeMemNode::make(mem);
 
     BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
     assert(copy_type != T_OBJECT || !bs->array_copy_requires_gc_barriers(false, T_OBJECT, false, BarrierSetC2::Optimization), "only tightly coupled allocations for object arrays");
@@ -432,7 +432,7 @@ Node* ArrayCopyNode::array_copy_backward(PhaseGVN *phase,
       }
       Node* v = load(bs, phase, backward_ctl, mm, adr_src, atp_src, value_type, copy_type);
       store(bs, phase, backward_ctl, mm, adr_dest, atp_dest, v, value_type, copy_type);
-    } else if(can_reshape) {
+    } else if (can_reshape) {
       PhaseIterGVN* igvn = phase->is_IterGVN();
       igvn->_worklist.push(adr_src);
       igvn->_worklist.push(adr_dest);
@@ -564,11 +564,7 @@ Node *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
   Node* dest = in(ArrayCopyNode::Dest);
   const TypePtr* atp_src = get_address_type(phase, _src_type, src);
   const TypePtr* atp_dest = get_address_type(phase, _dest_type, dest);
-
-  Node *in_mem = in(TypeFunc::Memory);
-  if (!in_mem->is_MergeMem()) {
-    in_mem = MergeMemNode::make(in_mem);
-  }
+  Node* in_mem = in(TypeFunc::Memory);
 
   if (can_reshape) {
     assert(!phase->is_IterGVN()->delay_transform(), "cannot delay transforms");
@@ -580,13 +576,13 @@ Node *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
   array_copy_test_overlap(phase, can_reshape, disjoint_bases, count, forward_ctl, backward_ctl);
 
   Node* forward_mem = array_copy_forward(phase, can_reshape, forward_ctl,
-                                         in_mem->as_MergeMem(),
+                                         in_mem,
                                          atp_src, atp_dest,
                                          adr_src, base_src, adr_dest, base_dest,
                                          copy_type, value_type, count);
 
   Node* backward_mem = array_copy_backward(phase, can_reshape, backward_ctl,
-                                           in_mem->as_MergeMem(),
+                                           in_mem,
                                            atp_src, atp_dest,
                                            adr_src, base_src, adr_dest, base_dest,
                                            copy_type, value_type, count);
diff --git a/src/hotspot/share/opto/arraycopynode.hpp b/src/hotspot/share/opto/arraycopynode.hpp
@@ -100,12 +100,12 @@ class ArrayCopyNode : public CallNode {
                                bool disjoint_bases, int count,
                                Node*& forward_ctl, Node*& backward_ctl);
   Node* array_copy_forward(PhaseGVN *phase, bool can_reshape, Node*& ctl,
-                           MergeMemNode* mm,
+                           Node* mem,
                            const TypePtr* atp_src, const TypePtr* atp_dest,
                            Node* adr_src, Node* base_src, Node* adr_dest, Node* base_dest,
                            BasicType copy_type, const Type* value_type, int count);
   Node* array_copy_backward(PhaseGVN *phase, bool can_reshape, Node*& ctl,
-                            MergeMemNode* mm,
+                            Node* mem,
                             const TypePtr* atp_src, const TypePtr* atp_dest,
                             Node* adr_src, Node* base_src, Node* adr_dest, Node* base_dest,
                             BasicType copy_type, const Type* value_type, int count);
diff --git a/src/hotspot/share/opto/cfgnode.cpp b/src/hotspot/share/opto/cfgnode.cpp
@@ -2302,12 +2302,7 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
           Node* phi = mms.memory();
           mms.set_memory(phase->transform(phi));
         }
-        if (igvn) { // Unhook.
-          igvn->hash_delete(hook);
-          for (uint i = 1; i < hook->req(); i++) {
-            hook->set_req(i, NULL);
-          }
-        }
+        hook->destruct(igvn);
         // Replace self with the result.
         return result;
       }
diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp
@@ -1128,8 +1128,7 @@ void Compile::print_missing_nodes() {
   }
 }
 void Compile::record_modified_node(Node* n) {
-  if (_modified_nodes != NULL && !_inlining_incrementally &&
-      n->outcnt() != 0 && !n->is_Con()) {
+  if (_modified_nodes != NULL && !_inlining_incrementally && !n->is_Con()) {
     _modified_nodes->push(n);
   }
 }
diff --git a/src/hotspot/share/opto/convertnode.cpp b/src/hotspot/share/opto/convertnode.cpp
@@ -403,8 +403,7 @@ Node *ConvI2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
     Node *hook = new Node(1);
     hook->init_req(0, cx);  // Add a use to cx to prevent him from dying
     Node* cy = phase->C->constrained_convI2L(phase, y, TypeInt::make(rylo, ryhi, widen), NULL);
-    hook->del_req(0);  // Just yank bogus edge
-    hook->destruct();
+    hook->destruct(phase);
     switch (op) {
       case Op_AddI:  return new AddLNode(cx, cy);
       case Op_SubI:  return new SubLNode(cx, cy);
diff --git a/src/hotspot/share/opto/divnode.cpp b/src/hotspot/share/opto/divnode.cpp
@@ -326,14 +326,7 @@ static Node* long_by_long_mulhi(PhaseGVN* phase, Node* dividend, jlong magic_con
   Node* temp2 = phase->transform(new RShiftLNode(w1, phase->intcon(N / 2)));
 
   // Remove the bogus extra edges used to keep things alive
-  PhaseIterGVN* igvn = phase->is_IterGVN();
-  if (igvn != NULL) {
-    igvn->remove_dead_node(hook);
-  } else {
-    for (int i = 0; i < 4; i++) {
-      hook->set_req(i, NULL);
-    }
-  }
+  hook->destruct(phase);
 
   return new AddLNode(temp1, temp2);
 }
@@ -916,11 +909,7 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) {
       // cmov2 is now the mod
 
       // Now remove the bogus extra edges used to keep things alive
-      if (can_reshape) {
-        phase->is_IterGVN()->remove_dead_node(hook);
-      } else {
-        hook->set_req(0, NULL);   // Just yank bogus edge during Parse phase
-      }
+      hook->destruct(phase);
       return cmov2;
     }
   }
@@ -972,11 +961,7 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) {
   }
 
   // Now remove the bogus extra edges used to keep things alive
-  if (can_reshape) {
-    phase->is_IterGVN()->remove_dead_node(hook);
-  } else {
-    hook->set_req(0, NULL);       // Just yank bogus edge during Parse phase
-  }
+  hook->destruct(phase);
 
   // return the value
   return result;
@@ -1091,11 +1076,7 @@ Node *ModLNode::Ideal(PhaseGVN *phase, bool can_reshape) {
       // cmov2 is now the mod
 
       // Now remove the bogus extra edges used to keep things alive
-      if (can_reshape) {
-        phase->is_IterGVN()->remove_dead_node(hook);
-      } else {
-        hook->set_req(0, NULL);   // Just yank bogus edge during Parse phase
-      }
+      hook->destruct(phase);
       return cmov2;
     }
   }
@@ -1147,11 +1128,7 @@ Node *ModLNode::Ideal(PhaseGVN *phase, bool can_reshape) {
   }
 
   // Now remove the bogus extra edges used to keep things alive
-  if (can_reshape) {
-    phase->is_IterGVN()->remove_dead_node(hook);
-  } else {
-    hook->set_req(0, NULL);       // Just yank bogus edge during Parse phase
-  }
+  hook->destruct(phase);
 
   // return the value
   return result;
diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp
@@ -999,8 +999,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f
     if (adjusted_lim == NULL) {
       adjusted_lim = igvn->transform(new SubINode(hi, lo));
     }
-    hook->del_req(0); // Just yank bogus edge
-    hook->destruct();
+    hook->destruct(igvn);
     Node* newcmp = igvn->transform(new CmpUNode(adjusted_val, adjusted_lim));
     Node* newbool = igvn->transform(new BoolNode(newcmp, cond));
 
diff --git a/src/hotspot/share/opto/lcm.cpp b/src/hotspot/share/opto/lcm.cpp
@@ -430,7 +430,7 @@ void PhaseCFG::implicit_null_check(Block* block, Node *proj, Node *val, int allo
     tmp1->replace_by(tmp);
     tmp2->replace_by(tmp1);
     tmp->replace_by(tmp2);
-    tmp->destruct();
+    tmp->destruct(NULL);
   }
 
   // Remove the existing null check; use a new implicit null check instead.
diff --git a/src/hotspot/share/opto/loopPredicate.cpp b/src/hotspot/share/opto/loopPredicate.cpp
@@ -727,7 +727,7 @@ BoolNode* PhaseIdealLoop::rc_predicate(IdealLoopTree *loop, Node* ctrl,
     idx_type = (TypeInt*)mul->mul_ring(idx_type, scale_type);
     if (overflow || TypeInt::INT->higher_equal(idx_type)) {
       // May overflow
-      mul->destruct();
+      mul->destruct(&_igvn);
       if (!overflow) {
         max_idx_expr = new ConvI2LNode(max_idx_expr);
         register_new_node(max_idx_expr, ctrl);
diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp
@@ -1400,8 +1400,6 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*& loop) {
   assert(x->Opcode() == Op_Loop, "regular loops only");
   C->print_method(PHASE_BEFORE_CLOOPS, 3);
 
-  Node *hook = new Node(6);
-
   // ===================================================
   // Generate loop limit check to avoid integer overflow
   // in cases like next (cyclic loops):
@@ -1691,9 +1689,6 @@ bool PhaseIdealLoop::is_counted_loop(Node* x, IdealLoopTree*& loop) {
     }
   }
 
-  // Free up intermediate goo
-  _igvn.remove_dead_node(hook);
-
 #ifdef ASSERT
   assert(l->is_valid_counted_loop(), "counted loop shape is messed up");
   assert(l == loop->_head && l->phi() == phi && l->loopexit_or_null() == lex, "" );
@@ -2656,7 +2651,7 @@ void IdealLoopTree::split_fall_in( PhaseIdealLoop *phase, int fall_in_cnt ) {
       // with the CSE to avoid O(N^2) node blow-up.
       Node *p2 = igvn.hash_find_insert(p); // Look for a CSE
       if( p2 ) {                // Found CSE
-        p->destruct();          // Recover useless new node
+        p->destruct(&igvn);     // Recover useless new node
         p = p2;                 // Use old node
       } else {
         igvn.register_new_node_with_optimizer(p, old_phi);
diff --git a/src/hotspot/share/opto/macro.cpp b/src/hotspot/share/opto/macro.cpp
@@ -329,7 +329,7 @@ Node* PhaseMacroExpand::make_arraycopy_load(ArrayCopyNode* ac, intptr_t offset,
     Node* base = ac->in(ArrayCopyNode::Src);
     Node* adr = _igvn.transform(new AddPNode(base, base, MakeConX(offset)));
     const TypePtr* adr_type = _igvn.type(base)->is_ptr()->add_offset(offset);
-    MergeMemNode* mergemen = MergeMemNode::make(mem);
+    MergeMemNode* mergemen = _igvn.transform(MergeMemNode::make(mem))->as_MergeMem();
     BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
     res = ArrayCopyNode::load(bs, &_igvn, ctl, mergemen, adr, adr_type, type, bt);
   } else {
@@ -369,7 +369,7 @@ Node* PhaseMacroExpand::make_arraycopy_load(ArrayCopyNode* ac, intptr_t offset,
           return NULL;
         }
       }
-      MergeMemNode* mergemen = MergeMemNode::make(mem);
+      MergeMemNode* mergemen = _igvn.transform(MergeMemNode::make(mem))->as_MergeMem();
       BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
       res = ArrayCopyNode::load(bs, &_igvn, ctl, mergemen, adr, adr_type, type, bt);
     }
diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp
@@ -581,8 +581,11 @@ void Node::setup_is_top() {
 
 //------------------------------~Node------------------------------------------
 // Fancy destructor; eagerly attempt to reclaim Node numberings and storage
-void Node::destruct() {
-  Compile* compile = Compile::current();
+void Node::destruct(PhaseValues* phase) {
+  Compile* compile = (phase != NULL) ? phase->C : Compile::current();
+  if (phase != NULL && phase->is_IterGVN()) {
+    phase->is_IterGVN()->_worklist.remove(this);
+  }
   // If this is the most recently created node, reclaim its index. Otherwise,
   // record the node as dead to keep liveness information accurate.
   if ((uint)_idx+1 == compile->unique()) {
@@ -1399,7 +1402,6 @@ static void kill_dead_code( Node *dead, PhaseIterGVN *igvn ) {
       // Done with outputs.
       igvn->hash_delete(dead);
       igvn->_worklist.remove(dead);
-      igvn->C->remove_modified_node(dead);
       igvn->set_type(dead, Type::TOP);
       if (dead->is_macro()) {
         igvn->C->remove_macro_node(dead);
@@ -1435,6 +1437,7 @@ static void kill_dead_code( Node *dead, PhaseIterGVN *igvn ) {
           }
         }
       }
+      igvn->C->remove_modified_node(dead);
     } // (dead->outcnt() == 0)
   }   // while (nstack.size() > 0) for outputs
   return;
diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp
@@ -236,7 +236,7 @@ class Node {
   // Delete is a NOP
   void operator delete( void *ptr ) {}
   // Fancy destructor; eagerly attempt to reclaim Node numberings and storage
-  void destruct();
+  void destruct(PhaseValues* phase);
 
   // Create a new Node.  Required is the number is of inputs required for
   // semantic correctness.
diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp
@@ -749,7 +749,7 @@ ConNode* PhaseValues::uncached_makecon(const Type *t) {
       loc->clear(); // do not put debug info on constants
     }
   } else {
-    x->destruct();              // Hit, destroy duplicate constant
+    x->destruct(this);          // Hit, destroy duplicate constant
     x = k;                      // use existing constant
   }
   return x;
@@ -1069,9 +1069,9 @@ void PhaseIterGVN::init_verifyPhaseIterGVN() {
   Unique_Node_List* modified_list = C->modified_nodes();
   while (modified_list != NULL && modified_list->size()) {
     Node* n = modified_list->pop();
-    if (n->outcnt() != 0 && !n->is_Con() && !_worklist.member(n)) {
+    if (!n->is_Con() && !_worklist.member(n)) {
       n->dump();
-      assert(false, "modified node is not on IGVN._worklist");
+      fatal("modified node is not on IGVN._worklist");
     }
   }
 #endif
@@ -1083,9 +1083,9 @@ void PhaseIterGVN::verify_PhaseIterGVN() {
   Unique_Node_List* modified_list = C->modified_nodes();
   while (modified_list != NULL && modified_list->size()) {
     Node* n = modified_list->pop();
-    if (n->outcnt() != 0 && !n->is_Con()) { // skip dead and Con nodes
+    if (!n->is_Con()) { // skip Con nodes
       n->dump();
-      assert(false, "modified node was not processed by IGVN.transform_old()");
+      fatal("modified node was not processed by IGVN.transform_old()");
     }
   }
 #endif
@@ -1472,8 +1472,7 @@ void PhaseIterGVN::subsume_node( Node *old, Node *nn ) {
     }
   }
 #endif
-  _worklist.remove(temp);   // this can be necessary
-  temp->destruct();         // reuse the _idx of this little guy
+  temp->destruct(this);     // reuse the _idx of this little guy
 }
 
 //------------------------------add_users_to_worklist--------------------------
diff --git a/src/hotspot/share/opto/split_if.cpp b/src/hotspot/share/opto/split_if.cpp
@@ -337,7 +337,7 @@ Node *PhaseIdealLoop::spinup( Node *iff_dom, Node *new_false, Node *new_true, No
       if( t ) {                 // See if we already have this one
         // phi_post will not be used, so kill it
         _igvn.remove_dead_node(phi_post);
-        phi_post->destruct();
+        phi_post->destruct(&_igvn);
         phi_post = t;
       } else {
         register_new_node( phi_post, prior_n );

Original file line number	Diff line number	Diff line change
`@@ -2377,7 +2377,7 @@ void MemoryGraphFixer::collect_memory_nodes() {`
`2377`	`2377`	`while (dead_phis.size() > 0) {`
`2378`	`2378`	`Node* n = dead_phis.pop();`
`2379`	`2379`	`n->replace_by(_phase->C->top());`
`2380`		`- n->destruct();`
	`2380`	`+ n->destruct(&_phase->igvn());`
`2381`	`2381`	`}`
`2382`	`2382`	`for (int i = rpo_list.size() - 1; i >= 0; i--) {`
`2383`	`2383`	`Node* c = rpo_list.at(i);`
Original file line number	Diff line number	Diff line change
`@@ -2302,12 +2302,7 @@ Node PhiNode::Ideal(PhaseGVN phase, bool can_reshape) {`
`2302`	`2302`	`Node* phi = mms.memory();`
`2303`	`2303`	`mms.set_memory(phase->transform(phi));`
`2304`	`2304`	`}`
`2305`		`- if (igvn) { // Unhook.`
`2306`		`- igvn->hash_delete(hook);`
`2307`		`- for (uint i = 1; i < hook->req(); i++) {`
`2308`		`- hook->set_req(i, NULL);`
`2309`		`- }`
`2310`		`- }`
	`2305`	`+ hook->destruct(igvn);`
`2311`	`2306`	`// Replace self with the result.`
`2312`	`2307`	`return result;`
`2313`	`2308`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1128,8 +1128,7 @@ void Compile::print_missing_nodes() {`
`1128`	`1128`	`}`
`1129`	`1129`	`}`
`1130`	`1130`	`void Compile::record_modified_node(Node* n) {`
`1131`		`- if (_modified_nodes != NULL && !_inlining_incrementally &&`
`1132`		`- n->outcnt() != 0 && !n->is_Con()) {`
	`1131`	`+ if (_modified_nodes != NULL && !_inlining_incrementally && !n->is_Con()) {`
`1133`	`1132`	`_modified_nodes->push(n);`
`1134`	`1133`	`}`
`1135`	`1134`	`}`
Original file line number	Diff line number	Diff line change
`@@ -999,8 +999,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f`
`999`	`999`	`if (adjusted_lim == NULL) {`
`1000`	`1000`	`adjusted_lim = igvn->transform(new SubINode(hi, lo));`
`1001`	`1001`	`}`
`1002`		`- hook->del_req(0); // Just yank bogus edge`
`1003`		`- hook->destruct();`
	`1002`	`+ hook->destruct(igvn);`
`1004`	`1003`	`Node* newcmp = igvn->transform(new CmpUNode(adjusted_val, adjusted_lim));`
`1005`	`1004`	`Node* newbool = igvn->transform(new BoolNode(newcmp, cond));`
`1006`	`1005`
Original file line number	Diff line number	Diff line change
`@@ -430,7 +430,7 @@ void PhaseCFG::implicit_null_check(Block* block, Node proj, Node val, int allo`
`430`	`430`	`tmp1->replace_by(tmp);`
`431`	`431`	`tmp2->replace_by(tmp1);`
`432`	`432`	`tmp->replace_by(tmp2);`
`433`		`- tmp->destruct();`
	`433`	`+ tmp->destruct(NULL);`
`434`	`434`	`}`
`435`	`435`
`436`	`436`	`// Remove the existing null check; use a new implicit null check instead.`