changeset 13050:314ea36f0728 mvt

8183137: [MVT] C2 doesn't eliminate redundant value type allocations inside loops Reviewed-by: thartmann, vlivanov
author roland
date Tue, 18 Jul 2017 11:38:16 +0200
parents ed323fd47c0f
children cbbd2c7f3259
files src/share/vm/opto/callGenerator.cpp src/share/vm/opto/cfgnode.cpp src/share/vm/opto/classes.hpp src/share/vm/opto/compile.cpp src/share/vm/opto/compile.hpp src/share/vm/opto/doCall.cpp src/share/vm/opto/memnode.cpp src/share/vm/opto/node.cpp src/share/vm/opto/node.hpp src/share/vm/opto/parse1.cpp src/share/vm/opto/phaseX.cpp src/share/vm/opto/valuetypenode.cpp src/share/vm/opto/valuetypenode.hpp test/compiler/valhalla/valuetypes/ValueCapableClass2.java test/compiler/valhalla/valuetypes/ValueTypeTestBench.java
diffstat 15 files changed, 527 insertions(+), 211 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/opto/callGenerator.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/callGenerator.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -496,11 +496,12 @@
 
   if (return_type->is_valuetype() && return_type != C->env()->___Value_klass()) {
     if (result->is_ValueType()) {
+      ValueTypeNode* vt = result->as_ValueType();
       if (!call->tf()->returns_value_type_as_fields()) {
-        result = result->as_ValueType()->allocate(&kit);
+        result = vt->allocate(&kit);
+        result = C->initial_gvn()->transform(new ValueTypePtrNode(vt, result, C));
       } else {
         // Return of multiple values (the fields of a value type)
-        ValueTypeNode* vt = result->as_ValueType();
         vt->replace_call_results(call, C);
       }
     } else {
--- a/src/share/vm/opto/cfgnode.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/cfgnode.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -1628,16 +1628,19 @@
 
   // If all inputs are value types, push the value type node down through the
   // phi because value type nodes should be merged through their input values.
-  bool all_vt = true;
-  for (uint i = 1; i < req() && all_vt; ++i) {
-    all_vt = in(i) && in(i)->is_ValueType();
-  }
-  if (req() > 2 && all_vt) {
-    ValueTypeNode* vt = in(1)->as_ValueType()->clone_with_phis(phase, in(0));
-    for (uint i = 2; i < req(); ++i) {
-      vt->merge_with(phase, in(i)->as_ValueType(), i, i == (req()-1));
+  if (req() > 2 && in(1) != NULL && in(1)->is_ValueTypeBase() && (can_reshape || in(1)->is_ValueType())) {
+    int opcode = in(1)->Opcode();
+    uint i = 2;
+    for (; i < req() && in(i) && in(i)->is_ValueTypeBase(); i++) {
+      assert(in(i)->Opcode() == opcode, "mixing pointers and values?");
     }
-    return vt;
+    if (i == req()) {
+      ValueTypeBaseNode* vt = in(1)->as_ValueTypeBase()->clone_with_phis(phase, in(0));
+      for (uint i = 2; i < req(); ++i) {
+        vt->merge_with(phase, in(i)->as_ValueTypeBase(), i, i == (req()-1));
+      }
+      return vt;
+    }
   }
 
   Node *top = phase->C->top();
--- a/src/share/vm/opto/classes.hpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/classes.hpp	Tue Jul 18 11:38:16 2017 +0200
@@ -284,6 +284,7 @@
 macro(XorI)
 macro(XorL)
 macro(ValueType)
+macro(ValueTypePtr)
 macro(Vector)
 macro(AddVB)
 macro(AddVS)
--- a/src/share/vm/opto/compile.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/compile.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -404,6 +404,12 @@
       remove_expensive_node(n);
     }
   }
+  for (int i = value_type_ptr_count() - 1; i >= 0; i--) {
+    ValueTypePtrNode* vtptr = value_type_ptr(i);
+    if (!useful.member(vtptr)) {
+      remove_value_type_ptr(vtptr);
+    }
+  }
   // clean up the late inline lists
   remove_useless_late_inlines(&_string_late_inlines, useful);
   remove_useless_late_inlines(&_boxing_late_inlines, useful);
@@ -1177,6 +1183,7 @@
   _predicate_opaqs = new(comp_arena()) GrowableArray<Node*>(comp_arena(), 8,  0, NULL);
   _expensive_nodes = new(comp_arena()) GrowableArray<Node*>(comp_arena(), 8,  0, NULL);
   _range_check_casts = new(comp_arena()) GrowableArray<Node*>(comp_arena(), 8,  0, NULL);
+  _value_type_ptr_nodes = new(comp_arena()) GrowableArray<ValueTypePtrNode*>(comp_arena(), 8,  0, NULL);
   register_library_intrinsics();
 }
 
@@ -1962,6 +1969,23 @@
   assert(range_check_cast_count() == 0, "should be empty");
 }
 
