address CR comments

src/hotspot/share/opto/escape.cpp

@@ -466,9 +466,9 @@ bool ConnectionGraph::can_reduce_phi_check_inputs(PhiNode* ophi) const {
   int nof_input_phi_nodes = 0;
   for (uint i = 1; i < ophi->req(); i++) {
     if (ophi->in(i)->is_Phi()) {
-      // ignore phi node with more than one input phi nodes
+      // ignore phi node with more than one input phi node
       if (++nof_input_phi_nodes > 1) {
-        NOT_PRODUCT(if (TraceReduceAllocationMerges && !found_sr_allocate) tty->print_cr("Cannot reduce Phi %d. More than one input phi node.", ophi->_idx);)
+        NOT_PRODUCT(if (TraceReduceAllocationMerges) tty->print_cr("Cannot reduce Phi %d. More than one input phi node.", ophi->_idx);)
         return false;
       }
     }
@@ -538,7 +538,7 @@ bool ConnectionGraph::has_been_reduced(PhiNode* n, SafePointNode* sfpt) const {
 bool ConnectionGraph::can_reduce_check_users(Node* n, uint phi_nest_level) const {
   for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) {
     Node* use = n->fast_out(i);
-    // Skip Phi -> Phi -> SafePoints and allow only Phi -> SafePoints, Phi -> CastPP -> SafePoints, 
+    // Skip Phi -> Phi -> SafePoints and allow only Phi -> SafePoints and Phi -> CastPP -> SafePoints
     if (use->is_SafePoint() && (!n->is_Phi() || phi_nest_level < 1)) {
       if (use->is_Call() && use->as_Call()->has_non_debug_use(n)) {
         NOT_PRODUCT(if (TraceReduceAllocationMerges) tty->print_cr("Cannot reduce Phi %d on invocation %d. Call has non_debug_use().", n->_idx, _invocation);)
@@ -572,8 +572,6 @@ bool ConnectionGraph::can_reduce_check_users(Node* n, uint phi_nest_level) const
       } else if (!can_reduce_phi_check_inputs(use->as_Phi()) || !can_reduce_check_users(use->as_Phi(), phi_nest_level+1)) {
         return false;
       }
-      //test
-      tty->print_cr("Can reduce parent Phi %d child phi %d .", n->_idx, use->_idx);
     } else if (use->is_CastPP()) {
       const Type* cast_t = _igvn->type(use);
       if (cast_t == nullptr || cast_t->make_ptr()->isa_instptr() == nullptr) {
@@ -1289,7 +1287,6 @@ bool ConnectionGraph::reduce_phi_on_safepoints_helper(Node* ophi, Node* cast, No
 }
 
 void ConnectionGraph::reduce_phi(PhiNode* ophi, GrowableArray<Node *>  &alloc_worklist, GrowableArray<Node *>  &memnode_worklist, Unique_Node_List &reducible_merges) {
-
   Unique_Node_List nested_phis;
   // Collect nested phi nodes first because the graph will change while splitting the child/nested phi node.
   for (DUIterator_Fast imax, i = ophi->fast_outs(imax); i < imax; i++) {
@@ -1299,12 +1296,6 @@ void ConnectionGraph::reduce_phi(PhiNode* ophi, GrowableArray<Node *>  &alloc_wo
       nested_phis.push(use);
     }
   }
-
-  if (nested_phis.size() > 1) {
-    tty->print_cr("parent Phi nodes %d", nested_phis.size());
-    ophi->dump(3);
-    ophi->dump(-3);
-  }
 
   // Splitting through the child phi nodes ahead of parent phi nodes in nested scenarios is crucial.
   // Ensure that the splits are applied to the load fields of child phi nodes before the parent phi nodes take place.
@@ -4472,7 +4463,7 @@ void ConnectionGraph::split_unique_types(GrowableArray<Node *>  &alloc_worklist,
 #ifdef ASSERT
         ptnode_adr(get_addp_base(n)->_idx)->dump();
         ptnode_adr(n->_idx)->dump();
-        assert(addp_base->is_Phi() || (jobj != nullptr && jobj != phantom_obj), "escaped allocation");
+        assert(jobj != nullptr && jobj != phantom_obj, "escaped allocation");
 #endif
         _compile->record_failure(_invocation > 0 ? C2Compiler::retry_no_iterative_escape_analysis() : C2Compiler::retry_no_escape_analysis());
         return;

src/hotspot/share/opto/memnode.cpp

@@ -1552,21 +1552,21 @@ Node* LoadNode::get_region_of_split_through_base_phi(Node *base) {
   bool base_is_phi = (base != nullptr) && base->is_Phi();
   Node* region = nullptr;  
   if (!base_is_phi) {
-    assert(mem->is_Phi(), "sanity");
+    assert(mem->is_Phi(), "memory node should be a Phi");
     region = mem->in(0);
     // Skip if the region dominates some control edge of the address.
     // 'region' dominates 'address' if its control edge and control edges
     // of all its inputs dominate or equal to address's control edge.
     if (!MemNode::all_controls_dominate(address, region))
       return nullptr;
   } else if (!mem->is_Phi()) {
-    assert(base_is_phi, "sanity");
+    assert(base_is_phi, "base node should be a Phi");
     region = base->in(0);
     // Skip if the region dominates some control edge of the memory.
     if (!MemNode::all_controls_dominate(mem, region))
       return nullptr;
   } else if (base->in(0) != mem->in(0)) {
-    assert(base_is_phi && mem->is_Phi(), "sanity");
+    assert(base_is_phi && mem->is_Phi(), "base and memory nodes should be Phi");
     if (MemNode::all_controls_dominate(mem, base->in(0))) {
       region = base->in(0);
     } else if (MemNode::all_controls_dominate(address, mem->in(0))) {
@@ -1575,7 +1575,7 @@ Node* LoadNode::get_region_of_split_through_base_phi(Node *base) {
       return nullptr; // complex graph
     }
   } else {
-    assert(base->in(0) == mem->in(0), "sanity");
+    assert(base->in(0) == mem->in(0), "base and memory nodes should have the same region");
     region = mem->in(0);
   }
   return region;
@@ -1604,7 +1604,7 @@ static bool can_split_through_phi_helper(Node *base, Node *mem) {
 // Phi *base*. This method is essentially a copy of the validations performed
 // by 'split_through_phi'. The first use of this method was in EA code as part
 // of simplification of allocation merges.
-// Some differences from original method (split_through_phi):
+// The difference from the original method (split_through_phi) is:
 //  - If base->is_CastPP(): base = base->in(1)
 bool LoadNode::can_split_through_phi_base(PhaseGVN* phase, bool nested) {
   Node* mem        = in(Memory);
@@ -1643,7 +1643,6 @@ Node* LoadNode::get_memory_node_for_nestedphi_after_split(PhaseGVN* phase, Node
   Node* mem        = in(Memory);
   Node* region = get_region_of_split_through_base_phi(base);
   if (region == nullptr) {
-    tty->print_cr("region null");
     return nullptr;
   }
 

src/hotspot/share/opto/memnode.hpp

@@ -248,9 +248,9 @@ class LoadNode : public MemNode {
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
 
   // Return true if it's possible to split the Load through a Phi merging the bases
-   bool can_split_through_phi_base(PhaseGVN *phase, bool nested = false);
+  bool can_split_through_phi_base(PhaseGVN *phase, bool nested = false);
 
-  // Returns a memory node for load node after spliting through nestedphi node
+  // Returns a memory node for load node after splitting through nestedphi node
   // Calling code is responsible to clean up (remove_dead_node) after use
   Node* get_memory_node_for_nestedphi_after_split(PhaseGVN* phase, Node *base, uint parent_idx);
   Node* get_region_of_split_through_base_phi(Node *base);

test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/NestedPhiAndRematerializeTests.java → test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/AllocationMergesNestedPhiTests.java

@@ -32,9 +32,9 @@
  * @summary Tests that C2 can correctly scalar replace some object allocation merges.
  * @library /test/lib /
  * @requires vm.debug == true & vm.flagless & vm.bits == 64 & vm.compiler2.enabled & vm.opt.final.EliminateAllocations
- * @run driver compiler.c2.irTests.scalarReplacement.NestedPhiAndRematerializeTests
+ * @run driver compiler.c2.irTests.scalarReplacement.AllocationMergesNestedPhiTests
  */
-public class NestedPhiAndRematerializeTests {
+public class AllocationMergesNestedPhiTests {
     private int invocations = 0;
     private static Point global_escape = new Point(2022, 2023);
 

test/micro/org/openjdk/bench/vm/compiler/NestedPhiAndRematerialize.java → test/micro/org/openjdk/bench/vm/compiler/AllocationMergesNestedPhi.java

@@ -34,7 +34,7 @@
 @Warmup(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
 @Measurement(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
 @Fork(value = 3)
-public abstract class NestedPhiAndRematerialize {
+public abstract class AllocationMergesNestedPhi {
     private static final int SIZE        = 1000000;
     private static final boolean cond1[] = new boolean[SIZE];
     private static final boolean cond2[] = new boolean[SIZE];
@@ -453,14 +453,14 @@ public void testMultiParentPhi_runner(Blackhole bh) {
         "-XX:+UseTLAB",
         "-XX:-ReduceAllocationMerges",
     })
-    public static class NopRAM extends NestedPhiAndRematerialize {
+    public static class NopRAM extends AllocationMergesNestedPhi {
     }
 
     @Fork(value = 3, jvmArgsPrepend = {
         "-XX:+UnlockDiagnosticVMOptions",
         "-XX:+ReduceAllocationMerges",
     })
-    public static class YesRAM extends NestedPhiAndRematerialize {
+    public static class YesRAM extends AllocationMergesNestedPhi {
     }