8069191: moving predicate out of loops may cause array accesses to bypass null check
authorroland
Tue, 24 Mar 2015 10:25:09 +0100
changeset 30300 4b12a5b40064
parent 30299 1f6f7d1e0c1e
child 30301 8ebb82d73cbc
8069191: moving predicate out of loops may cause array accesses to bypass null check Summary: Remove CastPP nodes only during final graph reshape Reviewed-by: kvn, jrose
hotspot/src/share/vm/opto/castnode.cpp
hotspot/src/share/vm/opto/castnode.hpp
hotspot/src/share/vm/opto/compile.cpp
hotspot/src/share/vm/opto/gcm.cpp
hotspot/src/share/vm/opto/matcher.cpp
hotspot/src/share/vm/opto/matcher.hpp
hotspot/src/share/vm/opto/memnode.cpp
hotspot/src/share/vm/opto/memnode.hpp
hotspot/src/share/vm/opto/narrowptrnode.cpp
hotspot/src/share/vm/opto/narrowptrnode.hpp
hotspot/src/share/vm/opto/node.cpp
hotspot/src/share/vm/opto/node.hpp
hotspot/src/share/vm/opto/phaseX.cpp
hotspot/test/compiler/loopopts/TestPredicateLostDependency.java
--- a/hotspot/src/share/vm/opto/castnode.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/castnode.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -73,16 +73,6 @@
   return (in(0) && remove_dead_region(phase, can_reshape)) ? this : NULL;
 }
 
-//------------------------------Ideal_DU_postCCP-------------------------------
-// Throw away cast after constant propagation
-Node *ConstraintCastNode::Ideal_DU_postCCP( PhaseCCP *ccp ) {
-  const Type *t = ccp->type(in(1));
-  ccp->hash_delete(this);
-  set_type(t);                   // Turn into ID function
-  ccp->hash_insert(this);
-  return this;
-}
-
 uint CastIINode::size_of() const {
   return sizeof(*this);
 }
@@ -164,13 +154,6 @@
   return res;
 }
 
-Node *CastIINode::Ideal_DU_postCCP(PhaseCCP *ccp) {
-  if (_carry_dependency) {
-    return NULL;
-  }
-  return ConstraintCastNode::Ideal_DU_postCCP(ccp);
-}
-
 #ifndef PRODUCT
 void CastIINode::dump_spec(outputStream *st) const {
   TypeNode::dump_spec(st);
@@ -181,20 +164,6 @@
 #endif
 
 //=============================================================================
-
-//------------------------------Ideal_DU_postCCP-------------------------------
-// If not converting int->oop, throw away cast after constant propagation
-Node *CastPPNode::Ideal_DU_postCCP( PhaseCCP *ccp ) {
-  const Type *t = ccp->type(in(1));
-  if (!t->isa_oop_ptr() || ((in(1)->is_DecodeN()) && Matcher::gen_narrow_oop_implicit_null_checks())) {
-    return NULL; // do not transform raw pointers or narrow oops
-  }
-  return ConstraintCastNode::Ideal_DU_postCCP(ccp);
-}
-
-
-
-//=============================================================================
 //------------------------------Identity---------------------------------------
 // If input is already higher or equal to cast type, then this is an identity.
 Node *CheckCastPPNode::Identity( PhaseTransform *phase ) {
--- a/hotspot/src/share/vm/opto/castnode.hpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/castnode.hpp	Tue Mar 24 10:25:09 2015 +0100
@@ -42,7 +42,6 @@
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
   virtual int Opcode() const;
   virtual uint ideal_reg() const = 0;
-  virtual Node *Ideal_DU_postCCP( PhaseCCP * );
 };
 
 //------------------------------CastIINode-------------------------------------
@@ -63,7 +62,6 @@
   virtual uint ideal_reg() const { return Op_RegI; }
   virtual Node *Identity( PhaseTransform *phase );
   virtual const Type *Value( PhaseTransform *phase ) const;
-  virtual Node *Ideal_DU_postCCP( PhaseCCP * );
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
 #endif
@@ -76,7 +74,6 @@
   CastPPNode (Node *n, const Type *t ): ConstraintCastNode(n, t) {}
   virtual int Opcode() const;
   virtual uint ideal_reg() const { return Op_RegP; }
-  virtual Node *Ideal_DU_postCCP( PhaseCCP * );
 };
 
 //------------------------------CheckCastPPNode--------------------------------
