This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Handle __builtin_unreachable () using assertions in VRP


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]