diff mbox

Handle __builtin_unreachable () using assertions in VRP

Message ID 20131025090640.GF30970@tucnak.zalov.cz
State New
Headers show

Commit Message

Jakub Jelinek Oct. 25, 2013, 9:06 a.m. UTC
Hi!

As discussed on IRC, this patch attempts to preserve VRP computed
range info for some simple __builtin_unreachable () using assertions.
If there are no immediate uses of some SSA_NAME except for those in
a condition guarding __builtin_unreachable () and in ASSERT_EXPR
in the following basic block, we can copy the range info from the assert
lhs (which is lost during remove_range_assertions) and thus preserve it.
Eventually we might then remove the __builtin_unreachable () in that case
and still benefit from user annotating the source.
In the testcase below in bar we have even without this patch the expected
range info on the SSA_NAME set to x + 1, but in foo there was no range
info preserved.  As we have just one user of get_range_info right now,
not creating a testcase.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

int
foo (int x)
{
  if (x < 26 || x > 37)
    __builtin_unreachable ();
  return x;
}

int
bar (int x)
{
  if (x < 26 || x > 37)
    __builtin_unreachable ();
  return x + 1;
}

2013-10-25  Jakub Jelinek  <jakub@redhat.com>

	* tree-vrp.c (remove_range_assertions): If ASSERT_EXPR_VAR
	has no other immediate uses but in the condition and ASSERT_EXPR
	and the other successor of the predecessor bb is
	__builtin_unreachable (), set_range_info of the ASSERT_EXPR_VAR
	to the range info of the ASSERT_EXPR's lhs.


	Jakub

Comments

Richard Biener Oct. 29, 2013, 11:28 a.m. UTC | #1
On Fri, 25 Oct 2013, Jakub Jelinek wrote:

> Hi!
> 
> As discussed on IRC, this patch attempts to preserve VRP computed
> range info for some simple __builtin_unreachable () using assertions.
> If there are no immediate uses of some SSA_NAME except for those in
> a condition guarding __builtin_unreachable () and in ASSERT_EXPR
> in the following basic block, we can copy the range info from the assert
> lhs (which is lost during remove_range_assertions) and thus preserve it.
> Eventually we might then remove the __builtin_unreachable () in that case
> and still benefit from user annotating the source.
> In the testcase below in bar we have even without this patch the expected
> range info on the SSA_NAME set to x + 1, but in foo there was no range
> info preserved.  As we have just one user of get_range_info right now,
> not creating a testcase.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> int
> foo (int x)
> {
>   if (x < 26 || x > 37)
>     __builtin_unreachable ();
>   return x;
> }
> 
> int
> bar (int x)
> {
>   if (x < 26 || x > 37)
>     __builtin_unreachable ();
>   return x + 1;
> }
> 
> 2013-10-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* tree-vrp.c (remove_range_assertions): If ASSERT_EXPR_VAR
> 	has no other immediate uses but in the condition and ASSERT_EXPR
> 	and the other successor of the predecessor bb is
> 	__builtin_unreachable (), set_range_info of the ASSERT_EXPR_VAR
> 	to the range info of the ASSERT_EXPR's lhs.
> 
> --- gcc/tree-vrp.c.jj	2013-10-24 10:19:21.000000000 +0200
> +++ gcc/tree-vrp.c	2013-10-24 14:32:29.065878208 +0200
> @@ -6488,12 +6488,16 @@ remove_range_assertions (void)
>  {
>    basic_block bb;
>    gimple_stmt_iterator si;
> +  /* 1 if looking at ASSERT_EXPRs immediately at the beginning of
> +     a basic block preceeded by GIMPLE_COND branching to it and
> +     __builtin_trap, -1 if not yet checked, 0 otherwise.  */
> +  int is_unreachable;
>  
>    /* Note that the BSI iterator bump happens at the bottom of the
>       loop and no bump is necessary if we're removing the statement
>       referenced by the current BSI.  */
>    FOR_EACH_BB (bb)
> -    for (si = gsi_start_bb (bb); !gsi_end_p (si);)
> +    for (si = gsi_after_labels (bb), is_unreachable = -1; !gsi_end_p (si);)
>        {
>  	gimple stmt = gsi_stmt (si);
>  	gimple use_stmt;
> @@ -6501,30 +6505,96 @@ remove_range_assertions (void)
>  	if (is_gimple_assign (stmt)
>  	    && gimple_assign_rhs_code (stmt) == ASSERT_EXPR)
>  	  {
> +	    tree lhs = gimple_assign_lhs (stmt);
>  	    tree rhs = gimple_assign_rhs1 (stmt);
>  	    tree var;
>  	    tree cond = fold (ASSERT_EXPR_COND (rhs));
> -	    use_operand_p use_p;
> +	    use_operand_p use_p, use2_p;
>  	    imm_use_iterator iter;
>  
>  	    gcc_assert (cond != boolean_false_node);
>  
> -	    /* Propagate the RHS into every use of the LHS.  */
>  	    var = ASSERT_EXPR_VAR (rhs);
> -	    FOR_EACH_IMM_USE_STMT (use_stmt, iter,
> -				   gimple_assign_lhs (stmt))
> +	    gcc_assert (TREE_CODE (var) == SSA_NAME);
> +
> +	    if (!POINTER_TYPE_P (TREE_TYPE (lhs))
> +		&& SSA_NAME_RANGE_INFO (lhs))
> +	      {
> +		if (is_unreachable == -1)
> +		  {
> +		    is_unreachable = 0;
> +		    if (single_pred_p (bb))
> +		      {
> +			basic_block pred_bb = single_pred (bb);
> +			gimple last = last_stmt (pred_bb);
> +			if (last && gimple_code (last) == GIMPLE_COND)
> +			  {
> +			    basic_block other_bb
> +			      = EDGE_SUCC (pred_bb, 0)->dest;
> +			    if (other_bb == bb)
> +			      other_bb = EDGE_SUCC (pred_bb, 1)->dest;
> +			    if (EDGE_COUNT (other_bb->succs) == 0)
> +			      {
> +				gimple_stmt_iterator gsi
> +				  = gsi_after_labels (other_bb);
> +				if (!gsi_end_p (gsi)
> +				    && gimple_call_builtin_p
> +							(gsi_stmt (gsi),
> +							 BUILT_IN_UNREACHABLE))
> +				  is_unreachable = 1;
> +			      }
> +			  }

Please factor this out into a separate function.  Like
obfuscated_fallthru_edge_p () (better name?).  Best somewhere
in tree-cfg.c.  Also you want to skip debug stmts ... (not sure
if we reliably enough remove them, but I can envision a compare-debug
fail with some DCE disabled at least).

> +		      }
> +		  }
> +		/* Handle
> +		   if (x_7 >= 10 && x_7 < 20)
> +		     __builtin_unreachable ();
> +		   x_8 = ASSERT_EXPR <x_7, ...>;
> +		   if the only uses of x_7 are in the ASSERT_EXPR and
> +		   in the condition.  In that case, we can copy the
> +		   range info from x_8 computed in this pass also
> +		   for x_7.  */
> +		if (is_unreachable)
> +		  {
> +		    bool ok = true;
> +		    FOR_EACH_IMM_USE_FAST (use_p, iter, var)
> +		      if (USE_STMT (use_p) != stmt)
> +			{
> +			  use_stmt = USE_STMT (use_p);
> +			  if (is_gimple_debug (use_stmt))
> +			    continue;
> +			  while (is_gimple_assign (use_stmt)
> +				 && single_imm_use
> +					(gimple_assign_lhs (use_stmt),
> +					 &use2_p, &use_stmt))
> +			    ;
> +			  if (gimple_code (use_stmt) != GIMPLE_COND
> +			      || gimple_bb (use_stmt) != single_pred (bb))
> +			    {
> +			      ok = false;
> +			      break;
> +			    }
> +			}

This also should be split out.

Otherwise ok.

Thanks,
Richard.

> +		    if (ok)
> +		      set_range_info (var, SSA_NAME_RANGE_INFO (lhs)->min,
> +				      SSA_NAME_RANGE_INFO (lhs)->max);
> +		  }
> +	      }
> +
> +	    /* Propagate the RHS into every use of the LHS.  */
> +	    FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
>  	      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
> -		{
> -		  SET_USE (use_p, var);
> -		  gcc_assert (TREE_CODE (var) == SSA_NAME);
> -		}
> +		SET_USE (use_p, var);
>  
>  	    /* And finally, remove the copy, it is not needed.  */
>  	    gsi_remove (&si, true);
>  	    release_defs (stmt);
>  	  }
>  	else
> -	  gsi_next (&si);
> +	  {
> +	    gsi_next (&si);
> +	    is_unreachable = 0;
> +	  }
>        }
>  }
>  
> 
> 	Jakub
diff mbox

Patch