+void Compile::add_value_type_ptr(ValueTypePtrNode* n) {
+  assert(!_value_type_ptr_nodes->contains(n), "duplicate entry");
+  _value_type_ptr_nodes->append(n);
+}
+
+void Compile::process_value_type_ptr_nodes(PhaseIterGVN &igvn) {
+  for (int i = value_type_ptr_count(); i > 0; i--) {
+    ValueTypePtrNode* vtptr = value_type_ptr(i-1);
+    // once all inlining is over otherwise debug info can get
+    // inconsistent
+    vtptr->make_scalar_in_safepoints(igvn.C->root(), &igvn);
+    igvn.replace_node(vtptr, vtptr->get_oop());
+  }
+  assert(value_type_ptr_count() == 0, "should be empty");
+  igvn.optimize();
+}
+
 // StringOpts and late inlining of string methods
 void Compile::inline_string_calls(bool parse_time) {
   {
@@ -2209,6 +2233,10 @@
     igvn.optimize();
   }
 
+  if (value_type_ptr_count() > 0) {
+    process_value_type_ptr_nodes(igvn);
+  }
+
   // Perform escape analysis
   if (_do_escape_analysis && ConnectionGraph::has_candidates(this)) {
     if (has_loops()) {
@@ -3438,12 +3466,16 @@
   }
   case Op_ValueType: {
     ValueTypeNode* vt = n->as_ValueType();
-    vt->make_scalar_in_safepoints(this);
+    vt->make_scalar_in_safepoints(root(), NULL);
     if (vt->outcnt() == 0) {
       vt->disconnect_inputs(NULL, this);
     }
     break;
   }
+  case Op_ValueTypePtr: {
+    ShouldNotReachHere();
+    break;
+  }
   default:
     assert( !n->is_Call(), "" );
     assert( !n->is_Mem(), "" );
--- a/src/share/vm/opto/compile.hpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/compile.hpp	Tue Jul 18 11:38:16 2017 +0200
@@ -83,6 +83,7 @@
 class TypePtr;
 class TypeOopPtr;
 class TypeFunc;
+class ValueTypePtrNode;
 class Unique_Node_List;
 class nmethod;
 class WarmCallInfo;
@@ -415,6 +416,7 @@
   GrowableArray<Node*>* _predicate_opaqs;       // List of Opaque1 nodes for the loop predicates.
   GrowableArray<Node*>* _expensive_nodes;       // List of nodes that are expensive to compute and that we'd better not let the GVN freely common
   GrowableArray<Node*>* _range_check_casts;     // List of CastII nodes with a range check dependency
+  GrowableArray<ValueTypePtrNode*>* _value_type_ptr_nodes; // List of ValueTypePtr nodes
   ConnectionGraph*      _congraph;
 #ifndef PRODUCT
   IdealGraphPrinter*    _printer;
@@ -807,6 +809,16 @@
   // Remove all range check dependent CastIINodes.
   void  remove_range_check_casts(PhaseIterGVN &igvn);
 
+  void add_value_type_ptr(ValueTypePtrNode* n);
+  void remove_value_type_ptr(ValueTypePtrNode* n) {
+    if (_value_type_ptr_nodes->contains(n)) {
+      _value_type_ptr_nodes->remove(n);
+    }
+  }
+  ValueTypePtrNode* value_type_ptr(int idx) const { return _value_type_ptr_nodes->at(idx);  }
+  int   value_type_ptr_count()       const { return _value_type_ptr_nodes->length(); }
+  void  process_value_type_ptr_nodes(PhaseIterGVN &igvn);
+
   // remove the opaque nodes that protect the predicates so that the unused checks and
   // uncommon traps will be eliminated from the graph.
   void cleanup_loop_predicates(PhaseIterGVN &igvn);
--- a/src/share/vm/opto/doCall.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/doCall.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -670,8 +670,10 @@
             assert(ctype == C->env()->___Value_klass(), "unexpected value type klass");
             Node* retnode = pop();
             assert(retnode->is_ValueType(), "inconsistent");
-            retnode = retnode->as_ValueType()->allocate(this);
-            push(retnode);
+            ValueTypeNode* vt = retnode->as_ValueType();
+            Node* alloc = vt->allocate(this);
+            Node* vtptr = _gvn.transform(new ValueTypePtrNode(vt, alloc, C));
+            push(vtptr);
           }
         } else {
           assert(rt == ct, "unexpected mismatch: rt=%s, ct=%s", type2name(rt), type2name(ct));
--- a/src/share/vm/opto/memnode.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/memnode.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -42,6 +42,7 @@
 #include "opto/narrowptrnode.hpp"
 #include "opto/phaseX.hpp"
 #include "opto/regmask.hpp"
+#include "opto/valuetypenode.hpp"
 #include "utilities/copy.hpp"
 
 // Portions of code courtesy of Clifford Click
@@ -1092,6 +1093,20 @@
 //------------------------------Identity---------------------------------------
 // Loads are identity if previous store is to same address
 Node* LoadNode::Identity(PhaseGVN* phase) {
+  // Loading from a ValueTypePtr? The ValueTypePtr has the values of
+  // all fields as input. Look for the field with matching offset.
+  Node* addr = in(Address);
+  intptr_t offset;
+  Node* base = AddPNode::Ideal_base_and_offset(addr, phase, offset);
+  if (base != NULL && base->is_ValueTypePtr()) {
+    Node* value = base->as_ValueTypePtr()->field_value_by_offset((int)offset, true);
+    if (bottom_type()->isa_narrowoop()) {
+      assert(!phase->type(value)->isa_narrowoop(), "should already be decoded");
+      value = phase->transform(new EncodePNode(value, bottom_type()));
+    }
+    return value;
+  }
+
   // If the previous store-maker is the right kind of Store, and the store is
   // to the same address, then we are equal to the value stored.
   Node* mem = in(Memory);
--- a/src/share/vm/opto/node.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/node.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -504,6 +504,9 @@
   if (cast != NULL && cast->has_range_check()) {
     C->add_range_check_cast(cast);
   }
+  if (n->is_ValueTypePtr()) {
+    C->add_value_type_ptr(n->as_ValueTypePtr());
+  }
 
   n->set_idx(C->next_unique()); // Get new unique index as well
   debug_only( n->verify_construction() );
@@ -612,6 +615,9 @@
   if (cast != NULL && cast->has_range_check()) {
     compile->remove_range_check_cast(cast);
   }
+  if (is_ValueTypePtr()) {
+    compile->remove_value_type_ptr(as_ValueTypePtr());
+  }
 
   if (is_SafePoint()) {
     as_SafePoint()->delete_replaced_nodes();
@@ -1352,6 +1358,9 @@
       if (cast != NULL && cast->has_range_check()) {
         igvn->C->remove_range_check_cast(cast);
       }
+      if (dead->is_ValueTypePtr()) {
+        igvn->C->remove_value_type_ptr(dead->as_ValueTypePtr());
+      }
       igvn->C->record_dead_node(dead->_idx);
       // Kill all inputs to the dead guy
       for (uint i=0; i < dead->req(); i++) {
--- a/src/share/vm/opto/node.hpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/node.hpp	Tue Jul 18 11:38:16 2017 +0200
@@ -142,6 +142,7 @@
 class TypeNode;
 class UnlockNode;
 class ValueTypeNode;
+class ValueTypeBaseNode;
 class VectorNode;
 class LoadVectorNode;
 class StoreVectorNode;
@@ -664,7 +665,9 @@
       DEFINE_CLASS_ID(EncodeNarrowPtr, Type, 6)
         DEFINE_CLASS_ID(EncodeP, EncodeNarrowPtr, 0)
         DEFINE_CLASS_ID(EncodePKlass, EncodeNarrowPtr, 1)
-      DEFINE_CLASS_ID(ValueType, Type, 7)
+      DEFINE_CLASS_ID(ValueTypeBase, Type, 7)
+        DEFINE_CLASS_ID(ValueType, ValueTypeBase, 0)
+        DEFINE_CLASS_ID(ValueTypePtr, ValueTypeBase, 1)
 
     DEFINE_CLASS_ID(Proj,  Node, 3)
       DEFINE_CLASS_ID(CatchProj, Proj, 0)
@@ -856,6 +859,8 @@
   DEFINE_CLASS_QUERY(Sub)
   DEFINE_CLASS_QUERY(Type)
   DEFINE_CLASS_QUERY(ValueType)
+  DEFINE_CLASS_QUERY(ValueTypeBase)
+  DEFINE_CLASS_QUERY(ValueTypePtr)
   DEFINE_CLASS_QUERY(Vector)
   DEFINE_CLASS_QUERY(LoadVector)
   DEFINE_CLASS_QUERY(StoreVector)
--- a/src/share/vm/opto/parse1.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/parse1.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -2125,7 +2125,7 @@
     return NULL;
   }
 
-  ValueTypeNode* vt = o->isa_ValueType();
+  ValueTypeBaseNode* vt = o->isa_ValueType();
   if (vt != NULL) {
     // Value types are merged by merging their field values.
     // Create a cloned ValueTypeNode with phi inputs that
--- a/src/share/vm/opto/phaseX.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/phaseX.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -1420,6 +1420,9 @@
       if (cast != NULL && cast->has_range_check()) {
         C->remove_range_check_cast(cast);
       }
+      if (dead->is_ValueTypePtr()) {
+        C->remove_value_type_ptr(dead->as_ValueTypePtr());
+      }
     }
   } // while (_stack.is_nonempty())
 }
--- a/src/share/vm/opto/valuetypenode.cpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/valuetypenode.cpp	Tue Jul 18 11:38:16 2017 +0200
@@ -30,6 +30,170 @@
 #include "opto/valuetypenode.hpp"
 #include "opto/phaseX.hpp"
 
+// Clones the values type to handle control flow merges involving multiple value types.
+// The inputs are replaced by PhiNodes to represent the merged values for the given region.
+ValueTypeBaseNode* ValueTypeBaseNode::clone_with_phis(PhaseGVN* gvn, Node* region) {
+  assert(!has_phi_inputs(region), "already cloned with phis");
+  ValueTypeBaseNode* vt = clone()->as_ValueTypeBase();
+
+  // Create a PhiNode for merging the oop values
+  const TypeValueTypePtr* vtptr = value_type_ptr();
+  PhiNode* oop = PhiNode::make(region, vt->get_oop(), vtptr);
+  gvn->set_type(oop, vtptr);
+  vt->set_oop(oop);
+
+  // Create a PhiNode each for merging the field values
+  for (uint i = 0; i < vt->field_count(); ++i) {
+    ciType* type = vt->field_type(i);
+    Node*  value = vt->field_value(i);
+    if (type->is_valuetype()) {
+      // Handle flattened value type fields recursively
+      value = value->as_ValueType()->clone_with_phis(gvn, region);
+    } else {
+      const Type* phi_type = Type::get_const_type(type);
+      value = PhiNode::make(region, value, phi_type);
+      gvn->set_type(value, phi_type);
+    }
+    vt->set_field_value(i, value);
+  }
+  gvn->set_type(vt, vt->bottom_type());
+  return vt;
+}
+
+// Checks if the inputs of the ValueBaseTypeNode were replaced by PhiNodes
+// for the given region (see ValueBaseTypeNode::clone_with_phis).
+bool ValueTypeBaseNode::has_phi_inputs(Node* region) {
+  // Check oop input
+  bool result = get_oop()->is_Phi() && get_oop()->as_Phi()->region() == region;
+#ifdef ASSERT
+  if (result) {
+    // Check all field value inputs for consistency
+    for (uint i = Oop; i < field_count(); ++i) {
+      Node* n = in(i);
+      if (n->is_ValueTypeBase()) {
+        assert(n->as_ValueTypeBase()->has_phi_inputs(region), "inconsistent phi inputs");
+      } else {
+        assert(n->is_Phi() && n->as_Phi()->region() == region, "inconsistent phi inputs");
+      }
+    }
+  }
+#endif
+  return result;
+}
+
+// Merges 'this' with 'other' by updating the input PhiNodes added by 'clone_with_phis'
+ValueTypeBaseNode* ValueTypeBaseNode::merge_with(PhaseGVN* gvn, const ValueTypeBaseNode* other, int pnum, bool transform) {
+  // Merge oop inputs
+  PhiNode* phi = get_oop()->as_Phi();
+  phi->set_req(pnum, other->get_oop());
+  if (transform) {
+    set_oop(gvn->transform(phi));
+    gvn->record_for_igvn(phi);
+  }
+  // Merge field values
+  for (uint i = 0; i < field_count(); ++i) {
+    Node* val1 =        field_value(i);
+    Node* val2 = other->field_value(i);
+    if (val1->isa_ValueType()) {
+      val1->as_ValueType()->merge_with(gvn, val2->as_ValueType(), pnum, transform);
+    } else {
+      assert(val1->is_Phi(), "must be a phi node");
+      assert(!val2->is_ValueType(), "inconsistent merge values");
+      val1->set_req(pnum, val2);
+    }
+    if (transform) {
+      set_field_value(i, gvn->transform(val1));
+      gvn->record_for_igvn(val1);
+    }
+  }
+  return this;
+}
+
+Node* ValueTypeBaseNode::field_value(uint index) const {
+  assert(index < field_count(), "index out of bounds");
+  return in(Values + index);
+}
+
+// Get the value of the field at the given offset.
+// If 'recursive' is true, flattened value type fields will be resolved recursively.
+Node* ValueTypeBaseNode::field_value_by_offset(int offset, bool recursive) const {
+  // If the field at 'offset' belongs to a flattened value type field, 'index' refers to the
+  // corresponding ValueTypeNode input and 'sub_offset' is the offset in flattened value type.
+  int index = value_klass()->field_index_by_offset(offset);
+  int sub_offset = offset - field_offset(index);
+  Node* value = field_value(index);
+  if (recursive && value->is_ValueType()) {
+    // Flattened value type field
+    ValueTypeNode* vt = value->as_ValueType();
+    sub_offset += vt->value_klass()->first_field_offset(); // Add header size
+    return vt->field_value_by_offset(sub_offset);
+  }
+  assert(!(recursive && value->is_ValueType()), "should not be a value type");
+  assert(sub_offset == 0, "offset mismatch");
+  return value;
+}
+
+void ValueTypeBaseNode::set_field_value(uint index, Node* value) {
+  assert(index < field_count(), "index out of bounds");
+  set_req(Values + index, value);
+}
+
+int ValueTypeBaseNode::field_offset(uint index) const {
+  assert(index < field_count(), "index out of bounds");
+  return value_klass()->field_offset_by_index(index);
+}
+
+ciType* ValueTypeBaseNode::field_type(uint index) const {
+  assert(index < field_count(), "index out of bounds");
+  return value_klass()->field_type_by_index(index);
+}
+
+int ValueTypeBaseNode::make_scalar_in_safepoint(SafePointNode* sfpt, Node* root, PhaseGVN* gvn) {
+  ciValueKlass* vk = value_klass();
+  uint nfields = vk->flattened_field_count();
+  JVMState* jvms = sfpt->jvms();
+  int start = jvms->debug_start();
+  int end   = jvms->debug_end();
+  // Replace safepoint edge by SafePointScalarObjectNode and add field values
+  assert(jvms != NULL, "missing JVMS");
+  uint first_ind = (sfpt->req() - jvms->scloff());
+  const TypeValueTypePtr* res_type = value_type_ptr();
+  SafePointScalarObjectNode* sobj = new SafePointScalarObjectNode(res_type,
+#ifdef ASSERT
+                                                                  NULL,
+#endif
+                                                                  first_ind, nfields);
+  sobj->init_req(0, root);
+  // Iterate over the value type fields in order of increasing
+  // offset and add the field values to the safepoint.
+  for (uint j = 0; j < nfields; ++j) {
+    int offset = vk->nonstatic_field_at(j)->offset();
+    Node* value = field_value_by_offset(offset, true /* include flattened value type fields */);
+    assert(value != NULL, "");
+    sfpt->add_req(value);
+  }
+  jvms->set_endoff(sfpt->req());
+  if (gvn != NULL) {
+    sobj = gvn->transform(sobj)->as_SafePointScalarObject();
+    gvn->igvn_rehash_node_delayed(sfpt);
+  }
+  return sfpt->replace_edges_in_range(this, sobj, start, end);
+}
+
+void ValueTypeBaseNode::make_scalar_in_safepoints(Node* root, PhaseGVN* gvn) {
+  for (DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++) {
+    Node* u = fast_out(i);
+    if (u->is_SafePoint() && (!u->is_Call() || u->as_Call()->has_debug_use(this))) {
+      SafePointNode* sfpt = u->as_SafePoint();
+      Node* in_oop = get_oop();
+      const Type* oop_type = in_oop->bottom_type();
+      assert(Opcode() == Op_ValueTypePtr || TypePtr::NULL_PTR->higher_equal(oop_type), "already heap allocated value type should be linked directly");
+      int nb = make_scalar_in_safepoint(sfpt, root, gvn);
+      --i; imax -= nb;
+    }
+  }
+}
+
 ValueTypeNode* ValueTypeNode::make(PhaseGVN& gvn, ciValueKlass* klass) {
   // Create a new ValueTypeNode with uninitialized values and NULL oop
   const TypeValueType* type = TypeValueType::make(klass);
@@ -60,7 +224,7 @@
   ValueTypeNode* vt = new ValueTypeNode(type, oop);
   vt->load(gvn, mem, oop, oop, type->value_klass());
   assert(vt->is_allocated(&gvn), "value type should be allocated");
-  assert(oop->is_Con() || oop->is_CheckCastPP() || vt->is_loaded(&gvn, type) == oop, "value type should be loaded");
+  assert(oop->is_Con() || oop->is_CheckCastPP() || oop->Opcode() == Op_ValueTypePtr || vt->is_loaded(&gvn, type) == oop, "value type should be loaded");
   return gvn.transform(vt);
 }
 
@@ -260,161 +424,6 @@
   return oop_type->meet(TypePtr::NULL_PTR) != oop_type;
 }
 