@@ -94,9 +91,6 @@
   virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
   virtual int   Opcode() const;
   virtual uint  ideal_reg() const { return Op_RegP; }
-  // No longer remove CheckCast after CCP as it gives me a place to hang
-  // the proper address type - which is required to compute anti-deps.
-  //virtual Node *Ideal_DU_postCCP( PhaseCCP * );
 };
 
 
--- a/hotspot/src/share/vm/opto/compile.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/compile.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -2811,9 +2811,38 @@
     break;
   }
 
-#ifdef _LP64
-  case Op_CastPP:
-    if (n->in(1)->is_DecodeN() && Matcher::gen_narrow_oop_implicit_null_checks()) {
+  case Op_CastPP: {
+    // Remove CastPP nodes to gain more freedom during scheduling but
+    // keep the dependency they encode as control or precedence edges
+    // (if control is set already) on memory operations. Some CastPP
+    // nodes don't have a control (don't carry a dependency): skip
+    // those.
+    if (n->in(0) != NULL) {
+      ResourceMark rm;
+      Unique_Node_List wq;
+      wq.push(n);
+      for (uint next = 0; next < wq.size(); ++next) {
+        Node *m = wq.at(next);
+        for (DUIterator_Fast imax, i = m->fast_outs(imax); i < imax; i++) {
+          Node* use = m->fast_out(i);
+          if (use->is_Mem() || use->is_EncodeNarrowPtr()) {
+            use->ensure_control_or_add_prec(n->in(0));
+          } else if (use->in(0) == NULL) {
+            switch(use->Opcode()) {
+            case Op_AddP:
+            case Op_DecodeN:
+            case Op_DecodeNKlass:
+            case Op_CheckCastPP:
+            case Op_CastPP:
+              wq.push(use);
+              break;
+            }
+          }
+        }
+      }
+    }
+    const bool is_LP64 = LP64_ONLY(true) NOT_LP64(false);
+    if (is_LP64 && n->in(1)->is_DecodeN() && Matcher::gen_narrow_oop_implicit_null_checks()) {
       Node* in1 = n->in(1);
       const Type* t = n->bottom_type();
       Node* new_in1 = in1->clone();
@@ -2846,9 +2875,15 @@
       if (in1->outcnt() == 0) {
         in1->disconnect_inputs(NULL, this);
       }
+    } else {
+      n->subsume_by(n->in(1), this);
+      if (n->outcnt() == 0) {
+        n->disconnect_inputs(NULL, this);
+      }
     }
     break;
-
+  }
+#ifdef _LP64
   case Op_CmpP:
     // Do this transformation here to preserve CmpPNode::sub() and
     // other TypePtr related Ideal optimizations (for example, ptr nullness).
--- a/hotspot/src/share/vm/opto/gcm.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/gcm.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -100,6 +100,9 @@
   }
 }
 
+static bool is_dominator(Block* d, Block* n) {
+  return d->dom_lca(n) == d;
+}
 
 //------------------------------schedule_pinned_nodes--------------------------
 // Set the basic block for Nodes pinned into blocks
@@ -122,6 +125,42 @@
         schedule_node_into_block(node, block);
       }
 
