# HG changeset patch # User poonam # Date 1477694183 0 # Node ID 3b6372514697a64764955ee83efe6f8223522228 # Parent 072770c9a6b982e3d690dbff14f79c34bbeef8b6 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 diff -r 072770c9a6b9 -r 3b6372514697 src/share/vm/opto/cfgnode.hpp --- a/src/share/vm/opto/cfgnode.hpp Fri Oct 21 20:34:04 2016 +0300 +++ b/src/share/vm/opto/cfgnode.hpp Fri Oct 28 22:36:23 2016 +0000 @@ -119,6 +119,9 @@ // input in slot 0. class PhiNode : public TypeNode { const TypePtr* const _adr_type; // non-null only for Type::MEMORY nodes. + // The following fields are only used for data PhiNodes to indicate + // that the PhiNode represents the value of a known instance field. + int _inst_mem_id; // Instance memory id (node index of the memory Phi) const int _inst_id; // Instance id of the memory slice. const int _inst_index; // Alias index of the instance memory slice. // Array elements references have the same alias_idx but different offset. @@ -138,11 +141,13 @@ }; PhiNode( Node *r, const Type *t, const TypePtr* at = NULL, + const int imid = -1, const int iid = TypeOopPtr::InstanceTop, const int iidx = Compile::AliasIdxTop, const int ioffs = Type::OffsetTop ) : TypeNode(t,r->req()), _adr_type(at), + _inst_mem_id(imid), _inst_id(iid), _inst_index(iidx), _inst_offset(ioffs) @@ -187,11 +192,14 @@ virtual bool pinned() const { return in(0) != 0; } virtual const TypePtr *adr_type() const { verify_adr_type(true); return _adr_type; } + void set_inst_mem_id(int inst_mem_id) { _inst_mem_id = inst_mem_id; } + const int inst_mem_id() const { return _inst_mem_id; } const int inst_id() const { return _inst_id; } const int inst_index() const { return _inst_index; } const int inst_offset() const { return _inst_offset; } - bool is_same_inst_field(const Type* tp, int id, int index, int offset) { + bool is_same_inst_field(const Type* tp, int mem_id, int id, int index, int offset) { return type()->basic_type() == tp->basic_type() && + inst_mem_id() == mem_id && inst_id() == id && inst_index() == index && inst_offset() == offset && diff -r 072770c9a6b9 -r 3b6372514697 src/share/vm/opto/macro.cpp --- a/src/share/vm/opto/macro.cpp Fri Oct 21 20:34:04 2016 +0300 +++ b/src/share/vm/opto/macro.cpp Fri Oct 28 22:36:23 2016 +0000 @@ -401,7 +401,7 @@ for (DUIterator_Fast kmax, k = region->fast_outs(kmax); k < kmax; k++) { Node* phi = region->fast_out(k); if (phi->is_Phi() && phi != mem && - phi->as_Phi()->is_same_inst_field(phi_type, instance_id, alias_idx, offset)) { + phi->as_Phi()->is_same_inst_field(phi_type, (int)mem->_idx, instance_id, alias_idx, offset)) { return phi; } } @@ -420,7 +420,7 @@ GrowableArray values(length, length, NULL, false); // create a new Phi for the value - PhiNode *phi = new (C) PhiNode(mem->in(0), phi_type, NULL, instance_id, alias_idx, offset); + PhiNode *phi = new (C) PhiNode(mem->in(0), phi_type, NULL, mem->_idx, instance_id, alias_idx, offset); transform_later(phi); value_phis->push(phi, mem->_idx); diff -r 072770c9a6b9 -r 3b6372514697 src/share/vm/opto/memnode.cpp --- a/src/share/vm/opto/memnode.cpp Fri Oct 21 20:34:04 2016 +0300 +++ b/src/share/vm/opto/memnode.cpp Fri Oct 28 22:36:23 2016 +0000 @@ -1155,7 +1155,7 @@ for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) { Node* phi = region->fast_out(i); if (phi->is_Phi() && phi != mem && - phi->as_Phi()->is_same_inst_field(this_type, this_iid, this_index, this_offset)) { + phi->as_Phi()->is_same_inst_field(this_type, (int)mem->_idx, this_iid, this_index, this_offset)) { return phi; } } @@ -1400,7 +1400,7 @@ this_iid = base->_idx; } PhaseIterGVN* igvn = phase->is_IterGVN(); - Node* phi = new (C) PhiNode(region, this_type, NULL, this_iid, this_index, this_offset); + Node* phi = new (C) PhiNode(region, this_type, NULL, mem->_idx, this_iid, this_index, this_offset); for (uint i = 1; i < region->req(); i++) { Node* x; Node* the_clone = NULL; diff -r 072770c9a6b9 -r 3b6372514697 src/share/vm/opto/phaseX.cpp --- a/src/share/vm/opto/phaseX.cpp Fri Oct 21 20:34:04 2016 +0300 +++ b/src/share/vm/opto/phaseX.cpp Fri Oct 28 22:36:23 2016 +0000 @@ -481,6 +481,8 @@ uint current_idx = 0; // The current new node ID. Incremented after every assignment. for (uint i = 0; i < _useful.size(); i++) { Node* n = _useful.at(i); + // Sanity check that fails if we ever decide to execute this phase after EA + assert(!n->is_Phi() || n->as_Phi()->inst_mem_id() == -1, "should not be linked to data Phi"); const Type* type = gvn->type_or_null(n); new_type_array.map(current_idx, type); @@ -1378,6 +1380,18 @@ i -= num_edges; // we deleted 1 or more copies of this edge } + // Search for instance field data PhiNodes in the same region pointing to the old + // memory PhiNode and update their instance memory ids to point to the new node. + if (old->is_Phi() && old->as_Phi()->type()->has_memory() && old->in(0) != NULL) { + Node* region = old->in(0); + for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) { + PhiNode* phi = region->fast_out(i)->isa_Phi(); + if (phi != NULL && phi->inst_mem_id() == (int)old->_idx) { + phi->set_inst_mem_id((int)nn->_idx); + } + } + } + // Smash all inputs to 'old', isolating him completely Node *temp = new (C) Node(1); temp->init_req(0,nn); // Add a use to nn to prevent him from dying diff -r 072770c9a6b9 -r 3b6372514697 src/share/vm/opto/type.hpp --- a/src/share/vm/opto/type.hpp Fri Oct 21 20:34:04 2016 +0300 +++ b/src/share/vm/opto/type.hpp Fri Oct 28 22:36:23 2016 +0000 @@ -882,7 +882,7 @@ // If not InstanceTop or InstanceBot, indicates that this is // a particular instance of this type which is distinct. - // This is the the node index of the allocation node creating this instance. + // This is the node index of the allocation node creating this instance. int _instance_id; // Extra type information profiling gave us. We propagate it the