Patchwork Fix make_range_step with -fwrapv (PR tree-optimization/55236)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 8, 2012, 8:04 p.m.
Message ID <20121108200413.GG1859@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/197875/
State New
Headers show

Comments

Jakub Jelinek - Nov. 8, 2012, 8:04 p.m.
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  <jakub@redhat.com>

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

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


	Jakub
Richard Guenther - Nov. 26, 2012, 2:45 p.m.
On Thu, Nov 8, 2012 at 9:04 PM, Jakub Jelinek <jakub@redhat.com> 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  <jakub@redhat.com>
>
>         PR tree-optimization/55236
>         * fold-const.c (make_range_step) <case NEGATE_EXPR>: 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.000000000 +0100
> +++ gcc/fold-const.c    2012-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

--- gcc/fold-const.c.jj	2012-11-07 11:24:34.000000000 +0100
+++ gcc/fold-const.c	2012-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;
+}