diff mbox

Range info handling fixes (PR tree-optimization/59417)

Message ID 20131211083315.GB892@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 11, 2013, 8:33 a.m. UTC
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?

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.


	Jakub

Comments

Richard Biener Dec. 11, 2013, 9:02 a.m. UTC | #1
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
> 
>
diff mbox

Patch

--- 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;
+}