Fix PR tree-optimization/80426

Submitted by Eric Botcazou on April 19, 2017, 6:20 a.m.

Details

Message ID 7983915.vF9qLkl6hX@polaris
State New
Headers show

Commit Message

Eric Botcazou April 19, 2017, 6:20 a.m.
Hi,

this is a regression on the mainline, but the underlying issue is present on 
the branches too, and similar to PR tree-optimization/79666, but this time for 
the invariant part instead of the symbolic one.

In extract_range_from_binary_expr_1, when the invariant part of the symbolic 
computation overflows, we drop to varying:

	  /* If we have overflow for the constant part and the resulting
	     range will be symbolic, drop to VR_VARYING.  */
	  if ((min_ovf && sym_min_op0 != sym_min_op1)
	      || (max_ovf && sym_max_op0 != sym_max_op1))
	    {
	      set_value_range_to_varying (vr);
	      return;
	    }

But we fail to compute the overflow when the operation is MINUS_EXPR and the 
first operand is 0, i.e. we can silently negate INT_MIN and fail to bail out.

Fixed by computing overflow in this case too.  Tested on x86_64-suse-linux, OK 
for mainline and 6 branch?


2017-04-19  Eric Botcazou  <ebotcazou@adacore.com>

	PR tree-optimization/80426
	* tree-vrp.c (extract_range_from_binary_expr_1): For an additive
	operation on symbolic operands, also compute the overflow for the
	invariant part when the operation degenerates into a negation.


2017-04-19  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/execute/20170419-1.c: New test.

Comments

Jakub Jelinek April 19, 2017, 9:29 a.m.
On Wed, Apr 19, 2017 at 08:20:40AM +0200, Eric Botcazou wrote:
> --- tree-vrp.c	(revision 246960)
> +++ tree-vrp.c	(working copy)
> @@ -2461,7 +2461,19 @@ extract_range_from_binary_expr_1 (value_
>  	  else if (min_op0)
>  	    wmin = min_op0;
>  	  else if (min_op1)
> -	    wmin = minus_p ? wi::neg (min_op1) : min_op1;
> +	    {
> +	      if (minus_p)
> +		{
> +		  wmin = wi::neg (min_op1);
> +
> +		  /* Check for overflow.  */
> +		  if (wi::cmp (0, min_op1, sgn)
> +		      != wi::cmp (wmin, 0, sgn))

I know this attempts to be a copy of what is used elsewhere, but
at least there it is a result of wi::sub etc.
Wouldn't it be simpler to
  if (sgn == SIGNED && wi::neg_p (min_op1) && wi::neg_p (wmin))
    min_ovf = 1;
  else if (sgn == UNSIGNED && wi::ne_p (min_op1, 0))
    min_ovf = -1;

I mean, for SIGNED if min_op1 is 0, then wmin is 0 to and we want
min_ovf = 0;
If it is positive, wmin will be surely negative and again we want
min_ovf = 0.  Only if it is negative and its negation is negative
too we want min_ovf = 1 (i.e. wi::cmps (0, most_negative) result).
For UNSIGNED, if min_op1 is 0, again all 3 wi::cmp will yield
0 and min_ovf = 0.  If it is non-zero, it is > 0, therefore it
the first wi::cmp will return -1, the second wi::cmp returns
1 and the third one -1.

Is that what we want (e.g. the UNSIGNED case to overflow pretty much always
except for 0 which should be optimized away anyway)?

Or, shouldn't we just set if (!min_op0 && min_op1 && minus_p) min_op0 =
build_int_cst (expr_type, 0); before the if (min_op0 && min_op1) case
and don't duplicate that?

	Jakub

Patch hide | download patch | download mbox

Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 246960)
+++ tree-vrp.c	(working copy)
@@ -2461,7 +2461,19 @@  extract_range_from_binary_expr_1 (value_
 	  else if (min_op0)
 	    wmin = min_op0;
 	  else if (min_op1)
-	    wmin = minus_p ? wi::neg (min_op1) : min_op1;
+	    {
+	      if (minus_p)
+		{
+		  wmin = wi::neg (min_op1);
+
+		  /* Check for overflow.  */
+		  if (wi::cmp (0, min_op1, sgn)
+		      != wi::cmp (wmin, 0, sgn))
+		    min_ovf = wi::cmp (0, min_op1, sgn);
+		}
+	      else
+		wmin = min_op1;
+	    }
 	  else
 	    wmin = wi::shwi (0, prec);
 
@@ -2489,7 +2501,19 @@  extract_range_from_binary_expr_1 (value_
 	  else if (max_op0)
 	    wmax = max_op0;
 	  else if (max_op1)
-	    wmax = minus_p ? wi::neg (max_op1) : max_op1;
+	    {
+	      if (minus_p)
+		{
+		  wmax = wi::neg (max_op1);
+
+		  /* Check for overflow.  */
+		  if (wi::cmp (0, max_op1, sgn)
+		      != wi::cmp (wmax, 0, sgn))
+		    max_ovf = wi::cmp (0, max_op1, sgn);
+		}
+	      else
+		wmax = max_op1;
+	    }
 	  else
 	    wmax = wi::shwi (0, prec);