8327963: C2: fix construction of memory graph around Initialize node to prevent incorrect execution if allocation is removed

src/hotspot/share/opto/addnode.hpp

@@ -201,6 +201,13 @@ class AddPNode : public Node {
 
   // Do not match base-ptr edge
   virtual uint match_edge(uint idx) const;
+
+#ifdef ASSERT
+  bool address_input_has_same_base() const {
+    Node *addp = in(Address);
+    return !addp->is_AddP() || addp->in(Base)->is_top() || addp->in(Base) == in(Base);
+  }
+#endif
 };
 
 //------------------------------OrINode----------------------------------------

src/hotspot/share/opto/c2_globals.hpp

@@ -677,11 +677,18 @@
   develop(bool, TraceIterativeGVN, false,                                   \
           "Print progress during Iterative Global Value Numbering")         \
                                                                             \
+  develop(bool, UseDeepIGVNRevisit, true,                                   \
+          "Re-process nodes that could benefit from a deep revisit after "  \
+          "the IGVN worklist drains")                                       \
+                                                                            \
   develop(uint, VerifyIterativeGVN, 0,                                      \
-          "Verify Iterative Global Value Numbering"                         \
-          "=XY, with Y: verify Def-Use modifications during IGVN"           \
-          "          X: verify that type(n) == n->Value() after IGVN"       \
-          "X and Y in 0=off; 1=on")                                         \
+          "Verify Iterative Global Value Numbering =EDCBA, with:"           \
+          "  E: verify node specific invariants"                            \
+          "  D: verify Node::Identity did not miss opportunities"           \
+          "  C: verify Node::Ideal did not miss opportunities"              \
+          "  B: verify that type(n) == n->Value() after IGVN"               \
+          "  A: verify Def-Use modifications during IGVN"                   \
+          "Each can be 0=off or 1=on")                                      \
           constraint(VerifyIterativeGVNConstraintFunc, AtParse)             \
                                                                             \
   develop(bool, TraceCISCSpill, false,                                      \

src/hotspot/share/opto/cfgnode.cpp

@@ -735,7 +735,7 @@ Node *RegionNode::Ideal(PhaseGVN *phase, bool can_reshape) {
 #endif
       }
       // Remove the RegionNode itself from DefUse info
-      igvn->remove_dead_node(this);
+      igvn->remove_dead_node(this, PhaseIterGVN::NodeOrigin::Graph);
       return nullptr;
     }
     return this;                // Record progress
@@ -1007,7 +1007,7 @@ bool RegionNode::optimize_trichotomy(PhaseIterGVN* igvn) {
     BoolNode* new_bol = new BoolNode(bol2->in(1), res);
     igvn->replace_input_of(iff2, 1, igvn->transform((proj2->_con == 1) ? new_bol : new_bol->negate(igvn)));
     if (new_bol->outcnt() == 0) {
-      igvn->remove_dead_node(new_bol);
+      igvn->remove_dead_node(new_bol, PhaseIterGVN::NodeOrigin::Speculative);
     }
   }
   return false;
@@ -2095,6 +2095,20 @@ bool PhiNode::is_split_through_mergemem_terminating() const {
   return true;
 }
 
+// Is one of the inputs a Cast that has not been processed by igvn yet?
+bool PhiNode::wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const {
+  for (uint i = 1, cnt = req(); i < cnt; ++i) {
+    Node* n = in(i);
+    while (n != nullptr && n->is_ConstraintCast()) {
+      if (igvn->_worklist.member(n)) {
+        return true;
+      }
+      n = n->in(1);
+    }
+  }
+  return false;
+}
+
 //------------------------------Ideal------------------------------------------
 // Return a node which is more "ideal" than the current node.  Must preserve
 // the CFG, but we can still strip out dead paths.
@@ -2153,6 +2167,28 @@ Node *PhiNode::Ideal(PhaseGVN *phase, bool can_reshape) {
       // If there is a chance that the region can be optimized out do
       // not add a cast node that we can't remove yet.
       !wait_for_region_igvn(phase)) {
+    // If one of the inputs is a cast that has yet to be processed by igvn, delay processing of this node to give the
+    // inputs a chance to optimize and possibly end up with identical inputs (casts included).
+    // Say we have:
+    // (Phi region (Cast#1 c uin) (Cast#2 c uin))
+    // and Cast#1 and Cast#2 have not had a chance to common yet
+    // if the unique_input() transformation below proceeds, then PhiNode::Ideal returns:
+    // (Cast#3 region uin) (1)
+    // If PhiNode::Ideal is delayed until Cast#1 and Cast#2 common, then it returns:
+    // (Cast#1 c uin) (2)
+    //
+    // In (1) the resulting cast is conservatively pinned at a later control and while Cast#3 and Cast#1/Cast#2 still
+    // have a chance to common, that requires proving that c dominates region in ConstraintCastNode::dominating_cast()
+    // which may not happen if control flow is too complicated and another pass of loop opts doesn't run. Delaying the
+    // transformation here should allow a more optimal result.
+    // Beyond the efficiency concern, there is a risk, if the casts are CastPPs, to end up with a chain of AddPs with
+    // different base inputs (but a unique uncasted base input). This breaks an invariant in the shape of address
+    // subtrees.
+    PhaseIterGVN* igvn = phase->is_IterGVN();
+    if (wait_for_cast_input_igvn(igvn)) {
+      igvn->_worklist.push(this);
+      return nullptr;
+    }
     uncasted = true;
     uin = unique_input(phase, true);
   }