--- gcc/tree-vrp.c.jj	2013-10-24 10:19:21.000000000 +0200
+++ gcc/tree-vrp.c	2013-10-24 14:32:29.065878208 +0200
@@ -6488,12 +6488,16 @@  remove_range_assertions (void)
 {
   basic_block bb;
   gimple_stmt_iterator si;
+  /* 1 if looking at ASSERT_EXPRs immediately at the beginning of
+     a basic block preceeded by GIMPLE_COND branching to it and
+     __builtin_trap, -1 if not yet checked, 0 otherwise.  */
+  int is_unreachable;
 
   /* Note that the BSI iterator bump happens at the bottom of the
      loop and no bump is necessary if we're removing the statement
      referenced by the current BSI.  */
   FOR_EACH_BB (bb)
-    for (si = gsi_start_bb (bb); !gsi_end_p (si);)
+    for (si = gsi_after_labels (bb), is_unreachable = -1; !gsi_end_p (si);)
       {
 	gimple stmt = gsi_stmt (si);
 	gimple use_stmt;
@@ -6501,30 +6505,96 @@  remove_range_assertions (void)
 	if (is_gimple_assign (stmt)
 	    && gimple_assign_rhs_code (stmt) == ASSERT_EXPR)
 	  {
+	    tree lhs = gimple_assign_lhs (stmt);
 	    tree rhs = gimple_assign_rhs1 (stmt);
 	    tree var;
 	    tree cond = fold (ASSERT_EXPR_COND (rhs));
-	    use_operand_p use_p;
+	    use_operand_p use_p, use2_p;
 	    imm_use_iterator iter;
 
 	    gcc_assert (cond != boolean_false_node);
 
-	    /* Propagate the RHS into every use of the LHS.  */
 	    var = ASSERT_EXPR_VAR (rhs);
-	    FOR_EACH_IMM_USE_STMT (use_stmt, iter,
-				   gimple_assign_lhs (stmt))
+	    gcc_assert (TREE_CODE (var) == SSA_NAME);
+
+	    if (!POINTER_TYPE_P (TREE_TYPE (lhs))
+		&& SSA_NAME_RANGE_INFO (lhs))
+	      {
+		if (is_unreachable == -1)
+		  {
+		    is_unreachable = 0;
+		    if (single_pred_p (bb))
+		      {
+			basic_block pred_bb = single_pred (bb);
+			gimple last = last_stmt (pred_bb);
+			if (last && gimple_code (last) == GIMPLE_COND)
+			  {
+			    basic_block other_bb
+			      = EDGE_SUCC (pred_bb, 0)->dest;
+			    if (other_bb == bb)
+			      other_bb = EDGE_SUCC (pred_bb, 1)->dest;
+			    if (EDGE_COUNT (other_bb->succs) == 0)
+			      {
+				gimple_stmt_iterator gsi
+				  = gsi_after_labels (other_bb);
+				if (!gsi_end_p (gsi)
+				    && gimple_call_builtin_p
+							(gsi_stmt (gsi),
+							 BUILT_IN_UNREACHABLE))
+				  is_unreachable = 1;
+			      }
+			  }
+		      }
+		  }
+		/* Handle
+		   if (x_7 >= 10 && x_7 < 20)
+		     __builtin_unreachable ();
+		   x_8 = ASSERT_EXPR <x_7, ...>;
+		   if the only uses of x_7 are in the ASSERT_EXPR and
+		   in the condition.  In that case, we can copy the
+		   range info from x_8 computed in this pass also
+		   for x_7.  */
+		if (is_unreachable)
+		  {
+		    bool ok = true;
+		    FOR_EACH_IMM_USE_FAST (use_p, iter, var)
+		      if (USE_STMT (use_p) != stmt)
+			{
+			  use_stmt = USE_STMT (use_p);
+			  if (is_gimple_debug (use_stmt))
+			    continue;
+			  while (is_gimple_assign (use_stmt)
+				 && single_imm_use
+					(gimple_assign_lhs (use_stmt),
+					 &use2_p, &use_stmt))
+			    ;
+			  if (gimple_code (use_stmt) != GIMPLE_COND
+			      || gimple_bb (use_stmt) != single_pred (bb))
+			    {
+			      ok = false;
+			      break;
+			    }
+			}
+		    if (ok)
+		      set_range_info (var, SSA_NAME_RANGE_INFO (lhs)->min,
+				      SSA_NAME_RANGE_INFO (lhs)->max);
+		  }
+	      }
+
+	    /* Propagate the RHS into every use of the LHS.  */
+	    FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
 	      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
-		{
-		  SET_USE (use_p, var);
-		  gcc_assert (TREE_CODE (var) == SSA_NAME);
-		}
+		SET_USE (use_p, var);
 
 	    /* And finally, remove the copy, it is not needed.  */
 	    gsi_remove (&si, true);
 	    release_defs (stmt);
 	  }
 	else
-	  gsi_next (&si);
+	  {
+	    gsi_next (&si);
+	    is_unreachable = 0;
+	  }
       }
 }