Re: [PATCH] Fix make_range_step with -fwrapv (PR tree-optimization/55236)

2012-11-26 Thread Richard Biener
On Thu, Nov 8, 2012 at 9:04 PM, Jakub Jelinek  wrote:
> Hi!
>
> With -fwrapv the range test optimization miscompiles the following testcase
> (both inter-bb version in 4.8+ in first function and the pure fold-const.c
> one since 3.4).  The problem is that with -fwrapv and signed type
> -x in [-,b] doesn't imply x in [-b,-] and -x in [a,-] doesn't imply
> x in [-,-a], as -INT_MIN is INT_MIN and thus if INT_MIN was in the range,
> after negation it needs to be in the range too (and similarly if it wasn't,
> it can't be there).  Turns out that if we make sure both low and high
> are non-NULL (i.e. replace - with INT_MIN resp. INT_MAX), normalize:
> will do the right thing that it does also for unsigned operation
> - if it detects the high bound is lower than low bound, it adjusts the
> range by inverting it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (what
> about 4.7 where bar is miscompiled too)?
>
> Perhaps for PLUS_EXPR/MINUS_EXPR and signed -fwrapv type we could do the
> same instead of return NULL_TREE; that we do right now (added by Kazu
> for PR23518).

Ok.

Thanks,
Richard.

> 2012-11-08  Jakub Jelinek  
>
> PR tree-optimization/55236
> * fold-const.c (make_range_step) : For -fwrapv
> and signed ARG0_TYPE, force low and high to be non-NULL.
>
> * gcc.dg/pr55236.c: New test.
>
> --- gcc/fold-const.c.jj 2012-11-07 11:24:34.0 +0100
> +++ gcc/fold-const.c2012-11-08 16:54:38.978339040 +0100
> @@ -3880,6 +3880,17 @@ make_range_step (location_t loc, enum tr
>return arg0;
>
>  case NEGATE_EXPR:
> +  /* If flag_wrapv and ARG0_TYPE is signed, make sure
> +low and high are non-NULL, then normalize will DTRT.  */
> +  if (!TYPE_UNSIGNED (arg0_type)
> + && !TYPE_OVERFLOW_UNDEFINED (arg0_type))
> +   {
> + if (low == NULL_TREE)
> +   low = TYPE_MIN_VALUE (arg0_type);
> + if (high == NULL_TREE)
> +   high = TYPE_MAX_VALUE (arg0_type);
> +   }
> +
>/* (-x) IN [a,b] -> x in [-b, -a]  */
>n_low = range_binop (MINUS_EXPR, exp_type,
>build_int_cst (exp_type, 0),
> --- gcc/testsuite/gcc.dg/pr55236.c.jj   2012-11-08 16:40:49.171781137 +0100
> +++ gcc/testsuite/gcc.dg/pr55236.c  2012-11-08 16:41:20.044578919 +0100
> @@ -0,0 +1,31 @@
> +/* PR tree-optimization/55236 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fwrapv" } */
> +
> +extern void abort ();
> +
> +__attribute__((noinline, noclone)) void
> +foo (int i)
> +{
> +  if (i > 0)
> +abort ();
> +  i = -i;
> +  if (i < 0)
> +return;
> +  abort ();
> +}
> +
> +__attribute__((noinline, noclone)) void
> +bar (int i)
> +{
> +  if (i > 0 || (-i) >= 0)
> +abort ();
> +}
> +
> +int
> +main ()
> +{
> +  foo (-__INT_MAX__ - 1);
> +  bar (-__INT_MAX__ - 1);
> +  return 0;
> +}
>
> Jakub


[PATCH] Fix make_range_step with -fwrapv (PR tree-optimization/55236)

2012-11-08 Thread Jakub Jelinek
Hi!

With -fwrapv the range test optimization miscompiles the following testcase
(both inter-bb version in 4.8+ in first function and the pure fold-const.c
one since 3.4).  The problem is that with -fwrapv and signed type
-x in [-,b] doesn't imply x in [-b,-] and -x in [a,-] doesn't imply
x in [-,-a], as -INT_MIN is INT_MIN and thus if INT_MIN was in the range,
after negation it needs to be in the range too (and similarly if it wasn't,
it can't be there).  Turns out that if we make sure both low and high
are non-NULL (i.e. replace - with INT_MIN resp. INT_MAX), normalize:
will do the right thing that it does also for unsigned operation
- if it detects the high bound is lower than low bound, it adjusts the
range by inverting it.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk (what
about 4.7 where bar is miscompiled too)?

Perhaps for PLUS_EXPR/MINUS_EXPR and signed -fwrapv type we could do the
same instead of return NULL_TREE; that we do right now (added by Kazu
for PR23518).

2012-11-08  Jakub Jelinek  

PR tree-optimization/55236
* fold-const.c (make_range_step) : For -fwrapv
and signed ARG0_TYPE, force low and high to be non-NULL.

* gcc.dg/pr55236.c: New test.

--- gcc/fold-const.c.jj 2012-11-07 11:24:34.0 +0100
+++ gcc/fold-const.c2012-11-08 16:54:38.978339040 +0100
@@ -3880,6 +3880,17 @@ make_range_step (location_t loc, enum tr
   return arg0;
 
 case NEGATE_EXPR:
+  /* If flag_wrapv and ARG0_TYPE is signed, make sure
+low and high are non-NULL, then normalize will DTRT.  */
+  if (!TYPE_UNSIGNED (arg0_type)
+ && !TYPE_OVERFLOW_UNDEFINED (arg0_type))
+   {
+ if (low == NULL_TREE)
+   low = TYPE_MIN_VALUE (arg0_type);
+ if (high == NULL_TREE)
+   high = TYPE_MAX_VALUE (arg0_type);
+   }
+
   /* (-x) IN [a,b] -> x in [-b, -a]  */
   n_low = range_binop (MINUS_EXPR, exp_type,
   build_int_cst (exp_type, 0),
--- gcc/testsuite/gcc.dg/pr55236.c.jj   2012-11-08 16:40:49.171781137 +0100
+++ gcc/testsuite/gcc.dg/pr55236.c  2012-11-08 16:41:20.044578919 +0100
@@ -0,0 +1,31 @@
+/* PR tree-optimization/55236 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fwrapv" } */
+
+extern void abort ();
+
+__attribute__((noinline, noclone)) void
+foo (int i)
+{
+  if (i > 0)
+abort ();
+  i = -i;
+  if (i < 0)
+return;
+  abort ();
+}
+
+__attribute__((noinline, noclone)) void
+bar (int i)
+{
+  if (i > 0 || (-i) >= 0)
+abort ();
+}
+
+int
+main ()
+{
+  foo (-__INT_MAX__ - 1);
+  bar (-__INT_MAX__ - 1);
+  return 0;
+}

Jakub