8038348: Instance field load is replaced by wrong data Phi

Fri, 28 Oct 2016 22:36:23 +0000

author
poonam
date
Fri, 28 Oct 2016 22:36:23 +0000
changeset 8945
3b6372514697
parent 8944
072770c9a6b9
child 8946
e59c874298de

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

src/share/vm/opto/cfgnode.hpp file | annotate | diff | comparison | revisions
src/share/vm/opto/macro.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/memnode.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/phaseX.cpp file | annotate | diff | comparison | revisions
src/share/vm/opto/type.hpp file | annotate | diff | comparison | revisions
     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

mercurial