changeset 54221:9d12c8ada27d lworld

8215477: [lworld] C2 should avoid value type allocations when inlining incrementally Reviewed-by: thartmann
author roland
date Fri, 15 Feb 2019 16:59:57 +0100
parents c2d0104c02f2
children be3f66d7e53b
files src/hotspot/share/opto/callGenerator.cpp src/hotspot/share/opto/callnode.cpp src/hotspot/share/opto/cfgnode.cpp src/hotspot/share/opto/cfgnode.hpp src/hotspot/share/opto/compile.cpp src/hotspot/share/opto/graphKit.cpp src/hotspot/share/opto/graphKit.hpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java
diffstat 9 files changed, 82 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/opto/callGenerator.cpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/callGenerator.cpp	Fri Feb 15 16:59:57 2019 +0100
@@ -183,7 +183,7 @@
       call->set_method_handle_invoke(true);
     }
   }
-  kit.set_arguments_for_java_call(call);
+  kit.set_arguments_for_java_call(call, is_late_inline());
   if (kit.stopped()) {
     return kit.transfer_exceptions_into_jvms();
   }
--- a/src/hotspot/share/opto/callnode.cpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/callnode.cpp	Fri Feb 15 16:59:57 2019 +0100
@@ -1474,6 +1474,7 @@
   if (can_reshape && in(AllocateNode::ValueNode) != NULL &&
       outcnt() != 0 && result_cast() == NULL) {
     // Remove allocation by replacing the projection nodes with its inputs
+    InitializeNode* init = initialization();
     PhaseIterGVN* igvn = phase->is_IterGVN();
     CallProjections* projs = extract_projections(true, false);
     assert(projs->nb_resproj <= 1, "unexpected number of results");
@@ -1499,6 +1500,16 @@
       igvn->replace_node(projs->resproj[0], phase->C->top());
     }
     igvn->replace_node(this, phase->C->top());
+    if (init != NULL) {
+      Node* ctrl_proj = init->proj_out_or_null(TypeFunc::Control);
+      Node* mem_proj = init->proj_out_or_null(TypeFunc::Memory);
+      if (ctrl_proj != NULL) {
+        igvn->replace_node(ctrl_proj, init->in(TypeFunc::Control));
+      }
+      if (mem_proj != NULL) {
+        igvn->replace_node(mem_proj, init->in(TypeFunc::Memory));
+      }
+    }
     return NULL;
   }
 
--- a/src/hotspot/share/opto/cfgnode.cpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/cfgnode.cpp	Fri Feb 15 16:59:57 2019 +0100
@@ -373,7 +373,7 @@
   return true; // The Region node is unreachable - it is dead.
 }
 
-bool RegionNode::try_clean_mem_phi(PhaseGVN *phase) {
+Node* PhiNode::try_clean_mem_phi(PhaseGVN *phase) {
   // Incremental inlining + PhaseStringOpts sometimes produce:
   //
   // cmpP with 1 top input
@@ -393,27 +393,25 @@
   // code below replaces the Phi with the MergeMem so that the Region
   // is simplified.
 
-  PhiNode* phi = has_unique_phi();
-  if (phi && phi->type() == Type::MEMORY && req() == 3 && phi->is_diamond_phi(true)) {
+  if (type() == Type::MEMORY && is_diamond_phi(true)) {
     MergeMemNode* m = NULL;
-    assert(phi->req() == 3, "same as region");
+    assert(req() == 3, "same as region");
     for (uint i = 1; i < 3; ++i) {
-      Node *mem = phi->in(i);
+      Node *mem = in(i);
       if (mem && mem->is_MergeMem() && in(i)->outcnt() == 1) {
         // Nothing is control-dependent on path #i except the region itself.
         m = mem->as_MergeMem();
         uint j = 3 - i;
-        Node* other = phi->in(j);
+        Node* other = in(j);
         if (other && other == m->base_memory()) {
           // m is a successor memory to other, and is not pinned inside the diamond, so push it out.
           // This will allow the diamond to collapse completely.
-          phase->is_IterGVN()->replace_node(phi, m);
-          return true;
+          return m;
         }
       }
     }
   }
-  return false;
+  return NULL;
 }
 
 //------------------------------Ideal------------------------------------------
@@ -428,8 +426,15 @@
   bool has_phis = false;
   if (can_reshape) {            // Need DU info to check for Phi users
     has_phis = (has_phi() != NULL);       // Cache result
-    if (has_phis && try_clean_mem_phi(phase)) {
-      has_phis = false;
+    if (has_phis) {
+      PhiNode* phi = has_unique_phi();
+      if (phi != NULL) {
+        Node* m = phi->try_clean_mem_phi(phase);
+        if (m != NULL) {
+          phase->is_IterGVN()->replace_node(phi, m);
+          has_phis = false;
+        }
+      }
     }
 
     if (!has_phis) {            // No Phi users?  Nothing merging?
@@ -1331,6 +1336,14 @@
     if (id != NULL)  return id;
   }
 
+  if (phase->is_IterGVN()) {
+    Node* m = try_clean_mem_phi(phase);
+    if (m != NULL) {
+      return m;
+    }
+  }
+
+
   return this;                     // No identity
 }
 
--- a/src/hotspot/share/opto/cfgnode.hpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/cfgnode.hpp	Fri Feb 15 16:59:57 2019 +0100
@@ -95,7 +95,6 @@
   virtual Node* Identity(PhaseGVN* phase);
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
   virtual const RegMask &out_RegMask() const;
-  bool try_clean_mem_phi(PhaseGVN *phase);
   bool optimize_trichotomy(PhaseIterGVN* igvn);
 };
 
@@ -215,6 +214,7 @@
            inst_offset() == offset &&
            type()->higher_equal(tp);
   }
+  Node* try_clean_mem_phi(PhaseGVN *phase);
 
   virtual const Type* Value(PhaseGVN* phase) const;
   virtual Node* Identity(PhaseGVN* phase);
--- a/src/hotspot/share/opto/compile.cpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/compile.cpp	Fri Feb 15 16:59:57 2019 +0100
@@ -2077,6 +2077,10 @@
     vt->make_scalar_in_safepoints(&igvn);
     if (vt->is_ValueTypePtr()) {
       igvn.replace_node(vt, vt->get_oop());
+    } else {
+      if (vt->outcnt() == 0) {
+        igvn.remove_dead_node(vt);
+      }
     }
   }
   _value_type_nodes = NULL;
