Fri, 28 Oct 2016 22:36:23 +0000
8038348: Instance field load is replaced by wrong data Phi
Summary: Store additional information in PhiNodes corresponding to known instance field values to avoid incorrect reusage.
Reviewed-by: kvn, thartmann
1.1 --- a/src/share/vm/opto/cfgnode.hpp Fri Oct 21 20:34:04 2016 +0300 1.2 +++ b/src/share/vm/opto/cfgnode.hpp Fri Oct 28 22:36:23 2016 +0000 1.3 @@ -119,6 +119,9 @@ 1.4 // input in slot 0. 1.5 class PhiNode : public TypeNode { 1.6 const TypePtr* const _adr_type; // non-null only for Type::MEMORY nodes. 1.7 + // The following fields are only used for data PhiNodes to indicate 1.8 + // that the PhiNode represents the value of a known instance field. 1.9 + int _inst_mem_id; // Instance memory id (node index of the memory Phi) 1.10 const int _inst_id; // Instance id of the memory slice. 1.11 const int _inst_index; // Alias index of the instance memory slice. 1.12 // Array elements references have the same alias_idx but different offset. 1.13 @@ -138,11 +141,13 @@ 1.14 }; 1.15 1.16 PhiNode( Node *r, const Type *t, const TypePtr* at = NULL, 1.17 + const int imid = -1, 1.18 const int iid = TypeOopPtr::InstanceTop, 1.19 const int iidx = Compile::AliasIdxTop, 1.20 const int ioffs = Type::OffsetTop ) 1.21 : TypeNode(t,r->req()), 1.22 _adr_type(at), 1.23 + _inst_mem_id(imid), 1.24 _inst_id(iid), 1.25 _inst_index(iidx), 1.26 _inst_offset(ioffs) 1.27 @@ -187,11 +192,14 @@ 1.28 virtual bool pinned() const { return in(0) != 0; } 1.29 virtual const TypePtr *adr_type() const { verify_adr_type(true); return _adr_type; } 1.30 1.31 + void set_inst_mem_id(int inst_mem_id) { _inst_mem_id = inst_mem_id; } 1.32 + const int inst_mem_id() const { return _inst_mem_id; } 1.33 const int inst_id() const { return _inst_id; } 1.34 const int inst_index() const { return _inst_index; } 1.35 const int inst_offset() const { return _inst_offset; } 1.36 - bool is_same_inst_field(const Type* tp, int id, int index, int offset) { 1.37 + bool is_same_inst_field(const Type* tp, int mem_id, int id, int index, int offset) { 1.38 return type()->basic_type() == tp->basic_type() && 1.39 + inst_mem_id() == mem_id && 1.40 inst_id() == id && 1.41 inst_index() == index && 1.42 inst_offset() == offset &&
2.1 --- a/src/share/vm/opto/macro.cpp Fri Oct 21 20:34:04 2016 +0300 2.2 +++ b/src/share/vm/opto/macro.cpp Fri Oct 28 22:36:23 2016 +0000 2.3 @@ -401,7 +401,7 @@ 2.4 for (DUIterator_Fast kmax, k = region->fast_outs(kmax); k < kmax; k++) { 2.5 Node* phi = region->fast_out(k); 2.6 if (phi->is_Phi() && phi != mem && 2.7 - phi->as_Phi()->is_same_inst_field(phi_type, instance_id, alias_idx, offset)) { 2.8 + phi->as_Phi()->is_same_inst_field(phi_type, (int)mem->_idx, instance_id, alias_idx, offset)) { 2.9 return phi; 2.10 } 2.11 } 2.12 @@ -420,7 +420,7 @@ 2.13 GrowableArray <Node *> values(length, length, NULL, false); 2.14 2.15 // create a new Phi for the value 2.16 - PhiNode *phi = new (C) PhiNode(mem->in(0), phi_type, NULL, instance_id, alias_idx, offset); 2.17 + PhiNode *phi = new (C) PhiNode(mem->in(0), phi_type, NULL, mem->_idx, instance_id, alias_idx, offset); 2.18 transform_later(phi); 2.19 value_phis->push(phi, mem->_idx); 2.20
3.1 --- a/src/share/vm/opto/memnode.cpp Fri Oct 21 20:34:04 2016 +0300 3.2 +++ b/src/share/vm/opto/memnode.cpp Fri Oct 28 22:36:23 2016 +0000 3.3 @@ -1155,7 +1155,7 @@ 3.4 for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) { 3.5 Node* phi = region->fast_out(i); 3.6 if (phi->is_Phi() && phi != mem && 3.7 - phi->as_Phi()->is_same_inst_field(this_type, this_iid, this_index, this_offset)) { 3.8 + phi->as_Phi()->is_same_inst_field(this_type, (int)mem->_idx, this_iid, this_index, this_offset)) { 3.9 return phi; 3.10 } 3.11 } 3.12 @@ -1400,7 +1400,7 @@ 3.13 this_iid = base->_idx; 3.14 } 3.15 PhaseIterGVN* igvn = phase->is_IterGVN(); 3.16 - Node* phi = new (C) PhiNode(region, this_type, NULL, this_iid, this_index, this_offset); 3.17 + Node* phi = new (C) PhiNode(region, this_type, NULL, mem->_idx, this_iid, this_index, this_offset); 3.18 for (uint i = 1; i < region->req(); i++) { 3.19 Node* x; 3.20 Node* the_clone = NULL;
4.1 --- a/src/share/vm/opto/phaseX.cpp Fri Oct 21 20:34:04 2016 +0300 4.2 +++ b/src/share/vm/opto/phaseX.cpp Fri Oct 28 22:36:23 2016 +0000 4.3 @@ -481,6 +481,8 @@ 4.4 uint current_idx = 0; // The current new node ID. Incremented after every assignment. 4.5 for (uint i = 0; i < _useful.size(); i++) { 4.6 Node* n = _useful.at(i); 4.7 + // Sanity check that fails if we ever decide to execute this phase after EA 4.8 + assert(!n->is_Phi() || n->as_Phi()->inst_mem_id() == -1, "should not be linked to data Phi"); 4.9 const Type* type = gvn->type_or_null(n); 4.10 new_type_array.map(current_idx, type); 4.11 4.12 @@ -1378,6 +1380,18 @@ 4.13 i -= num_edges; // we deleted 1 or more copies of this edge 4.14 } 4.15 4.16 + // Search for instance field data PhiNodes in the same region pointing to the old 4.17 + // memory PhiNode and update their instance memory ids to point to the new node. 4.18 + if (old->is_Phi() && old->as_Phi()->type()->has_memory() && old->in(0) != NULL) { 4.19 + Node* region = old->in(0); 4.20 + for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) { 4.21 + PhiNode* phi = region->fast_out(i)->isa_Phi(); 4.22 + if (phi != NULL && phi->inst_mem_id() == (int)old->_idx) { 4.23 + phi->set_inst_mem_id((int)nn->_idx); 4.24 + } 4.25 + } 4.26 + } 4.27 + 4.28 // Smash all inputs to 'old', isolating him completely 4.29 Node *temp = new (C) Node(1); 4.30 temp->init_req(0,nn); // Add a use to nn to prevent him from dying
5.1 --- a/src/share/vm/opto/type.hpp Fri Oct 21 20:34:04 2016 +0300 5.2 +++ b/src/share/vm/opto/type.hpp Fri Oct 28 22:36:23 2016 +0000 5.3 @@ -882,7 +882,7 @@ 5.4 5.5 // If not InstanceTop or InstanceBot, indicates that this is 5.6 // a particular instance of this type which is distinct. 5.7 - // This is the the node index of the allocation node creating this instance. 5.8 + // This is the node index of the allocation node creating this instance. 5.9 int _instance_id; 5.10 5.11 // Extra type information profiling gave us. We propagate it the