8143542: C2 doesn't eliminate identical checks
authorroland
Wed, 03 Feb 2016 12:36:18 +0100
changeset 36065 4f0e0cb7b016
parent 36064 20a60a2de977
child 36066 60ce66ce3c76
8143542: C2 doesn't eliminate identical checks Summary: Two identical Ifs back to back can be merged Reviewed-by: kvn
hotspot/src/share/vm/opto/castnode.cpp
hotspot/src/share/vm/opto/loopnode.hpp
hotspot/src/share/vm/opto/loopopts.cpp
--- a/hotspot/src/share/vm/opto/castnode.cpp	Fri Feb 12 12:18:44 2016 +0100
+++ b/hotspot/src/share/vm/opto/castnode.cpp	Wed Feb 03 12:36:18 2016 +0100
@@ -37,7 +37,6 @@
 Node* ConstraintCastNode::Identity(PhaseGVN* phase) {
   Node* dom = dominating_cast(phase);
   if (dom != NULL) {
-    assert(_carry_dependency, "only for casts that carry a dependency");
     return dom;
   }
   if (_carry_dependency) {
@@ -110,15 +109,18 @@
 }
 
 TypeNode* ConstraintCastNode::dominating_cast(PhaseTransform *phase) const {
-  if (!carry_dependency()) {
-    return NULL;
-  }
   Node* val = in(1);
   Node* ctl = in(0);
   int opc = Opcode();
   if (ctl == NULL) {
     return NULL;
   }
+  // Range check CastIIs may all end up under a single range check and
+  // in that case only the narrower CastII would be kept by the code
+  // below which would be incorrect.
+  if (is_CastII() && as_CastII()->has_range_check()) {
+    return NULL;
+  }
   for (DUIterator_Fast imax, i = val->fast_outs(imax); i < imax; i++) {
     Node* u = val->fast_out(i);
     if (u != this &&
@@ -300,7 +302,6 @@
 Node* CheckCastPPNode::Identity(PhaseGVN* phase) {
   Node* dom = dominating_cast(phase);
   if (dom != NULL) {
-    assert(_carry_dependency, "only for casts that carry a dependency");
     return dom;
   }
   if (_carry_dependency) {
--- a/hotspot/src/share/vm/opto/loopnode.hpp	Fri Feb 12 12:18:44 2016 +0100
+++ b/hotspot/src/share/vm/opto/loopnode.hpp	Wed Feb 03 12:36:18 2016 +0100
@@ -1105,6 +1105,8 @@
   Node *place_near_use( Node *useblock ) const;
   Node* try_move_store_before_loop(Node* n, Node *n_ctrl);
   void try_move_store_after_loop(Node* n);
+  bool identical_backtoback_ifs(Node *n);
+  bool can_split_if(Node *n_ctrl);
 
   bool _created_loop_node;
 public:
--- a/hotspot/src/share/vm/opto/loopopts.cpp	Fri Feb 12 12:18:44 2016 +0100
+++ b/hotspot/src/share/vm/opto/loopopts.cpp	Wed Feb 03 12:36:18 2016 +0100
@@ -1020,108 +1020,193 @@
 }
 
 
+bool PhaseIdealLoop::identical_backtoback_ifs(Node *n) {
+  if (!n->is_If()) {
+    return false;
+  }
+  if (!n->in(0)->is_Region()) {
+    return false;
+  }
+  Node* region = n->in(0);
+  Node* dom = idom(region);
+  if (!dom->is_If() || dom->in(1) != n->in(1)) {
+    return false;
+  }
+  IfNode* dom_if = dom->as_If();
+  Node* proj_true = dom_if->proj_out(1);
+  Node* proj_false = dom_if->proj_out(0);
+
+  for (uint i = 1; i < region->req(); i++) {
+    if (is_dominator(proj_true, region->in(i))) {
+      continue;
+    }
+    if (is_dominator(proj_false, region->in(i))) {
+      continue;
+    }
+    return false;
+  }
+
+  return true;
+}
+
+bool PhaseIdealLoop::can_split_if(Node *n_ctrl) {
+  if (C->live_nodes() > 35000) {
+    return false; // Method too big
+  }
+
+  // Do not do 'split-if' if irreducible loops are present.
+  if (_has_irreducible_loops) {
+    return false;
+  }
+
+  if (merge_point_too_heavy(C, n_ctrl)) {
+    return false;
+  }
+
+  // Do not do 'split-if' if some paths are dead.  First do dead code
+  // elimination and then see if its still profitable.
+  for (uint i = 1; i < n_ctrl->req(); i++) {
+    if (n_ctrl->in(i) == C->top()) {
+      return false;
+    }
+  }
+
+  // If trying to do a 'Split-If' at the loop head, it is only
+  // profitable if the cmp folds up on BOTH paths.  Otherwise we
+  // risk peeling a loop forever.
+
+  // CNC - Disabled for now.  Requires careful handling of loop
+  // body selection for the cloned code.  Also, make sure we check
+  // for any input path not being in the same loop as n_ctrl.  For
+  // irreducible loops we cannot check for 'n_ctrl->is_Loop()'
+  // because the alternative loop entry points won't be converted
+  // into LoopNodes.
+  IdealLoopTree *n_loop = get_loop(n_ctrl);
+  for (uint j = 1; j < n_ctrl->req(); j++) {
+    if (get_loop(n_ctrl->in(j)) != n_loop) {
+      return false;
+    }
+  }
+
+  // Check for safety of the merge point.
+  if (!merge_point_safe(n_ctrl)) {
+    return false;
+  }
+
+  return true;
+}
+
 //------------------------------split_if_with_blocks_post----------------------
 // Do the real work in a non-recursive function.  CFG hackery wants to be
 // in the post-order, so it can dirty the I-DOM info and not use the dirtied
 // info.
-void PhaseIdealLoop::split_if_with_blocks_post( Node *n ) {
+void PhaseIdealLoop::split_if_with_blocks_post(Node *n) {
 
   // Cloning Cmp through Phi's involves the split-if transform.
   // FastLock is not used by an If
-  if( n->is_Cmp() && !n->is_FastLock() ) {
-    if( C->live_nodes() > 35000 ) return; // Method too big
-
-    // Do not do 'split-if' if irreducible loops are present.
-    if( _has_irreducible_loops )
-      return;
-
+  if (n->is_Cmp() && !n->is_FastLock()) {
     Node *n_ctrl = get_ctrl(n);
     // Determine if the Node has inputs from some local Phi.
     // Returns the block to clone thru.
-    Node *n_blk = has_local_phi_input( n );
-    if( n_blk != n_ctrl ) return;
+    Node *n_blk = has_local_phi_input(n);
+    if (n_blk != n_ctrl) {
+      return;
+    }
 
-    if( merge_point_too_heavy(C, n_ctrl) )
+    if (!can_split_if(n_ctrl)) {
       return;
+    }
 
-    if( n->outcnt() != 1 ) return; // Multiple bool's from 1 compare?
+    if (n->outcnt() != 1) {
+      return; // Multiple bool's from 1 compare?
+    }
     Node *bol = n->unique_out();
-    assert( bol->is_Bool(), "expect a bool here" );
-    if( bol->outcnt() != 1 ) return;// Multiple branches from 1 compare?
+    assert(bol->is_Bool(), "expect a bool here");
+    if (bol->outcnt() != 1) {
+      return;// Multiple branches from 1 compare?
+    }
     Node *iff = bol->unique_out();
 
     // Check some safety conditions
-    if( iff->is_If() ) {        // Classic split-if?
-      if( iff->in(0) != n_ctrl ) return; // Compare must be in same blk as if
+    if (iff->is_If()) {        // Classic split-if?
+      if (iff->in(0) != n_ctrl) {
+        return; // Compare must be in same blk as if
+      }
     } else if (iff->is_CMove()) { // Trying to split-up a CMOVE
       // Can't split CMove with different control edge.
-      if (iff->in(0) != NULL && iff->in(0) != n_ctrl ) return;
-      if( get_ctrl(iff->in(2)) == n_ctrl ||
-          get_ctrl(iff->in(3)) == n_ctrl )
+      if (iff->in(0) != NULL && iff->in(0) != n_ctrl ) {
+        return;
+      }
+      if (get_ctrl(iff->in(2)) == n_ctrl ||
+          get_ctrl(iff->in(3)) == n_ctrl) {
         return;                 // Inputs not yet split-up
-      if ( get_loop(n_ctrl) != get_loop(get_ctrl(iff)) ) {
+      }
+      if (get_loop(n_ctrl) != get_loop(get_ctrl(iff))) {
         return;                 // Loop-invar test gates loop-varying CMOVE
       }
     } else {
       return;  // some other kind of node, such as an Allocate
     }
 
-    // Do not do 'split-if' if some paths are dead.  First do dead code
-    // elimination and then see if its still profitable.
-    for( uint i = 1; i < n_ctrl->req(); i++ )
-      if( n_ctrl->in(i) == C->top() )
-        return;
-
     // When is split-if profitable?  Every 'win' on means some control flow
     // goes dead, so it's almost always a win.
     int policy = 0;
-    // If trying to do a 'Split-If' at the loop head, it is only
-    // profitable if the cmp folds up on BOTH paths.  Otherwise we
-    // risk peeling a loop forever.
-
-    // CNC - Disabled for now.  Requires careful handling of loop
-    // body selection for the cloned code.  Also, make sure we check
-    // for any input path not being in the same loop as n_ctrl.  For
-    // irreducible loops we cannot check for 'n_ctrl->is_Loop()'
-    // because the alternative loop entry points won't be converted
-    // into LoopNodes.
-    IdealLoopTree *n_loop = get_loop(n_ctrl);
-    for( uint j = 1; j < n_ctrl->req(); j++ )
-      if( get_loop(n_ctrl->in(j)) != n_loop )
-        return;
-
-    // Check for safety of the merge point.
-    if( !merge_point_safe(n_ctrl) ) {
+    // Split compare 'n' through the merge point if it is profitable
+    Node *phi = split_thru_phi( n, n_ctrl, policy);
+    if (!phi) {
       return;
     }
 
-    // Split compare 'n' through the merge point if it is profitable
-    Node *phi = split_thru_phi( n, n_ctrl, policy );
-    if( !phi ) return;
-
     // Found a Phi to split thru!
     // Replace 'n' with the new phi
-    _igvn.replace_node( n, phi );
+    _igvn.replace_node(n, phi);
 
     // Now split the bool up thru the phi
-    Node *bolphi = split_thru_phi( bol, n_ctrl, -1 );
+    Node *bolphi = split_thru_phi(bol, n_ctrl, -1);
     guarantee(bolphi != NULL, "null boolean phi node");
 
-    _igvn.replace_node( bol, bolphi );
-    assert( iff->in(1) == bolphi, "" );
+    _igvn.replace_node(bol, bolphi);
+    assert(iff->in(1) == bolphi, "");
 
-    if( bolphi->Value(&_igvn)->singleton() )
+    if (bolphi->Value(&_igvn)->singleton()) {
       return;
+    }
 
     // Conditional-move?  Must split up now
-    if( !iff->is_If() ) {
-      Node *cmovphi = split_thru_phi( iff, n_ctrl, -1 );
-      _igvn.replace_node( iff, cmovphi );
+    if (!iff->is_If()) {
+      Node *cmovphi = split_thru_phi(iff, n_ctrl, -1);
+      _igvn.replace_node(iff, cmovphi);
       return;
     }
 
     // Now split the IF
-    do_split_if( iff );
+    do_split_if(iff);
+    return;
+  }
+
+  // Two identical ifs back to back can be merged
+  if (identical_backtoback_ifs(n) && can_split_if(n->in(0))) {
+    Node *n_ctrl = n->in(0);
+    PhiNode* bolphi = PhiNode::make_blank(n_ctrl, n->in(1));
+    IfNode* dom_if = idom(n_ctrl)->as_If();
+    Node* proj_true = dom_if->proj_out(1);
+    Node* proj_false = dom_if->proj_out(0);
+    Node* con_true = _igvn.makecon(TypeInt::ONE);
+    Node* con_false = _igvn.makecon(TypeInt::ZERO);
+
+    for (uint i = 1; i < n_ctrl->req(); i++) {
+      if (is_dominator(proj_true, n_ctrl->in(i))) {
+        bolphi->init_req(i, con_true);
+      } else {
+        assert(is_dominator(proj_false, n_ctrl->in(i)), "bad if");
+        bolphi->init_req(i, con_false);
+      }
+    }
+    register_new_node(bolphi, n_ctrl);
+    _igvn.replace_input_of(n, 1, bolphi);
+
+    // Now split the IF
+    do_split_if(n);
     return;
   }