+      // If the node has precedence edges (added when CastPP nodes are
+      // removed in final_graph_reshaping), fix the control of the
+      // node to cover the precedence edges and remove the
+      // dependencies.
+      Node* n = NULL;
+      for (uint i = node->len()-1; i >= node->req(); i--) {
+        Node* m = node->in(i);
+        if (m == NULL) continue;
+        // Skip the precedence edge if the test that guarded a CastPP:
+        // - was optimized out during escape analysis
+        // (OptimizePtrCompare): the CastPP's control isn't an end of
+        // block.
+        // - is moved in the branch of a dominating If: the control of
+        // the CastPP is then a Region.
+        if (m->is_block_proj() || m->is_block_start()) {
+          node->rm_prec(i);
+          if (n == NULL) {
+            n = m;
+          } else {
+            Block* bn = get_block_for_node(n);
+            Block* bm = get_block_for_node(m);
+            assert(is_dominator(bn, bm) || is_dominator(bm, bn), "one must dominate the other");
+            n = is_dominator(bn, bm) ? m : n;
+          }
+        }
+      }
+      if (n != NULL) {
+        assert(node->in(0), "control should have been set");
+        Block* bn = get_block_for_node(n);
+        Block* bnode = get_block_for_node(node->in(0));
+        assert(is_dominator(bn, bnode) || is_dominator(bnode, bn), "one must dominate the other");
+        if (!is_dominator(bn, bnode)) {
+          node->set_req(0, n);
+        }
+      }
+
       // process all inputs that are non NULL
       for (int i = node->req() - 1; i >= 0; --i) {
         if (node->in(i) != NULL) {
--- a/hotspot/src/share/vm/opto/matcher.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/matcher.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -1049,6 +1049,15 @@
         mstack.push(m, Visit, n, -1);
       }
 
+      // Handle precedence edges for interior nodes
+      for (i = n->len()-1; (uint)i >= n->req(); i--) {
+        Node *m = n->in(i);
+        if (m == NULL || C->node_arena()->contains(m)) continue;
+        n->rm_prec(i);
+        // set -1 to call add_prec() instead of set_req() during Step1
+        mstack.push(m, Visit, n, -1);
+      }
+
       // For constant debug info, I'd rather have unmatched constants.
       int cnt = n->req();
       JVMState* jvms = n->jvms();
@@ -1738,6 +1747,14 @@
   return ex;
 }
 