src/hotspot/share/opto/cfgnode.hpp

@@ -182,6 +182,8 @@ class PhiNode : public TypeNode {
 
   bool is_split_through_mergemem_terminating() const;
 
+  bool wait_for_cast_input_igvn(const PhaseIterGVN* igvn) const;
+
 public:
   // Node layout (parallels RegionNode):
   enum { Region,                // Control input is the Phi's region.

src/hotspot/share/opto/classes.hpp

@@ -273,6 +273,7 @@ macro(NegL)
 macro(NegD)
 macro(NegF)
 macro(NeverBranch)
+macro(NarrowMemProj)
 macro(OnSpinWait)
 macro(Opaque1)
 macro(OpaqueLoopInit)

src/hotspot/share/opto/compile.cpp

@@ -2280,7 +2280,7 @@ void Compile::remove_root_to_sfpts_edges(PhaseIterGVN& igvn) {
       if (n != nullptr && n->is_SafePoint()) {
         r->rm_prec(i);
         if (n->outcnt() == 0) {
-          igvn.remove_dead_node(n);
+          igvn.remove_dead_node(n, PhaseIterGVN::NodeOrigin::Graph);
         }
         --i;
       }
@@ -2325,7 +2325,7 @@ void Compile::Optimize() {
 #endif
   {
     TracePhase tp(_t_iterGVN);
-    igvn.optimize();
+    igvn.optimize(true);
   }
 
   if (failing())  return;
@@ -2389,7 +2389,7 @@ void Compile::Optimize() {
       PhaseRenumberLive prl(initial_gvn(), *igvn_worklist());
     }
     igvn.reset_from_gvn(initial_gvn());
-    igvn.optimize();
+    igvn.optimize(true);
     if (failing()) return;
   }
 
@@ -2421,7 +2421,7 @@ void Compile::Optimize() {
       int mcount = macro_count(); // Record number of allocations and locks before IGVN
 
       // Optimize out fields loads from scalar replaceable allocations.
-      igvn.optimize();
+      igvn.optimize(true);
       print_method(PHASE_ITER_GVN_AFTER_EA, 2);
 
       if (failing()) return;
@@ -2500,7 +2500,7 @@ void Compile::Optimize() {
   {
     TracePhase tp(_t_iterGVN2);
     igvn.reset_from_igvn(&ccp);
-    igvn.optimize();
+    igvn.optimize(true);
   }
   print_method(PHASE_ITER_GVN2, 2);
 
@@ -3383,10 +3383,7 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
 
   case Op_AddP: {               // Assert sane base pointers
     Node *addp = n->in(AddPNode::Address);
-    assert( !addp->is_AddP() ||
-            addp->in(AddPNode::Base)->is_top() || // Top OK for allocation
-            addp->in(AddPNode::Base) == n->in(AddPNode::Base),
-            "Base pointers must match (addp %u)", addp->_idx );
+    assert(n->as_AddP()->address_input_has_same_base(), "Base pointers must match (addp %u)", addp->_idx );
 #ifdef _LP64
     if ((UseCompressedOops || UseCompressedClassPointers) &&
         addp->Opcode() == Op_ConP &&

src/hotspot/share/opto/escape.cpp

@@ -201,7 +201,7 @@ bool ConnectionGraph::compute_escape() {
         if (!UseStoreStoreForCtor || n->req() > MemBarNode::Precedent) {
           storestore_worklist.append(n->as_MemBarStoreStore());
         }
-        break;
+        // If MemBarStoreStore has a precedent edge add it to the worklist (like MemBarRelease)
       case Op_MemBarRelease:
         if (n->req() > MemBarNode::Precedent) {
           record_for_optimizer(n);
@@ -863,7 +863,7 @@ Node* ConnectionGraph::split_castpp_load_through_phi(Node* curr_addp, Node* curr
 //                      \|/
 //                      Phi        # "Field" Phi
 //
-void ConnectionGraph::reduce_phi_on_castpp_field_load(Node* curr_castpp, GrowableArray<Node *>  &alloc_worklist, GrowableArray<Node *>  &memnode_worklist) {
+void ConnectionGraph::reduce_phi_on_castpp_field_load(Node* curr_castpp, GrowableArray<Node*> &alloc_worklist) {
   Node* ophi = curr_castpp->in(1);
   assert(ophi->is_Phi(), "Expected this to be a Phi node.");
 
@@ -918,7 +918,7 @@ void ConnectionGraph::reduce_phi_on_castpp_field_load(Node* curr_castpp, Growabl
         j = MIN2(j, (int)use->outcnt()-1);
       }
 
-      _igvn->remove_dead_node(use);
+      _igvn->remove_dead_node(use, PhaseIterGVN::NodeOrigin::Graph);
     }
     --i;
     i = MIN2(i, (int)curr_castpp->outcnt()-1);
@@ -1279,7 +1279,7 @@ bool ConnectionGraph::reduce_phi_on_safepoints_helper(Node* ophi, Node* cast, No
   return true;
 }
 
-void ConnectionGraph::reduce_phi(PhiNode* ophi, GrowableArray<Node *>  &alloc_worklist, GrowableArray<Node *>  &memnode_worklist) {
+void ConnectionGraph::reduce_phi(PhiNode* ophi, GrowableArray<Node*> &alloc_worklist) {
   bool delay = _igvn->delay_transform();
   _igvn->set_delay_transform(true);
   _igvn->hash_delete(ophi);
@@ -1307,7 +1307,7 @@ void ConnectionGraph::reduce_phi(PhiNode* ophi, GrowableArray<Node *>  &alloc_wo
   // splitting CastPPs we make reference to the inputs of the Cmp that is used
   // by the If controlling the CastPP.
   for (uint i = 0; i < castpps.size(); i++) {
-    reduce_phi_on_castpp_field_load(castpps.at(i), alloc_worklist, memnode_worklist);
+    reduce_phi_on_castpp_field_load(castpps.at(i), alloc_worklist);
   }
 
   for (uint i = 0; i < others.size(); i++) {
@@ -4151,6 +4151,11 @@ Node* ConnectionGraph::find_inst_mem(Node *orig_mem, int alias_idx, GrowableArra
         // which contains this memory slice, otherwise skip over it.
         if (alloc == nullptr || alloc->_idx != (uint)toop->instance_id()) {
           result = proj_in->in(TypeFunc::Memory);
+        } else if (C->get_alias_index(result->adr_type()) != alias_idx) {
+          assert(C->get_general_index(alias_idx) == C->get_alias_index(result->adr_type()), "should be projection for the same field/array element");
+          result = get_map(result->_idx);
+          assert(result != nullptr, "new projection should have been allocated");
+          break;
         }
       } else if (proj_in->is_MemBar()) {
         // Check if there is an array copy for a clone
@@ -4447,6 +4452,22 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
       _compile->get_alias_index(tinst->add_offset(oopDesc::mark_offset_in_bytes()));
       _compile->get_alias_index(tinst->add_offset(oopDesc::klass_offset_in_bytes()));
       if (alloc->is_Allocate() && (t->isa_instptr() || t->isa_aryptr())) {
+        // Add a new NarrowMem projection for each existing NarrowMem projection with new adr type
+        InitializeNode* init = alloc->as_Allocate()->initialization();
+        assert(init != nullptr, "can't find Initialization node for this Allocate node");
+        auto process_narrow_proj = [&](NarrowMemProjNode* proj) {
+          const TypePtr* adr_type = proj->adr_type();
+          const TypePtr* new_adr_type = tinst->add_offset(adr_type->offset());
+          if (adr_type != new_adr_type && !init->already_has_narrow_mem_proj_with_adr_type(new_adr_type)) {
+            DEBUG_ONLY( uint alias_idx = _compile->get_alias_index(new_adr_type); )
+            assert(_compile->get_general_index(alias_idx) == _compile->get_alias_index(adr_type), "new adr type should be narrowed down from existing adr type");
+            NarrowMemProjNode* new_proj = new NarrowMemProjNode(init, new_adr_type);
+            igvn->set_type(new_proj, new_proj->bottom_type());
+            record_for_optimizer(new_proj);
+            set_map(proj, new_proj); // record it so ConnectionGraph::find_inst_mem() can find it
+          }
+        };
+        init->for_each_narrow_mem_proj_with_new_uses(process_narrow_proj);
 
         // First, put on the worklist all Field edges from Connection Graph
         // which is more accurate than putting immediate users from Ideal Graph.
@@ -4518,7 +4539,7 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
       // finishes. For now we just try to split out the SR inputs of the merge.
       Node* parent = n->in(1);
       if (reducible_merges.member(n)) {
-        reduce_phi(n->as_Phi(), alloc_worklist, memnode_worklist);
+        reduce_phi(n->as_Phi(), alloc_worklist);
 #ifdef ASSERT
         if (VerifyReduceAllocationMerges) {
           reduced_merges.push(n);
@@ -4710,11 +4731,13 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
     }
     if (n->is_Phi() || n->is_ClearArray()) {
       // we don't need to do anything, but the users must be pushed
-    } else if (n->is_MemBar()) { // Initialize, MemBar nodes
-      // we don't need to do anything, but the users must be pushed
-      n = n->as_MemBar()->proj_out_or_null(TypeFunc::Memory);
-      if (n == nullptr) {
-        continue;
+    } else if (n->is_MemBar()) { // MemBar nodes
+      if (!n->is_Initialize()) { // memory projections for Initialize pushed below (so we get to all their uses)
+        // we don't need to do anything, but the users must be pushed
+        n = n->as_MemBar()->proj_out_or_null(TypeFunc::Memory);
+        if (n == nullptr) {
+          continue;
+        }
       }
     } else if (n->is_CallLeaf()) {
       // Runtime calls with narrow memory input (no MergeMem node)
@@ -4731,6 +4754,8 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
       // get the memory projection
       n = n->find_out_with(Op_SCMemProj);
       assert(n != nullptr && n->Opcode() == Op_SCMemProj, "memory projection required");
+    } else if (n->is_Proj()) {
+      assert(n->in(0)->is_Initialize(), "we only push memory projections for Initialize");
     } else {
 #ifdef ASSERT
       if (!n->is_Mem()) {
@@ -4774,6 +4799,11 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
         if (use->in(TypeFunc::Memory) == n) { // Ignore precedent edge
           memnode_worklist.append_if_missing(use);
         }
+      } else if (use->is_Proj()) {
+        assert(n->is_Initialize(), "We only push projections of Initialize");
+        if (use->as_Proj()->_con == TypeFunc::Memory) { // Ignore precedent edge
+          memnode_worklist.append_if_missing(use);
+        }
 #ifdef ASSERT
       } else if(use->is_Mem()) {
         assert(use->in(MemNode::Memory) != n, "EA: missing memory path");
@@ -4825,7 +4855,7 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
       // First, update mergemem by moving memory nodes to corresponding slices
       // if their type became more precise since this mergemem was created.
       while (mem->is_Mem()) {
-        const Type *at = igvn->type(mem->in(MemNode::Address));
+        const Type* at = igvn->type(mem->in(MemNode::Address));
         if (at != Type::TOP) {
           assert (at->isa_ptr() != nullptr, "pointer type required.");
           uint idx = (uint)_compile->get_alias_index(at->is_ptr());
@@ -4945,7 +4975,7 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
       record_for_optimizer(n);
     } else {
       assert(n->is_Allocate() || n->is_CheckCastPP() ||
-             n->is_AddP() || n->is_Phi(), "unknown node used for set_map()");
+             n->is_AddP() || n->is_Phi() || n->is_NarrowMemProj(), "unknown node used for set_map()");
     }
   }
 #if 0 // ifdef ASSERT

src/hotspot/share/opto/escape.hpp

@@ -563,8 +563,10 @@ class ConnectionGraph: public ArenaObj {
                         // Memory Phi    - most recent unique Phi split out
                         //                 from this Phi
                         // MemNode       - new memory input for this node
-                        // ChecCastPP    - allocation that this is a cast of
+                        // CheckCastPP   - allocation that this is a cast of
                         // allocation    - CheckCastPP of the allocation
+                        // NarrowMem     - newly created projection (type includes instance_id) from projection created
+                        //                 before EA
 
   // manage entries in _node_map
 
@@ -609,11 +611,11 @@ class ConnectionGraph: public ArenaObj {
   bool can_reduce_phi_check_inputs(PhiNode* ophi) const;
 
   void reduce_phi_on_field_access(Node* previous_addp, GrowableArray<Node *>  &alloc_worklist);
-  void reduce_phi_on_castpp_field_load(Node* castpp, GrowableArray<Node *>  &alloc_worklist, GrowableArray<Node *>  &memnode_worklist);
+  void reduce_phi_on_castpp_field_load(Node* castpp, GrowableArray<Node*> &alloc_worklist);
   void reduce_phi_on_cmp(Node* cmp);
   bool reduce_phi_on_safepoints(PhiNode* ophi);
   bool reduce_phi_on_safepoints_helper(Node* ophi, Node* cast, Node* selector, Unique_Node_List& safepoints);
-  void reduce_phi(PhiNode* ophi, GrowableArray<Node *>  &alloc_worklist, GrowableArray<Node *>  &memnode_worklist);
+  void reduce_phi(PhiNode* ophi, GrowableArray<Node*> &alloc_worklist);
 
   void set_not_scalar_replaceable(PointsToNode* ptn NOT_PRODUCT(COMMA const char* reason)) const {
 #ifndef PRODUCT