Re: [PATCH] Range info handling fixes (PR tree-optimization/59417)

[Richard Biener <rguenther at suse dot de>]

On Wed, 11 Dec 2013, Jakub Jelinek wrote:

> Hi!
> 
> As discussed in the PR, range info as well as alignment info for pointers
> can be position dependent, can be e.g. computed from VRP ASSERT_EXPRs,
> so we can't blindly propagate it to SSA_NAMEs that the SSA_NAME with range
> info has been set to.  The following patch limits that to SSA_NAMEs defined
> within the same bb, in that case it should be safe.
> 
> Also, the patch makes determine_value_range more robust, insert of asserting
> it will just punt and use solely range info of var, rather than also
> considering ranges of PHI results.  If VR_UNDEFINED comes into play, range
> info can be weird (of course on undefined behavior only code, but we
> shouldn't ICE on it).
> 
> Either of these changes fixes the ICE on it's own.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2013-12-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/59417
> 	* tree-ssa-copy.c (fini_copy_prop): If copy_of[i].value is defined
> 	in a different bb rhan var, only duplicate points-to info and
> 	not alignment info and don't duplicate range info.
> 	* tree-ssa-loop-niter.c (determine_value_range): Instead of
> 	assertion failure handle inconsistencies in range info by only
> 	using var's range info and not PHI result range infos.
> 
> 	* gcc.c-torture/compile/pr59417.c: New test.
> 
> --- gcc/tree-ssa-copy.c.jj	2013-12-10 08:52:13.000000000 +0100
> +++ gcc/tree-ssa-copy.c	2013-12-10 17:55:16.819293842 +0100
> @@ -567,14 +567,28 @@ fini_copy_prop (void)
>        if (copy_of[i].value != var
>  	  && TREE_CODE (copy_of[i].value) == SSA_NAME)
>  	{
> +	  basic_block copy_of_bb
> +	    = gimple_bb (SSA_NAME_DEF_STMT (copy_of[i].value));
> +	  basic_block var_bb = gimple_bb (SSA_NAME_DEF_STMT (var));
>  	  if (POINTER_TYPE_P (TREE_TYPE (var))
>  	      && SSA_NAME_PTR_INFO (var)
>  	      && !SSA_NAME_PTR_INFO (copy_of[i].value))
> -	    duplicate_ssa_name_ptr_info (copy_of[i].value,
> -					 SSA_NAME_PTR_INFO (var));
> +	    {
> +	      duplicate_ssa_name_ptr_info (copy_of[i].value,
> +					   SSA_NAME_PTR_INFO (var));
> +	      /* Points-to information is cfg insensitive,
> +		 but alignment info might be cfg sensitive, if it
> +		 e.g. is derived from VRP derived non-zero bits.
> +		 So, do not copy alignment info if the two SSA_NAMEs
> +		 aren't defined in the same basic block.  */
> +	      if (var_bb != copy_of_bb)
> +		mark_ptr_info_alignment_unknown
> +				(SSA_NAME_PTR_INFO (copy_of[i].value));
> +	    }
>  	  else if (!POINTER_TYPE_P (TREE_TYPE (var))
>  		   && SSA_NAME_RANGE_INFO (var)
> -		   && !SSA_NAME_RANGE_INFO (copy_of[i].value))
> +		   && !SSA_NAME_RANGE_INFO (copy_of[i].value)
> +		   && var_bb == copy_of_bb)
>  	    duplicate_ssa_name_range_info (copy_of[i].value,
>  					   SSA_NAME_RANGE_TYPE (var),
>  					   SSA_NAME_RANGE_INFO (var));
> --- gcc/tree-ssa-loop-niter.c.jj	2013-12-02 23:34:02.000000000 +0100
> +++ gcc/tree-ssa-loop-niter.c	2013-12-10 17:59:07.744105878 +0100
> @@ -173,7 +173,15 @@ determine_value_range (struct loop *loop
>  		{
>  		  minv = minv.max (minc, TYPE_UNSIGNED (type));
>  		  maxv = maxv.min (maxc, TYPE_UNSIGNED (type));
> -		  gcc_assert (minv.cmp (maxv, TYPE_UNSIGNED (type)) <= 0);
> +		  /* If the PHI result range are inconsistent with
> +		     the VAR range, give up on looking at the PHI
> +		     results.  This can happen if VR_UNDEFINED is
> +		     involved.  */
> +		  if (minv.cmp (maxv, TYPE_UNSIGNED (type)) > 0)
> +		    {
> +		      rtype = get_range_info (var, &minv, &maxv);
> +		      break;
> +		    }
>  		}
>  	    }
>  	}
> --- gcc/testsuite/gcc.c-torture/compile/pr59417.c.jj	2013-12-10 17:59:29.235996171 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr59417.c	2013-12-09 16:26:53.000000000 +0100
> @@ -0,0 +1,39 @@
> +/* PR tree-optimization/59417 */
> +
> +int a, b, d;
> +short c;
> +
> +void
> +f (void)
> +{
> +  if (b)
> +    {
> +      int *e;
> +
> +      if (d)
> +	{
> +	  for (; b; a++)
> +	  lbl1:
> +	    d = 0;
> +
> +	  for (; d <= 1; d++)
> +	    {
> +	      int **q = &e;
> +	      for (**q = 0; **q <= 0; **q++)
> +		d = 0;
> +	    }
> +	}
> +    }
> +
> +  else
> +    {
> +      int t;
> +      for (c = 0; c < 77; c++)
> +	for (c = 0; c < 46; c++);
> +      for (; t <= 0; t++)
> +      lbl2:
> +	;
> +      goto lbl1;
> +    }
> +  goto lbl2;
> +}
> 
> 	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