+void Matcher::handle_precedence_edges(Node* n, MachNode *mach) {
+  for (uint i = n->req(); i < n->len(); i++) {
+    if (n->in(i) != NULL) {
+      mach->add_prec(n->in(i));
+    }
+  }
+}
+
 void Matcher::ReduceInst_Chain_Rule( State *s, int rule, Node *&mem, MachNode *mach ) {
   // 'op' is what I am expecting to receive
   int op = _leftOp[rule];
@@ -1772,6 +1789,8 @@
 
 
 uint Matcher::ReduceInst_Interior( State *s, int rule, Node *&mem, MachNode *mach, uint num_opnds ) {
+  handle_precedence_edges(s->_leaf, mach);
+
   if( s->_leaf->is_Load() ) {
     Node *mem2 = s->_leaf->in(MemNode::Memory);
     assert( mem == (Node*)1 || mem == mem2, "multiple Memories being matched at once?" );
@@ -1854,6 +1873,9 @@
     mem = s->_leaf->in(MemNode::Memory);
     debug_only(_mem_node = s->_leaf;)
   }
+
+  handle_precedence_edges(s->_leaf, mach);
+
   if( s->_leaf->in(0) && s->_leaf->req() > 1) {
     if( !mach->in(0) )
       mach->set_req(0,s->_leaf->in(0));
--- a/hotspot/src/share/vm/opto/matcher.hpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/matcher.hpp	Tue Mar 24 10:25:09 2015 +0100
@@ -124,6 +124,8 @@
   // Mach node for ConP #NULL
   MachNode* _mach_null;
 
+  void handle_precedence_edges(Node* n, MachNode *mach);
+
 public:
   int LabelRootDepth;
   // Convert ideal machine register to a register mask for spill-loads
--- a/hotspot/src/share/vm/opto/memnode.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/memnode.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -652,216 +652,6 @@
   }
 }
 
-//------------------------adr_phi_is_loop_invariant----------------------------
-// A helper function for Ideal_DU_postCCP to check if a Phi in a counted
-// loop is loop invariant. Make a quick traversal of Phi and associated
-// CastPP nodes, looking to see if they are a closed group within the loop.
-bool MemNode::adr_phi_is_loop_invariant(Node* adr_phi, Node* cast) {
-  // The idea is that the phi-nest must boil down to only CastPP nodes
-  // with the same data. This implies that any path into the loop already
-  // includes such a CastPP, and so the original cast, whatever its input,
-  // must be covered by an equivalent cast, with an earlier control input.
-  ResourceMark rm;
-
-  // The loop entry input of the phi should be the unique dominating
-  // node for every Phi/CastPP in the loop.
-  Unique_Node_List closure;
-  closure.push(adr_phi->in(LoopNode::EntryControl));
-
-  // Add the phi node and the cast to the worklist.
-  Unique_Node_List worklist;
-  worklist.push(adr_phi);
-  if( cast != NULL ){
-    if( !cast->is_ConstraintCast() ) return false;
-    worklist.push(cast);
-  }
-
-  // Begin recursive walk of phi nodes.
-  while( worklist.size() ){
-    // Take a node off the worklist
-    Node *n = worklist.pop();
-    if( !closure.member(n) ){
-      // Add it to the closure.
-      closure.push(n);
-      // Make a sanity check to ensure we don't waste too much time here.
-      if( closure.size() > 20) return false;
-      // This node is OK if:
-      //  - it is a cast of an identical value
-      //  - or it is a phi node (then we add its inputs to the worklist)
-      // Otherwise, the node is not OK, and we presume the cast is not invariant
-      if( n->is_ConstraintCast() ){
-        worklist.push(n->in(1));
-      } else if( n->is_Phi() ) {
-        for( uint i = 1; i < n->req(); i++ ) {
-          worklist.push(n->in(i));
-        }
-      } else {
-        return false;
-      }
-    }
-  }
-
-  // Quit when the worklist is empty, and we've found no offending nodes.
-  return true;
-}
-
-//------------------------------Ideal_DU_postCCP-------------------------------
-// Find any cast-away of null-ness and keep its control.  Null cast-aways are
-// going away in this pass and we need to make this memory op depend on the
-// gating null check.
-Node *MemNode::Ideal_DU_postCCP( PhaseCCP *ccp ) {
-  return Ideal_common_DU_postCCP(ccp, this, in(MemNode::Address));
-}
-
-// I tried to leave the CastPP's in.  This makes the graph more accurate in
-// some sense; we get to keep around the knowledge that an oop is not-null
-// after some test.  Alas, the CastPP's interfere with GVN (some values are
-// the regular oop, some are the CastPP of the oop, all merge at Phi's which
-// cannot collapse, etc).  This cost us 10% on SpecJVM, even when I removed
-// some of the more trivial cases in the optimizer.  Removing more useless
-// Phi's started allowing Loads to illegally float above null checks.  I gave
-// up on this approach.  CNC 10/20/2000
-// This static method may be called not from MemNode (EncodePNode calls it).
-// Only the control edge of the node 'n' might be updated.
-Node *MemNode::Ideal_common_DU_postCCP( PhaseCCP *ccp, Node* n, Node* adr ) {
-  Node *skipped_cast = NULL;
-  // Need a null check?  Regular static accesses do not because they are
-  // from constant addresses.  Array ops are gated by the range check (which
-  // always includes a NULL check).  Just check field ops.
-  if( n->in(MemNode::Control) == NULL ) {
-    // Scan upwards for the highest location we can place this memory op.
-    while( true ) {
-      switch( adr->Opcode() ) {
-
-      case Op_AddP:             // No change to NULL-ness, so peek thru AddP's
-        adr = adr->in(AddPNode::Base);
-        continue;
-
-      case Op_DecodeN:         // No change to NULL-ness, so peek thru
-      case Op_DecodeNKlass:
-        adr = adr->in(1);
-        continue;
-
-      case Op_EncodeP:
-      case Op_EncodePKlass:
-        // EncodeP node's control edge could be set by this method
-        // when EncodeP node depends on CastPP node.
-        //
-        // Use its control edge for memory op because EncodeP may go away
-        // later when it is folded with following or preceding DecodeN node.
-        if (adr->in(0) == NULL) {
-          // Keep looking for cast nodes.
-          adr = adr->in(1);
-          continue;
-        }
-        ccp->hash_delete(n);
-        n->set_req(MemNode::Control, adr->in(0));
-        ccp->hash_insert(n);
-        return n;
-
-      case Op_CastPP:
-        // If the CastPP is useless, just peek on through it.
-        if( ccp->type(adr) == ccp->type(adr->in(1)) ) {
-          // Remember the cast that we've peeked though. If we peek
-          // through more than one, then we end up remembering the highest
-          // one, that is, if in a loop, the one closest to the top.
-          skipped_cast = adr;
-          adr = adr->in(1);
-          continue;
-        }
-        // CastPP is going away in this pass!  We need this memory op to be
-        // control-dependent on the test that is guarding the CastPP.
-        ccp->hash_delete(n);
-        n->set_req(MemNode::Control, adr->in(0));
-        ccp->hash_insert(n);
-        return n;
-
-      case Op_Phi:
-        // Attempt to float above a Phi to some dominating point.
-        if (adr->in(0) != NULL && adr->in(0)->is_CountedLoop()) {
-          // If we've already peeked through a Cast (which could have set the
-          // control), we can't float above a Phi, because the skipped Cast
-          // may not be loop invariant.
-          if (adr_phi_is_loop_invariant(adr, skipped_cast)) {
-            adr = adr->in(1);
-            continue;
-          }
-        }
-
-        // Intentional fallthrough!
-
-        // No obvious dominating point.  The mem op is pinned below the Phi
-        // by the Phi itself.  If the Phi goes away (no true value is merged)
-        // then the mem op can float, but not indefinitely.  It must be pinned
-        // behind the controls leading to the Phi.
-      case Op_CheckCastPP:
-        // These usually stick around to change address type, however a
-        // useless one can be elided and we still need to pick up a control edge
-        if (adr->in(0) == NULL) {
-          // This CheckCastPP node has NO control and is likely useless. But we
-          // need check further up the ancestor chain for a control input to keep
-          // the node in place. 4959717.
-          skipped_cast = adr;
-          adr = adr->in(1);
-          continue;
-        }
-        ccp->hash_delete(n);
-        n->set_req(MemNode::Control, adr->in(0));
-        ccp->hash_insert(n);
-        return n;
-
-        // List of "safe" opcodes; those that implicitly block the memory
-        // op below any null check.
-      case Op_CastX2P:          // no null checks on native pointers
-      case Op_Parm:             // 'this' pointer is not null
-      case Op_LoadP:            // Loading from within a klass
-      case Op_LoadN:            // Loading from within a klass
-      case Op_LoadKlass:        // Loading from within a klass
-      case Op_LoadNKlass:       // Loading from within a klass
-      case Op_ConP:             // Loading from a klass
-      case Op_ConN:             // Loading from a klass
-      case Op_ConNKlass:        // Loading from a klass
-      case Op_CreateEx:         // Sucking up the guts of an exception oop
-      case Op_Con:              // Reading from TLS
-      case Op_CMoveP:           // CMoveP is pinned
-      case Op_CMoveN:           // CMoveN is pinned
-        break;                  // No progress
-
-      case Op_Proj:             // Direct call to an allocation routine
-      case Op_SCMemProj:        // Memory state from store conditional ops
-#ifdef ASSERT
-        {
-          assert(adr->as_Proj()->_con == TypeFunc::Parms, "must be return value");
-          const Node* call = adr->in(0);
-          if (call->is_CallJava()) {
-            const CallJavaNode* call_java = call->as_CallJava();
-            const TypeTuple *r = call_java->tf()->range();
-            assert(r->cnt() > TypeFunc::Parms, "must return value");
-            const Type* ret_type = r->field_at(TypeFunc::Parms);
-            assert(ret_type && ret_type->isa_ptr(), "must return pointer");
-            // We further presume that this is one of
-            // new_instance_Java, new_array_Java, or
-            // the like, but do not assert for this.
-          } else if (call->is_Allocate()) {
-            // similar case to new_instance_Java, etc.
-          } else if (!call->is_CallLeaf()) {
-            // Projections from fetch_oop (OSR) are allowed as well.
-            ShouldNotReachHere();
-          }
-        }
-#endif
-        break;
-      default:
-        ShouldNotReachHere();
-      }
-      break;
-    }
-  }
-
-  return  NULL;               // No progress
-}
-
-
 //=============================================================================
 // Should LoadNode::Ideal() attempt to remove control edges?
 bool LoadNode::can_remove_control() const {
--- a/hotspot/src/share/vm/opto/memnode.hpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/memnode.hpp	Tue Mar 24 10:25:09 2015 +0100
@@ -84,10 +84,6 @@
   // This one should probably be a phase-specific function:
   static bool all_controls_dominate(Node* dom, Node* sub);
 
-  // Find any cast-away of null-ness and keep its control.
-  static  Node *Ideal_common_DU_postCCP( PhaseCCP *ccp, Node* n, Node* adr );
-  virtual Node *Ideal_DU_postCCP( PhaseCCP *ccp );
-
   virtual const class TypePtr *adr_type() const;  // returns bottom_type of address
 
   // Shared code for Ideal methods:
--- a/hotspot/src/share/vm/opto/narrowptrnode.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/narrowptrnode.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -67,10 +67,6 @@
 }
 
 
-Node *EncodeNarrowPtrNode::Ideal_DU_postCCP( PhaseCCP *ccp ) {
-  return MemNode::Ideal_common_DU_postCCP(ccp, this, in(1));
-}
-
 Node* DecodeNKlassNode::Identity(PhaseTransform* phase) {
   const Type *t = phase->type( in(1) );
   if( t == Type::TOP ) return in(1);
--- a/hotspot/src/share/vm/opto/narrowptrnode.hpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/narrowptrnode.hpp	Tue Mar 24 10:25:09 2015 +0100
@@ -39,7 +39,6 @@
   }
   public:
   virtual uint  ideal_reg() const { return Op_RegN; }
-  virtual Node *Ideal_DU_postCCP( PhaseCCP *ccp );
 };
 
 //------------------------------EncodeP--------------------------------