-// Clones the values type to handle control flow merges involving multiple value types.
-// The inputs are replaced by PhiNodes to represent the merged values for the given region.
-ValueTypeNode* ValueTypeNode::clone_with_phis(PhaseGVN* gvn, Node* region) {
-  assert(!has_phi_inputs(region), "already cloned with phis");
-  ValueTypeNode* vt = clone()->as_ValueType();
-
-  // Create a PhiNode for merging the oop values
-  const TypeValueTypePtr* vtptr = TypeValueTypePtr::make(vt->bottom_type()->isa_valuetype());
-  PhiNode* oop = PhiNode::make(region, vt->get_oop(), vtptr);
-  gvn->set_type(oop, vtptr);
-  vt->set_oop(oop);
-
-  // Create a PhiNode each for merging the field values
-  for (uint i = 0; i < vt->field_count(); ++i) {
-    ciType* type = vt->field_type(i);
-    Node*  value = vt->field_value(i);
-    if (type->is_valuetype()) {
-      // Handle flattened value type fields recursively
-      value = value->as_ValueType()->clone_with_phis(gvn, region);
-    } else {
-      const Type* phi_type = Type::get_const_type(type);
-      value = PhiNode::make(region, value, phi_type);
-      gvn->set_type(value, phi_type);
-    }
-    vt->set_field_value(i, value);
-  }
-  gvn->set_type(vt, vt->bottom_type());
-  return vt;
-}
-
-// Checks if the inputs of the ValueTypeNode were replaced by PhiNodes
-// for the given region (see ValueTypeNode::clone_with_phis).
-bool ValueTypeNode::has_phi_inputs(Node* region) {
-  // Check oop input
-  bool result = get_oop()->is_Phi() && get_oop()->as_Phi()->region() == region;
-#ifdef ASSERT
-  if (result) {
-    // Check all field value inputs for consistency
-    for (uint i = Oop; i < field_count(); ++i) {
-      Node* n = in(i);
-      if (n->is_ValueType()) {
-        assert(n->as_ValueType()->has_phi_inputs(region), "inconsistent phi inputs");
-      } else {
-        assert(n->is_Phi() && n->as_Phi()->region() == region, "inconsistent phi inputs");
-      }
-    }
-  }
-#endif
-  return result;
-}
-
-// Merges 'this' with 'other' by updating the input PhiNodes added by 'clone_with_phis'
-ValueTypeNode* ValueTypeNode::merge_with(PhaseGVN* gvn, const ValueTypeNode* other, int pnum, bool transform) {
-  // Merge oop inputs
-  PhiNode* phi = get_oop()->as_Phi();
-  phi->set_req(pnum, other->get_oop());
-  if (transform) {
-    set_oop(gvn->transform(phi));
-    gvn->record_for_igvn(phi);
-  }
-  // Merge field values
-  for (uint i = 0; i < field_count(); ++i) {
-    Node* val1 =        field_value(i);
-    Node* val2 = other->field_value(i);
-    if (val1->isa_ValueType()) {
-      val1->as_ValueType()->merge_with(gvn, val2->as_ValueType(), pnum, transform);
-    } else {
-      assert(val1->is_Phi(), "must be a phi node");
-      assert(!val2->is_ValueType(), "inconsistent merge values");
-      val1->set_req(pnum, val2);
-    }
-    if (transform) {
-      set_field_value(i, gvn->transform(val1));
-      gvn->record_for_igvn(val1);
-    }
-  }
-  return this;
-}
-
-Node* ValueTypeNode::field_value(uint index) const {
-  assert(index < field_count(), "index out of bounds");
-  return in(Values + index);
-}
-
-// Get the value of the field at the given offset.
-// If 'recursive' is true, flattened value type fields will be resolved recursively.
-Node* ValueTypeNode::field_value_by_offset(int offset, bool recursive) const {
-  // If the field at 'offset' belongs to a flattened value type field, 'index' refers to the
-  // corresponding ValueTypeNode input and 'sub_offset' is the offset in flattened value type.
-  int index = value_klass()->field_index_by_offset(offset);
-  int sub_offset = offset - field_offset(index);
-  Node* value = field_value(index);
-  if (recursive && value->is_ValueType()) {
-    // Flattened value type field
-    ValueTypeNode* vt = value->as_ValueType();
-    sub_offset += vt->value_klass()->first_field_offset(); // Add header size
-    return vt->field_value_by_offset(sub_offset);
-  }
-  assert(!(recursive && value->is_ValueType()), "should not be a value type");
-  assert(sub_offset == 0, "offset mismatch");
-  return value;
-}
-
-void ValueTypeNode::set_field_value(uint index, Node* value) {
-  assert(index < field_count(), "index out of bounds");
-  set_req(Values + index, value);
-}
-
-int ValueTypeNode::field_offset(uint index) const {
-  assert(index < field_count(), "index out of bounds");
-  return value_klass()->field_offset_by_index(index);
-}
-
-ciType* ValueTypeNode::field_type(uint index) const {
-  assert(index < field_count(), "index out of bounds");
-  return value_klass()->field_type_by_index(index);
-}
-
-void ValueTypeNode::make_scalar_in_safepoints(Compile* C) {
-  const TypeValueTypePtr* res_type = TypeValueTypePtr::make(bottom_type()->isa_valuetype(), TypePtr::NotNull);
-  ciValueKlass* vk = value_klass();
-  uint nfields = vk->flattened_field_count();
-  for (DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++) {
-    Node* u = fast_out(i);
-    if (u->is_SafePoint() && (!u->is_Call() || u->as_Call()->has_debug_use(this))) {
-      Node* in_oop = get_oop();
-      const Type* oop_type = in_oop->bottom_type();
-      SafePointNode* sfpt = u->as_SafePoint();
-      JVMState* jvms = sfpt->jvms();
-      int start = jvms->debug_start();
-      int end   = jvms->debug_end();
-      assert(TypePtr::NULL_PTR->higher_equal(oop_type), "already heap allocated value type should be linked directly");
-      // Replace safepoint edge by SafePointScalarObjectNode and add field values
-      assert(jvms != NULL, "missing JVMS");
-      uint first_ind = (sfpt->req() - jvms->scloff());
-      SafePointScalarObjectNode* sobj = new SafePointScalarObjectNode(res_type,
-#ifdef ASSERT
-                                                                      NULL,
-#endif
-                                                                      first_ind, nfields);
-      sobj->init_req(0, C->root());
-      // Iterate over the value type fields in order of increasing
-      // offset and add the field values to the safepoint.
-      for (uint j = 0; j < nfields; ++j) {
-        int offset = vk->nonstatic_field_at(j)->offset();
-        Node* value = field_value_by_offset(offset, true /* include flattened value type fields */);
-        sfpt->add_req(value);
-      }
-      jvms->set_endoff(sfpt->req());
-      int nb = sfpt->replace_edges_in_range(this, sobj, start, end);
-      --i; imax -= nb;
-    }
-  }
-}
-
 void ValueTypeNode::pass_klass(Node* n, uint pos, const GraphKit& kit) {
   ciValueKlass* vk = value_klass();
   const TypeKlassPtr* tk = TypeKlassPtr::make(vk);
--- a/src/share/vm/opto/valuetypenode.hpp	Mon Jul 17 13:13:15 2017 +0200
+++ b/src/share/vm/opto/valuetypenode.hpp	Tue Jul 18 11:38:16 2017 +0200
@@ -30,29 +30,13 @@
 
 class GraphKit;
 
-//------------------------------ValueTypeNode-------------------------------------
-// Node representing a value type in C2 IR
-class ValueTypeNode : public TypeNode {
-private:
-  ValueTypeNode(const TypeValueType* t, Node* oop)
-    : TypeNode(t, Values + t->value_klass()->nof_declared_nonstatic_fields()) {
-    init_class_id(Class_ValueType);
-    init_req(Oop, oop);
+class ValueTypeBaseNode : public TypeNode {
+protected:
+  ValueTypeBaseNode(const Type* t, int nb_fields)
+    : TypeNode(t, nb_fields) {
+    init_class_id(Class_ValueTypeBase);
   }
 
-  ValueTypeNode(const TypeValueType* t, Node* oop, int field_count)
-    : TypeNode(t, Values + field_count) {
-    init_class_id(Class_ValueType);
-    init_req(Oop, oop);
-  }
-
-  // Get the klass defining the field layout of the value type
-  ciValueKlass* value_klass() const { return type()->is_valuetype()->value_klass(); }
-  // Initialize the value type by loading its field values from memory
-  void load(PhaseGVN& gvn, Node* mem, Node* base, Node* ptr, ciInstanceKlass* holder, int holder_offset = 0);
-  // Checks if the value type is loaded from memory and if so returns the oop
-  Node* is_loaded(PhaseGVN* phase, const TypeValueType* t, Node* base = NULL, int holder_offset = 0);
-
   enum { Control,   // Control input
          Oop,       // Oop of TypeValueTypePtr
          Values     // Nodes corresponding to values of the value type's fields.
@@ -60,6 +44,58 @@
                     // they correspond to. Field indeces are defined in ciValueKlass::_field_index_map.
   };
 
+  virtual const TypeValueTypePtr* value_type_ptr() const = 0;
+  // Get the klass defining the field layout of the value type
+  virtual ciValueKlass* value_klass() const = 0;
+  int make_scalar_in_safepoint(SafePointNode* sfpt, Node* root, PhaseGVN* gvn);
+public:
+  // Support for control flow merges
+  bool  has_phi_inputs(Node* region);
+  ValueTypeBaseNode* clone_with_phis(PhaseGVN* gvn, Node* region);
+  ValueTypeBaseNode* merge_with(PhaseGVN* gvn, const ValueTypeBaseNode* other, int pnum, bool transform);
+
+  // Get oop for heap allocated value type (may be TypePtr::NULL_PTR)
+  Node* get_oop() const    { return in(Oop); }
+  void  set_oop(Node* oop) { set_req(Oop, oop); }
+
+  // Value type fields
+  uint          field_count() const { return req() - Values; }
+  Node*         field_value(uint index) const;
+  Node*         field_value_by_offset(int offset, bool recursive = false) const;
+  void      set_field_value(uint index, Node* value);
+  int           field_offset(uint index) const;
+  ciType*       field_type(uint index) const;
+
+  // Replace ValueTypeNodes in debug info at safepoints with SafePointScalarObjectNodes
+  void make_scalar_in_safepoints(Node* root, PhaseGVN* gvn);
+};
+
+//------------------------------ValueTypeNode-------------------------------------
+// Node representing a value type in C2 IR
+class ValueTypeNode : public ValueTypeBaseNode {
+  friend class ValueTypeBaseNode;
+private:
+  ValueTypeNode(const TypeValueType* t, Node* oop)
+    : ValueTypeBaseNode(t, Values + t->value_klass()->nof_declared_nonstatic_fields()) {
+    init_class_id(Class_ValueType);
+    init_req(Oop, oop);
+  }
+
+  ValueTypeNode(const TypeValueType* t, Node* oop, int field_count)
+    : ValueTypeBaseNode(t, Values + field_count) {
+    init_class_id(Class_ValueType);
+    init_req(Oop, oop);
+  }
+
+  // Initialize the value type by loading its field values from memory
+  void load(PhaseGVN& gvn, Node* mem, Node* base, Node* ptr, ciInstanceKlass* holder, int holder_offset = 0);
+  // Checks if the value type is loaded from memory and if so returns the oop
+  Node* is_loaded(PhaseGVN* phase, const TypeValueType* t, Node* base = NULL, int holder_offset = 0);
+
+  const TypeValueTypePtr* value_type_ptr() const { return TypeValueTypePtr::make(bottom_type()->isa_valuetype()); }
+  // Get the klass defining the field layout of the value type
+  ciValueKlass* value_klass() const { return type()->is_valuetype()->value_klass(); }
+
 public:
   // Create a new ValueTypeNode with uninitialized values
   static ValueTypeNode* make(PhaseGVN& gvn, ciValueKlass* klass);
@@ -70,33 +106,16 @@
   // Create a new ValueTypeNode and load its values from a flattened value type field or array
   static Node* make(PhaseGVN& gvn, ciValueKlass* vk, Node* mem, Node* obj, Node* ptr, ciInstanceKlass* holder = NULL, int holder_offset = 0);
 
-  // Support for control flow merges
-  ValueTypeNode* clone_with_phis(PhaseGVN* gvn, Node* region);
-  bool  has_phi_inputs(Node* region);
-  ValueTypeNode* merge_with(PhaseGVN* gvn, const ValueTypeNode* other, int pnum, bool transform);
 
   // Store the value type as a flattened (headerless) representation
   void store_flattened(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* holder = NULL, int holder_offset = 0) const;
   // Store the field values to memory
   void store(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* holder, int holder_offset = 0) const;
 
-  // Get oop for heap allocated value type (may be TypePtr::NULL_PTR)
-  Node* get_oop() const    { return in(Oop); }
-  void  set_oop(Node* oop) { set_req(Oop, oop); }
   // Allocates the value type (if not yet allocated) and returns the oop
   Node* allocate(GraphKit* kit);
   bool  is_allocated(PhaseGVN* phase) const;
 
-  // Value type fields
-  uint          field_count() const { return req() - Values; }
-  Node*         field_value(uint index) const;
-  Node*         field_value_by_offset(int offset, bool recursive = false) const;
-  void      set_field_value(uint index, Node* value);
-  int           field_offset(uint index) const;
-  ciType*       field_type(uint index) const;
-
-  // Replace ValueTypeNodes in debug info at safepoints with SafePointScalarObjectNodes
-  void make_scalar_in_safepoints(Compile* C);
   void pass_klass(Node* n, uint pos, const GraphKit& kit);
   uint pass_fields(Node* call, int base_input, const GraphKit& kit, ciValueKlass* base_vk = NULL, int base_offset = 0);
   void replace_call_results(Node* call, Compile* C);
@@ -112,4 +131,25 @@
 #endif
 };
 
+//------------------------------ValueTypePtrNode-------------------------------------
+// Node representing a value type as a pointer in C2 IR
+class ValueTypePtrNode : public ValueTypeBaseNode {
+private:
+  ciValueKlass* value_klass() const { return type()->is_valuetypeptr()->value_type()->value_klass(); }
+  const TypeValueTypePtr* value_type_ptr() const { return bottom_type()->isa_valuetypeptr(); }
+public:
+
+  ValueTypePtrNode(ValueTypeNode* vt, Node* oop, Compile* C)
+    : ValueTypeBaseNode(TypeValueTypePtr::make(vt->type()->is_valuetype())->cast_to_ptr_type(TypePtr::NotNull), vt->req()) {
+    init_class_id(Class_ValueTypePtr);
+    for (uint i = Oop+1; i < vt->req(); i++) {
+      init_req(i, vt->in(i));
+    }
+    init_req(Oop, oop);
+    C->add_value_type_ptr(this);
+  }
+
+  virtual int Opcode() const;
+};
+
 #endif // SHARE_VM_OPTO_VALUETYPENODE_HPP
--- a/test/compiler/valhalla/valuetypes/ValueCapableClass2.java	Mon Jul 17 13:13:15 2017 +0200
+++ b/test/compiler/valhalla/valuetypes/ValueCapableClass2.java	Tue Jul 18 11:38:16 2017 +0200
@@ -28,10 +28,25 @@
  */
 package compiler.valhalla.valuetypes;
 
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import jdk.experimental.value.ValueType;
+
 @jvm.internal.value.DeriveValueType
 public final class ValueCapableClass2 {
     public final long u;
 
+    public static final MethodHandle FACTORY;
+    static {
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
+        ValueType<?> VT = ValueType.forClass(ValueCapableClass2.class);
+        try {
+            FACTORY = VT.unreflectWithers(lookup, true, VT.valueFields());
+        } catch(NoSuchFieldException|IllegalAccessException e) {
+            throw new RuntimeException("method handle lookup fails");
+        }
+    }
+
     private ValueCapableClass2(long u) {
         this.u = u;
     }
--- a/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java	Mon Jul 17 13:13:15 2017 +0200
+++ b/test/compiler/valhalla/valuetypes/ValueTypeTestBench.java	Tue Jul 18 11:38:16 2017 +0200
@@ -2683,6 +2683,173 @@
         }
     }
 
+    // method handle combinators
+    static {
+        try {
+            MethodHandles.Lookup lookup = MethodHandles.lookup();
+
+            MethodType test99_mt = MethodType.fromMethodDescriptorString("()Qcompiler/valhalla/valuetypes/MyValue3;", ValueTypeTestBench.class.getClassLoader());
+            MethodHandle test99_mh1 = lookup.findVirtual(ValueTypeTestBench.class, "test99_target1", test99_mt);
+            MethodHandle test99_mh2 = lookup.findVirtual(ValueTypeTestBench.class, "test99_target2", test99_mt);
+            MethodType boolean_mt = MethodType.methodType(boolean.class);
+            MethodHandle test99_mh_test = lookup.findVirtual(ValueTypeTestBench.class, "test99_test", boolean_mt);
+            test99_mh = MethodHandles.guardWithTest(test99_mh_test, test99_mh1, test99_mh2);
+
+            MethodType myvalue2_mt = MethodType.fromMethodDescriptorString("()Qcompiler/valhalla/valuetypes/MyValue2;", ValueTypeTestBench.class.getClassLoader());
+            test100_mh1 = lookup.findStatic(ValueTypeTestBench.class, "test100_target1", myvalue2_mt);
+            MethodHandle test100_mh2 = lookup.findStatic(ValueTypeTestBench.class, "test100_target2", myvalue2_mt);
+            MethodHandle test100_mh_test = lookup.findStatic(ValueTypeTestBench.class, "test100_test", boolean_mt);
+            test100_mh = MethodHandles.guardWithTest(test100_mh_test,
+                                                    MethodHandles.invoker(myvalue2_mt),
+                                                    MethodHandles.dropArguments(test100_mh2, 0, MethodHandle.class));
+
+            MethodHandle test101_mh1 = lookup.findStatic(ValueTypeTestBench.class, "test101_target1", myvalue2_mt);
+            test101_mh2 = lookup.findStatic(ValueTypeTestBench.class, "test101_target2", myvalue2_mt);
+            MethodHandle test101_mh_test = lookup.findStatic(ValueTypeTestBench.class, "test101_test", boolean_mt);
+            test101_mh = MethodHandles.guardWithTest(test101_mh_test,
+                                                    MethodHandles.dropArguments(test101_mh1, 0, MethodHandle.class),
+                                                    MethodHandles.invoker(myvalue2_mt));
+
+            MethodHandle test102_count = MethodHandles.constant(int.class, 100);
+            ValueType<?> test102_VT = ValueType.forClass(ValueCapableClass2.class);
+            MethodHandle test102_init = test102_VT.defaultValueConstant();
+            MethodHandle test102_getfield = test102_VT.findGetter(lookup, "u", long.class);
+            MethodHandle test102_add = lookup.findStatic(Long.class, "sum", MethodType.methodType(long.class, long.class, long.class));
+            MethodHandle test102_body = MethodHandles.collectArguments(ValueCapableClass2.FACTORY,
+                                                                      0,
+                                                                      MethodHandles.dropArguments(MethodHandles.collectArguments(MethodHandles.insertArguments(test102_add,
+                                                                                                                                                               0,
+                                                                                                                                                               1L),
+                                                                                                                                 0,
+                                                                                                                                 test102_getfield),
+                                                                                                  1,
+                                                                                                  int.class));
+            test102_mh = MethodHandles.collectArguments(test102_getfield,
+                                                       0,
+                                                       MethodHandles.countedLoop(test102_count, test102_init, test102_body));
+
+        } catch (NoSuchMethodException|IllegalAccessException|NoSuchFieldException e) {
+            e.printStackTrace();
+            throw new RuntimeException("method handle lookup fails");
+        }
+    }
+
+    // Return of target1 and target2 merged in a Lambda Form as an
+    // __Value. Shouldn't cause any allocation
+    final MyValue3 test99_vt1 = MyValue3.create();
+    @ForceInline
+    MyValue3 test99_target1() {
+        return test99_vt1;
+    }
+
+    final MyValue3 test99_vt2 = MyValue3.create();
+    @ForceInline
+    MyValue3 test99_target2() {
+        return test99_vt2;
+    }
+
+    boolean test99_bool = true;
+    @ForceInline
+    boolean test99_test() {
+        return test99_bool;
+    }
+
+    static final MethodHandle test99_mh;
+
+    @Test(valid = ValueTypeReturnedAsFieldsOn, failOn = ALLOC + ALLOCA + STORE)
+    @Test(valid = ValueTypeReturnedAsFieldsOff)
+    MyValue3 test99() throws Throwable {
+        return (MyValue3)test99_mh.invokeExact(this);
+    }
+
+    @DontCompile
+    public void test99_verifier(boolean warmup) throws Throwable {
+        test99_bool = !test99_bool;
+        MyValue3 vt = test99();
+        vt.verify(test99_bool ? test99_vt1 : test99_vt2);
+    }
+
+    // Similar as above but with the method handle for target1 not
+    // constant. Shouldn't cause any allocation but does currently.
+    @ForceInline
+    static MyValue2 test100_target1() {
+        return MyValue2.createWithFieldsInline(rI, true);
+    }
+
+    @ForceInline
+    static MyValue2 test100_target2() {
+        return MyValue2.createWithFieldsInline(rI+1, false);
+    }
+
+    static boolean test100_bool = true;
+    @ForceInline
+    static boolean test100_test() {
+        return test100_bool;
+    }
+
+    static final MethodHandle test100_mh;
+    static MethodHandle test100_mh1;
+
+    @Test
+    long test100() throws Throwable {
+        return ((MyValue2)test100_mh.invokeExact(test100_mh1)).hash();
+    }
+
+    @DontCompile
+    public void test100_verifier(boolean warmup) throws Throwable {
+        test100_bool = !test100_bool;
+        long hash = test100();
+        Asserts.assertEQ(hash, MyValue2.createWithFieldsInline(rI+(test100_bool ? 0 : 1), test100_bool).hash());
+    }
+
+    // Same as above but with the method handle for target2 not
+    // constant. Shouldn't cause any allocation but does currently.
+    @ForceInline
+    static MyValue2 test101_target1() {
+        return MyValue2.createWithFieldsInline(rI, true);
+    }
+
+    @ForceInline
+    static MyValue2 test101_target2() {
+        return MyValue2.createWithFieldsInline(rI+1, false);
+    }
+
+    static boolean test101_bool = true;
+    @ForceInline
+    static boolean test101_test() {
+        return test101_bool;
+    }
+
+    static final MethodHandle test101_mh;
+    static MethodHandle test101_mh2;
+
+    @Test
+    long test101() throws Throwable {
+        return ((MyValue2)test101_mh.invokeExact(test101_mh2)).hash();
+    }
+
+    @DontCompile
+    public void test101_verifier(boolean warmup) throws Throwable {
+        test101_bool = !test101_bool;
+        long hash = test101();
+        Asserts.assertEQ(hash, MyValue2.createWithFieldsInline(rI+(test101_bool ? 0 : 1), test101_bool).hash());
+    }
+
+    // Simple reduction with intermediate result merged in a Lambda
+    // Form as an __Value. Shouldn't cause any allocations. The entire
+    // loop should go away as the result is a constant.
+    static final MethodHandle test102_mh;
+
+    @Test(failOn = ALLOC + STORE + LOOP)
+    long test102() throws Throwable {
+        return (long)test102_mh.invokeExact();
+    }
+
+    @DontCompile
+    public void test102_verifier(boolean warmup) throws Throwable {
+        long v = test102();
+        Asserts.assertEQ(v, 100L);
+    }
     // ========== Test infrastructure ==========
 
     private static final WhiteBox WHITE_BOX = WhiteBox.getWhiteBox();
@@ -2770,7 +2937,9 @@
                     "-XX:CompileCommand=compileonly,compiler.valhalla.valuetypes.MyValue4::*",
                     "-XX:CompileCommand=compileonly,java.lang.Object::<init>",
                     "-XX:CompileCommand=inline,java.lang.__Value::hashCode",
-                    "-XX:CompileCommand=compileonly,java.lang.invoke.*::*");
+                    "-XX:CompileCommand=compileonly,java.lang.invoke.*::*",
+                    "-XX:CompileCommand=compileonly,compiler.valhalla.valuetypes.ValueCapableClass2_*::*",
+                    "-XX:CompileCommand=compileonly,java.lang.Long::sum");
         } else {
             // Execute tests
             ValueTypeTestBench bench = new ValueTypeTestBench();