diff mbox

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

Message ID 20121108200413.GG1859@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 8, 2012, 8:04 p.m. UTC
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

Comments

Richard Biener Nov. 26, 2012, 2:45 p.m. UTC | #1
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
diff mbox

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