--- a/hotspot/src/share/vm/opto/node.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/node.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -1387,12 +1387,6 @@
   return false;
 }
 
-//------------------------------Ideal_DU_postCCP-------------------------------
-// Idealize graph, using DU info.  Must clone result into new-space
-Node *Node::Ideal_DU_postCCP( PhaseCCP * ) {
-  return NULL;                 // Default to no change
-}
-
 //------------------------------hash-------------------------------------------
 // Hash function over Nodes.
 uint Node::hash() const {
@@ -2081,6 +2075,14 @@
   return found;
 }
 
+void Node::ensure_control_or_add_prec(Node* c) {
+  if (in(0) == NULL) {
+    set_req(0, c);
+  } else if (in(0) != c) {
+    add_prec(c);
+  }
+}
+
 //=============================================================================
 //------------------------------yank-------------------------------------------
 // Find and remove
--- a/hotspot/src/share/vm/opto/node.hpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/node.hpp	Tue Mar 24 10:25:09 2015 +0100
@@ -906,9 +906,6 @@
   bool remove_dead_region(PhaseGVN *phase, bool can_reshape);
 public:
 
-  // Idealize graph, using DU info.  Done after constant propagation
-  virtual Node *Ideal_DU_postCCP( PhaseCCP *ccp );
-
   // See if there is valid pipeline info
   static  const Pipeline *pipeline_class();
   virtual const Pipeline *pipeline() const;