--- a/src/hotspot/share/opto/graphKit.cpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/graphKit.cpp	Fri Feb 15 16:59:57 2019 +0100
@@ -1781,7 +1781,7 @@
 
 //-------------------------set_arguments_for_java_call-------------------------
 // Arguments (pre-popped from the stack) are taken from the JVMS.
-void GraphKit::set_arguments_for_java_call(CallJavaNode* call) {
+void GraphKit::set_arguments_for_java_call(CallJavaNode* call, bool incremental_inlining) {
   // Add the call arguments:
   const TypeTuple* domain = call->tf()->domain_sig();
   ExtendedSignature sig_cc = ExtendedSignature(call->method()->get_sig_cc(), SigEntryFilter());
@@ -1805,7 +1805,11 @@
       continue;
     } else if (arg->is_ValueType()) {
       // Pass value type argument via oop to callee
-      arg = arg->as_ValueType()->allocate(this)->get_oop();
+      if (!incremental_inlining) {
+        arg = arg->as_ValueType()->allocate(this)->get_oop();
+      } else {
+        arg = ValueTypePtrNode::make_from_value_type(this, arg->as_ValueType(), false);
+      }
     }
     call->init_req(idx++, arg);
     // Skip reserved arguments
@@ -3693,6 +3697,7 @@
     MergeMemNode* minit_in = MergeMemNode::make(malloc);
     init->set_req(InitializeNode::Memory, minit_in);
     record_for_igvn(minit_in); // fold it up later, if possible
+    _gvn.set_type(minit_in, Type::MEMORY);
     Node* minit_out = memory(rawidx);
     assert(minit_out->is_Proj() && minit_out->in(0) == init, "");
     if (oop_type->isa_aryptr()) {
--- a/src/hotspot/share/opto/graphKit.hpp	Thu Feb 14 11:02:07 2019 -0800
+++ b/src/hotspot/share/opto/graphKit.hpp	Fri Feb 15 16:59:57 2019 +0100
@@ -693,7 +693,7 @@
 
   // Fill in argument edges for the call from argument(0), argument(1), ...
   // (The next step is to call set_edges_for_java_call.)
-  void  set_arguments_for_java_call(CallJavaNode* call);
+  void  set_arguments_for_java_call(CallJavaNode* call, bool incremental_inlining = false);
 
   // Fill in non-argument edges for the call.
   // Transform the call, and update the basics: control, i_o, memory.
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java	Thu Feb 14 11:02:07 2019 -0800
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestBasicFunctionality.java	Fri Feb 15 16:59:57 2019 +0100
@@ -755,4 +755,28 @@
         staticVal3.verify(vt);
         va[0].verify(vt);
     }
+
+    // Merge value types created from two branches
+
+    private Object test36_helper(Object v) {
+        return v;
+    }
+
+    @Test(failOn = ALLOC + STORE + TRAP)
+    public long test36(boolean b) {
+        Object o;
+        if (b) {
+            o = test36_helper(MyValue1.createWithFieldsInline(rI, rL));
+        } else {
+            o = test36_helper(MyValue1.createWithFieldsDontInline(rI + 1, rL + 1));
+        }
+        MyValue1 v = (MyValue1)o;
+        return v.hash();
+    }
+
+    @DontCompile
+    public void test36_verifier(boolean warmup) {
+        Asserts.assertEQ(test36(true), hash());
+        Asserts.assertEQ(test36(false), hash(rI + 1, rL + 1));
+    }
 }
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Thu Feb 14 11:02:07 2019 -0800
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/ValueTypeTest.java	Fri Feb 15 16:59:57 2019 +0100
@@ -182,7 +182,7 @@
         if (TEST_C1) {
             return 1;
         } else {
-            return 5;
+            return 6;
         }
     }
 
@@ -242,6 +242,14 @@
                 "-XX:+ValueTypePassFieldsAsArgs",
                 "-XX:-ValueTypeReturnedAsFields",
                 "-XX:+StressValueTypePassFieldsAsArgs"};
+        case 5: return new String[] {
+                "-XX:+AlwaysIncrementalInline",
+                "-XX:ValueArrayElemMaxFlatOops=-1",
+                "-XX:ValueArrayElemMaxFlatSize=-1",
+                "-XX:+ValueArrayFlatten",
+                "-XX:ValueFieldMaxFlatSize=-1",
+                "-XX:-ValueTypePassFieldsAsArgs",
+                "-XX:-ValueTypeReturnedAsFields"};
         }
 
         return null;