--- old/src/hotspot/share/opto/castnode.cpp 2017-09-28 12:26:36.595254868 +0200 +++ new/src/hotspot/share/opto/castnode.cpp 2017-09-28 12:26:36.295254873 +0200 @@ -411,7 +411,7 @@ // ValueTypePtrNode from the call. if (can_reshape && in(0) == NULL && - phase->C->can_add_value_type_ptr() && + phase->C->can_add_value_type() && type()->isa_valuetypeptr() && in(1) != NULL && in(1)->is_Proj() && in(1)->in(0) != NULL && in(1)->in(0)->is_CallStaticJava() && --- old/src/hotspot/share/opto/compile.cpp 2017-09-28 12:26:37.711254853 +0200 +++ new/src/hotspot/share/opto/compile.cpp 2017-09-28 12:26:37.367254858 +0200 @@ -405,11 +405,9 @@ 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); - } + // Remove useless value type nodes + if (_value_type_nodes != NULL) { + _value_type_nodes->remove_useless_nodes(useful.member_set()); } // clean up the late inline lists remove_useless_late_inlines(&_string_late_inlines, useful); @@ -1185,7 +1183,7 @@ _predicate_opaqs = new(comp_arena()) GrowableArray(comp_arena(), 8, 0, NULL); _expensive_nodes = new(comp_arena()) GrowableArray(comp_arena(), 8, 0, NULL); _range_check_casts = new(comp_arena()) GrowableArray(comp_arena(), 8, 0, NULL); - _value_type_ptr_nodes = new(comp_arena()) GrowableArray(comp_arena(), 8, 0, NULL); + _value_type_nodes = new (comp_arena()) Unique_Node_List(comp_arena()); register_library_intrinsics(); } @@ -1971,22 +1969,30 @@ assert(range_check_cast_count() == 0, "should be empty"); } -void Compile::add_value_type_ptr(ValueTypePtrNode* n) { - assert(can_add_value_type_ptr(), "too late"); - 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()); +void Compile::add_value_type(Node* n) { + assert(n->is_ValueTypeBase(), "unexpected node"); + if (_value_type_nodes != NULL) { + _value_type_nodes->push(n); + } +} + +void Compile::remove_value_type(Node* n) { + assert(n->is_ValueTypeBase(), "unexpected node"); + if (_value_type_nodes != NULL) { + _value_type_nodes->remove(n); + } +} + +void Compile::process_value_types(PhaseIterGVN &igvn) { + // Make value types scalar in safepoints + while (_value_type_nodes->size() != 0) { + ValueTypeBaseNode* vt = _value_type_nodes->pop()->as_ValueTypeBase(); + vt->make_scalar_in_safepoints(igvn.C->root(), &igvn); + if (vt->is_ValueTypePtr()) { + igvn.replace_node(vt, vt->get_oop()); + } } - assert(value_type_ptr_count() == 0, "should be empty"); - _value_type_ptr_nodes = NULL; + _value_type_nodes = NULL; igvn.optimize(); } @@ -2237,8 +2243,9 @@ igvn.optimize(); } - if (value_type_ptr_count() > 0) { - process_value_type_ptr_nodes(igvn); + if (_value_type_nodes->size() > 0) { + // Do this once all inlining is over to avoid getting inconsistent debug info + process_value_types(igvn); } // Perform escape analysis @@ -2385,25 +2392,9 @@ print_method(PHASE_OPTIMIZE_FINISHED, 2); } -// Fixme remove -static void check_for_value_node(Node &n, void* C) { - if (n.is_ValueType()) { -#ifdef ASSERT - ((Compile*)C)->method()->print_short_name(); - tty->print_cr(""); - n.dump(-1); - assert(false, "Unable to match ValueTypeNode"); -#endif - ((Compile*)C)->record_failure("Unable to match ValueTypeNode"); - } -} - //------------------------------Code_Gen--------------------------------------- // Given a graph, generate code for it void Compile::Code_Gen() { - // FIXME remove - root()->walk(Node::nop, check_for_value_node, this); - if (failing()) { return; } @@ -3382,18 +3373,14 @@ } break; } +#ifdef ASSERT + case Op_ValueTypePtr: case Op_ValueType: { - ValueTypeNode* vt = n->as_ValueType(); - vt->make_scalar_in_safepoints(root(), NULL); - if (vt->outcnt() == 0) { - vt->disconnect_inputs(NULL, this); - } - break; - } - case Op_ValueTypePtr: { - ShouldNotReachHere(); + n->dump(-1); + assert(false, "value type node was not removed"); break; } +#endif default: assert( !n->is_Call(), "" ); assert( !n->is_Mem(), "" ); --- old/src/hotspot/share/opto/compile.hpp 2017-09-28 12:26:38.847254837 +0200 +++ new/src/hotspot/share/opto/compile.hpp 2017-09-28 12:26:38.555254841 +0200 @@ -83,7 +83,7 @@ class TypePtr; class TypeOopPtr; class TypeFunc; -class ValueTypePtrNode; +class ValueTypeBaseNode; class Unique_Node_List; class nmethod; class WarmCallInfo; @@ -416,7 +416,7 @@ GrowableArray* _predicate_opaqs; // List of Opaque1 nodes for the loop predicates. GrowableArray* _expensive_nodes; // List of nodes that are expensive to compute and that we'd better not let the GVN freely common GrowableArray* _range_check_casts; // List of CastII nodes with a range check dependency - GrowableArray* _value_type_ptr_nodes; // List of ValueTypePtr nodes + Unique_Node_List* _value_type_nodes; // List of ValueType nodes ConnectionGraph* _congraph; #ifndef PRODUCT IdealGraphPrinter* _printer; @@ -809,16 +809,11 @@ // 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); - bool can_add_value_type_ptr() const { return _value_type_ptr_nodes != NULL; } + // Keep track of value type nodes for later processing + void add_value_type(Node* n); + void remove_value_type(Node* n); + void process_value_types(PhaseIterGVN &igvn); + bool can_add_value_type() const { return _value_type_nodes != NULL; } // remove the opaque nodes that protect the predicates so that the unused checks and // uncommon traps will be eliminated from the graph. --- old/src/hotspot/share/opto/loopopts.cpp 2017-09-28 12:26:39.727254825 +0200 +++ new/src/hotspot/share/opto/loopopts.cpp 2017-09-28 12:26:39.447254829 +0200 @@ -1015,11 +1015,6 @@ Node* m = n->fast_out(j); if (m->is_FastLock()) return false; - if (m->is_ValueType()) { - // TODO this breaks optimizations! - // Value types should not be split through phis - //return false; - } #ifdef _LP64 if (m->Opcode() == Op_ConvI2L) return false; @@ -1372,6 +1367,7 @@ // Remove multiple allocations of the same value type if (n->is_ValueType() && EliminateAllocations) { n->as_ValueType()->remove_redundant_allocations(&_igvn, this); + return; // n is now dead } // Check for Opaque2's who's loop has disappeared - who's input is in the --- old/src/hotspot/share/opto/macro.cpp 2017-09-28 12:26:40.603254813 +0200 +++ new/src/hotspot/share/opto/macro.cpp 2017-09-28 12:26:40.303254817 +0200 @@ -868,6 +868,7 @@ // // Process the safepoint uses // + Unique_Node_List value_worklist; while (safepoints.length() > 0) { SafePointNode* sfpt = safepoints.pop(); Node* mem = sfpt->memory(); @@ -996,6 +997,9 @@ } else { field_val = transform_later(new DecodeNNode(field_val, field_val->get_ptr_type())); } + } else if (field_val->is_ValueType()) { + // Keep track of value types to scalarize them later + value_worklist.push(field_val); } sfpt->add_req(field_val); } @@ -1009,6 +1013,11 @@ _igvn._worklist.push(sfpt); safepoints_done.append_if_missing(sfpt); // keep it for rollback } + // Scalarize value types that were added to the safepoint + for (uint i = 0; i < value_worklist.size(); ++i) { + Node* vt = value_worklist.at(i); + vt->as_ValueType()->make_scalar_in_safepoints(C->root(), &_igvn); + } return true; } --- old/src/hotspot/share/opto/node.cpp 2017-09-28 12:26:41.807254796 +0200 +++ new/src/hotspot/share/opto/node.cpp 2017-09-28 12:26:41.267254804 +0200 @@ -504,9 +504,6 @@ 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() ); @@ -540,6 +537,9 @@ if (n->is_SafePoint()) { n->as_SafePoint()->clone_replaced_nodes(); } + if (n->is_ValueTypeBase()) { + C->add_value_type(n); + } return n; // Return the clone } @@ -615,8 +615,8 @@ 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_ValueTypeBase()) { + compile->remove_value_type(this); } if (is_SafePoint()) { @@ -1358,8 +1358,8 @@ 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()); + if (dead->is_ValueTypeBase()) { + igvn->C->remove_value_type(dead); } igvn->C->record_dead_node(dead->_idx); // Kill all inputs to the dead guy --- old/src/hotspot/share/opto/node.hpp 2017-09-28 12:26:42.739254783 +0200 +++ new/src/hotspot/share/opto/node.hpp 2017-09-28 12:26:42.451254787 +0200 @@ -141,8 +141,9 @@ class Type; class TypeNode; class UnlockNode; -class ValueTypeNode; class ValueTypeBaseNode; +class ValueTypeNode; +class ValueTypePtrNode; class VectorNode; class LoadVectorNode; class StoreVectorNode; --- old/src/hotspot/share/opto/phaseX.cpp 2017-09-28 12:26:43.631254771 +0200 +++ new/src/hotspot/share/opto/phaseX.cpp 2017-09-28 12:26:43.331254775 +0200 @@ -1421,8 +1421,8 @@ 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()); + if (dead->is_ValueTypeBase()) { + C->remove_value_type(dead); } } } // while (_stack.is_nonempty()) --- old/src/hotspot/share/opto/split_if.cpp 2017-09-28 12:26:44.527254759 +0200 +++ new/src/hotspot/share/opto/split_if.cpp 2017-09-28 12:26:44.255254762 +0200 @@ -27,6 +27,7 @@ #include "opto/callnode.hpp" #include "opto/loopnode.hpp" #include "opto/movenode.hpp" +#include "opto/valuetypenode.hpp" //------------------------------split_thru_region------------------------------ @@ -194,8 +195,16 @@ rtype = TypeLong::INT; } + // Value types should not be split through Phis but each value input + // needs to be merged individually. At this point, value types should + // only be used by AllocateNodes. Try to remove redundant allocations + // and unlink the now dead value type node. + if (n->is_ValueType()) { + n->as_ValueType()->remove_redundant_allocations(&_igvn, this); + return true; // n is now dead + } + // Now actually split-up this guy. One copy per control path merging. - assert(!n->is_ValueType(), "value types should not be split through phis"); Node *phi = PhiNode::make_blank(blk1, n); for( uint j = 1; j < blk1->req(); j++ ) { Node *x = n->clone(); --- old/src/hotspot/share/opto/valuetypenode.cpp 2017-09-28 12:26:45.519254745 +0200 +++ new/src/hotspot/share/opto/valuetypenode.cpp 2017-09-28 12:26:45.175254750 +0200 @@ -201,6 +201,7 @@ } void ValueTypeBaseNode::make_scalar_in_safepoints(Node* root, PhaseGVN* gvn) { + // Process all safepoint uses and scalarize value type Unique_Node_List worklist; for (DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++) { Node* u = fast_out(i); @@ -213,9 +214,9 @@ --i; imax -= nb; } } - - for (uint next = 0; next < worklist.size(); ++next) { - Node* vt = worklist.at(next); + // Now scalarize non-flattened fields + for (uint i = 0; i < worklist.size(); ++i) { + Node* vt = worklist.at(i); vt->as_ValueType()->make_scalar_in_safepoints(root, gvn); } } @@ -469,7 +470,7 @@ ValueTypeNode* ValueTypeNode::make(PhaseGVN& gvn, ciValueKlass* klass) { // Create a new ValueTypeNode with uninitialized values and NULL oop const TypeValueType* type = TypeValueType::make(klass); - return new ValueTypeNode(type, gvn.zerocon(T_VALUETYPE)); + return new ValueTypeNode(type, gvn.zerocon(T_VALUETYPE), gvn.C); } Node* ValueTypeNode::make_default(PhaseGVN& gvn, ciValueKlass* vk) { @@ -493,7 +494,7 @@ // Create and initialize a ValueTypeNode by loading all field // values from a heap-allocated version and also save the oop. const TypeValueType* type = gvn.type(oop)->is_valuetypeptr()->value_type(); - ValueTypeNode* vt = new ValueTypeNode(type, oop); + ValueTypeNode* vt = new ValueTypeNode(type, oop, gvn.C); if (null_check && !vt->is_allocated(&gvn)) { // Add oop null check @@ -769,6 +770,30 @@ phase->lazy_replace(projs.catchall_catchproj, phase->C->top()); phase->lazy_replace(projs.resproj, phase->C->top()); } + + // Process users + for (DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++) { + Node* out = fast_out(i); + if (out->isa_ValueType() != NULL) { + // Recursively process value type users + out->as_ValueType()->remove_redundant_allocations(igvn, phase); + --i; --imax; + } else if (out->isa_Allocate() != NULL) { + // Unlink AllocateNode + assert(out->in(AllocateNode::ValueNode) == this, "should be linked"); + igvn->replace_input_of(out, AllocateNode::ValueNode, NULL); + --i; --imax; + } else { +#ifdef ASSERT + // The value type should not have any other users at this time + out->dump(); + assert(false, "unexpected user of value type"); +#endif + } + } + + // Should be dead now + igvn->remove_dead_node(this); } #ifndef PRODUCT --- old/src/hotspot/share/opto/valuetypenode.hpp 2017-09-28 12:26:46.471254732 +0200 +++ new/src/hotspot/share/opto/valuetypenode.hpp 2017-09-28 12:26:46.211254735 +0200 @@ -32,9 +32,10 @@ class ValueTypeBaseNode : public TypeNode { protected: - ValueTypeBaseNode(const Type* t, int nb_fields) + ValueTypeBaseNode(const Type* t, int nb_fields, Compile* C) : TypeNode(t, nb_fields) { init_class_id(Class_ValueTypeBase); + C->add_value_type(this); } enum { Control, // Control input @@ -93,8 +94,8 @@ class ValueTypeNode : public ValueTypeBaseNode { friend class ValueTypeBaseNode; private: - ValueTypeNode(const TypeValueType* t, Node* oop) - : ValueTypeBaseNode(t, Values + t->value_klass()->nof_declared_nonstatic_fields()) { + ValueTypeNode(const TypeValueType* t, Node* oop, Compile* C) + : ValueTypeBaseNode(t, Values + t->value_klass()->nof_declared_nonstatic_fields(), C) { init_class_id(Class_ValueType); init_req(Oop, oop); } @@ -146,21 +147,19 @@ const TypeValueTypePtr* value_type_ptr() const { return bottom_type()->isa_valuetypeptr(); } ValueTypePtrNode(ciValueKlass* vk, Node* oop, Compile* C) - : ValueTypeBaseNode(TypeValueTypePtr::make(TypePtr::NotNull, vk), Values + vk->nof_declared_nonstatic_fields()) { + : ValueTypeBaseNode(TypeValueTypePtr::make(TypePtr::NotNull, vk), Values + vk->nof_declared_nonstatic_fields(), C) { init_class_id(Class_ValueTypePtr); init_req(Oop, oop); - C->add_value_type_ptr(this); } public: ValueTypePtrNode(ValueTypeNode* vt, Node* oop, Compile* C) - : ValueTypeBaseNode(TypeValueTypePtr::make(vt->type()->is_valuetype())->cast_to_ptr_type(TypePtr::NotNull), vt->req()) { + : ValueTypeBaseNode(TypeValueTypePtr::make(vt->type()->is_valuetype())->cast_to_ptr_type(TypePtr::NotNull), vt->req(), C) { 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); } static ValueTypePtrNode* make(GraphKit* kit, ciValueKlass* vk, CallNode* call);