@@ -942,6 +939,9 @@
   // Return the unique control out if only one. Null if none or more than one.
   Node* unique_ctrl_out() const;
 
+  // Set control or add control as precedence edge
+  void ensure_control_or_add_prec(Node* c);
+
 //----------------- Code Generation
 
   // Ideal register class for Matching.  Zero means unmatched instruction
--- a/hotspot/src/share/vm/opto/phaseX.cpp	Fri Apr 17 17:39:19 2015 -0700
+++ b/hotspot/src/share/vm/opto/phaseX.cpp	Tue Mar 24 10:25:09 2015 +0100
@@ -1605,21 +1605,6 @@
   C->set_root( transform(C->root())->as_Root() );
   assert( C->top(),  "missing TOP node" );
   assert( C->root(), "missing root" );
-
-  // Eagerly remove castPP nodes here. CastPP nodes might not be
-  // removed in the subsequent IGVN phase if a node that changes
-  // in(1) of a castPP is processed prior to the castPP node.
-  for (uint i = 0; i < _worklist.size(); i++) {
-    Node* n = _worklist.at(i);
-
-    if (n->is_ConstraintCast()) {
-      Node* nn = n->Identity(this);
-      if (nn != n) {
-        replace_node(n, nn);
-        --i;
-      }
-    }
-  }
 }
 
 //------------------------------transform--------------------------------------
