[PATCH] Fix __builtin_unreachable related regression (PR middle-end/60482)

Richard Biener rguenther@suse.de
Wed Mar 12 08:53:00 GMT 2014


On Tue, 11 Mar 2014, Jakub Jelinek wrote:

> Hi!
> 
> As described in the PR, the r208165 change regressed following test.
> The problem is that VRP inserts a useless ASSERT_EXPR right before
> __builtin_unreachable () (obviously, the uses of the ASSERT_EXPR
> lhs aren't and can't be used by anything), which then prevents
> assert_unreachable_fallthru_edge_p from detecting it properly
> (but, even ignoring ASSERT_EXPRs there still would fail, because
> the ASSERT_EXPR adds another user of the SSA_NAME we check imm uses for).
> 
> Perhaps FOUND_IN_SUBGRAPH (4.3 and earlier era) would be always true
> here, but live_on_edge provably isn't always true, so it makes sense to test
> it, something that isn't live on the edge is useless.
> 
> The tree-cfg.c change is just small improvement discovered when looking into
> it, clobber stmts before __builtin_unreachable can be certainly ignored,
> they don't do anything.
> 
> The patch regresses ssa-ifcombine-10.c testcase, I'll post a fix for that
> momentarily.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2014-03-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/60482
> 	* tree-vrp.c (register_edge_assert_for_1): Don't add assert
> 	if there are multiple uses, but op doesn't live on E edge.
> 	* tree-cfg.c (assert_unreachable_fallthru_edge_p): Also ignore
> 	clobber stmts before __builtin_unreachable.
> 
> 	* gcc.dg/vect/pr60482.c: New test.
> 
> --- gcc/tree-vrp.c.jj	2014-01-25 00:11:37.000000000 +0100
> +++ gcc/tree-vrp.c	2014-03-10 14:59:03.748267354 +0100
> @@ -5423,12 +5423,9 @@ register_edge_assert_for_1 (tree op, enu
>      return false;
>  
>    /* We know that OP will have a zero or nonzero value.  If OP is used
> -     more than once go ahead and register an assert for OP.
> -
> -     The FOUND_IN_SUBGRAPH support is not helpful in this situation as
> -     it will always be set for OP (because OP is used in a COND_EXPR in
> -     the subgraph).  */
> -  if (!has_single_use (op))
> +     more than once go ahead and register an assert for OP.  */
> +  if (live_on_edge (e, op)
> +      && !has_single_use (op))
>      {
>        val = build_int_cst (TREE_TYPE (op), 0);
>        register_new_assert_for (op, op, code, val, NULL, e, bsi);
> --- gcc/tree-cfg.c.jj	2014-02-20 21:38:42.000000000 +0100
> +++ gcc/tree-cfg.c	2014-03-10 14:59:52.058957446 +0100
> @@ -410,9 +410,9 @@ assert_unreachable_fallthru_edge_p (edge
>  	  if (gsi_end_p (gsi))
>  	    return false;
>  	  stmt = gsi_stmt (gsi);
> -	  if (is_gimple_debug (stmt))
> +	  while (is_gimple_debug (stmt) || gimple_clobber_p (stmt))
>  	    {
> -	      gsi_next_nondebug (&gsi);
> +	      gsi_next (&gsi);
>  	      if (gsi_end_p (gsi))
>  		return false;
>  	      stmt = gsi_stmt (gsi);
> --- gcc/testsuite/gcc.dg/vect/pr60482.c.jj	2014-03-10 15:08:16.700085976 +0100
> +++ gcc/testsuite/gcc.dg/vect/pr60482.c	2014-03-10 15:15:09.609738455 +0100
> @@ -0,0 +1,20 @@
> +/* PR middle-end/60482 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-Ofast" } */
> +/* { dg-require-effective-target vect_int } */
> +
> +double
> +foo (double *x, int n)
> +{
> +  double p = 0.0;
> +  int i;
> +  x = __builtin_assume_aligned (x, 128);
> +  if (n % 128)
> +    __builtin_unreachable ();
> +  for (i = 0; i < n; i++)
> +    p += x[i];
> +  return p;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "epilog loop required" "vect" } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer



More information about the Gcc-patches mailing list