@@ -1700,11 +1685,6 @@
     _worklist.push(n);          // n re-enters the hash table via the worklist
   }
 
-  // Idealize graph using DU info.  Must clone() into new-space.
-  // DU info is generally used to show profitability, progress or safety
-  // (but generally not needed for correctness).
-  Node *nn = n->Ideal_DU_postCCP(this);
-
   // TEMPORARY fix to ensure that 2nd GVN pass eliminates NULL checks
   switch( n->Opcode() ) {
   case Op_FastLock:      // Revisit FastLocks for lock coarsening
@@ -1721,12 +1701,6 @@
   default:
     break;
   }
-  if( nn ) {
-    _worklist.push(n);
-    // Put users of 'n' onto worklist for second igvn transform
-    add_users_to_worklist(n);
-    return nn;
-  }
 
   return  n;
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/loopopts/TestPredicateLostDependency.java	Tue Mar 24 10:25:09 2015 +0100
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 8069191
+ * @summary predicate moved out of loops and CastPP removal causes dependency to be lost
+ * @run main/othervm -Xcomp -XX:CompileOnly=TestPredicateLostDependency.m1 -XX:+IgnoreUnrecognizedVMOptions -XX:+StressGCM TestPredicateLostDependency
+ *
+ */
+
+public class TestPredicateLostDependency {
+    static class A {
+        int i;
+    }
+
+    static class B extends A {
+    }
+
+    static boolean crash = false;
+
+    static boolean m2() {
+        return crash;
+    }
+
+    static int m3(float[] arr) {
+        return 0;
+    }
+
+    static float m1(A aa) {
+        float res = 0;
+        float[] arr = new float[10];
+        for (int i = 0; i < 10; i++) {
+            if (m2()) {
+                arr = null;
+            }
+            m3(arr);
+            int j = arr.length;
+            int k = 0;
+            for (k = 9; k < j; k++) {
+            }
+            if (k == 10) {
+                if (aa instanceof B) {
+                }
+            }
+            res += arr[0];
+            res += arr[1];
+        }
+        return res;
+    }
+
+    static public void main(String args[]) {
+        A a = new A();
+        B b = new B();
+        for (int i = 0; i < 20000; i++) {
+            m1(a);
+        }
+        crash = true;
+        try {
+            m1(a);
+        } catch (NullPointerException npe) {}
+    }
+}