[PATCH] Fix PR31115, wrong code with VRP and shifts

[Richard Guenther]

On 3/11/07, Richard Guenther <rguenther@suse.de> wrote:
>
> This fixes PR31115, some oversights with my last patch introducing
> support for RSHIFT_EXPR in VRP and a bad interaction with the overflow
> handling changes.  Basically the problem is that -[-INF, -1] is now
> [1, +INF(OVL)] and we now "overflow" right shifting of that even if
> right shifting (by a positive shift) never overflows.  This does I
> believe cause some optimiation regressions :/
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to mainline.

Argh.  I should read what the svn commit prints ;)  Now Ian was first, so
I re-merged and committed.  I left in the now probably unused case of
overflowing RSHIFT_EXPR in vrp_int_const_binop.  The rest still applies.

Richard.

> Richard.
>
> 2007-03-11  Richard Guenther  <rguenther@suse.de>
>
>         PR tree-optimization/31115
>         * tree-vrp.c (extract_range_from_binary_expr): Make sure
>         the shift count is positive and non-anti-range for RSHIFT_EXPR.
>         A shift count of zero is not special as with *_DIV_EXPR.
>         (vrp_int_const_binop): Handle RSHIFT_EXPR for determining overflow
>         direction.
>
>         * gcc.dg/torture/pr31115.c: New testcase.
>
> Index: tree-vrp.c
> ===================================================================
> *** tree-vrp.c  (revision 122805)
> --- tree-vrp.c  (working copy)
> *************** vrp_int_const_binop (enum tree_code code
> *** 1554,1559 ****
> --- 1554,1565 ----
>               && (sgn1 >= 0
>                   ? !is_positive_overflow_infinity (val2)
>                   : is_negative_overflow_infinity (val2)))
> +         /* We only get in here with positive shift count, so the
> +            overflow direction is the same as the sign of val1.
> +            Actually rshift does not overflow at all, but we only
> +            handle the case of shifting overflowed -INF and +INF.  */
> +         || (code == RSHIFT_EXPR
> +             && sgn1 >= 0)
>           /* For division, the only case is -INF / -1 = +INF.  */
>           || code == TRUNC_DIV_EXPR
>           || code == FLOOR_DIV_EXPR
> *************** extract_range_from_binary_expr (value_ra
> *** 1787,1792 ****
> --- 1793,1809 ----
>           return;
>         }
>
> +       /* If we have a RSHIFT_EXPR with a possibly negative shift
> +        count or an anti-range shift count drop to VR_VARYING.
> +        We currently cannot handle the overflow cases correctly.  */
> +       if (code == RSHIFT_EXPR
> +         && (vr1.type == VR_ANTI_RANGE
> +             || !vrp_expr_computes_nonnegative (op1, &sop)))
> +       {
> +         set_value_range_to_varying (vr);
> +         return;
> +       }
> +
>         /* Multiplications and divisions are a bit tricky to handle,
>          depending on the mix of signs we have in the two ranges, we
>          need to operate on different values to get the minimum and
> *************** extract_range_from_binary_expr (value_ra
> *** 1801,1807 ****
>          the new range.  */
>
>         /* Divisions by zero result in a VARYING value.  */
> !       if (code != MULT_EXPR
>           && (vr0.type == VR_ANTI_RANGE || range_includes_zero_p (&vr1)))
>         {
>           set_value_range_to_varying (vr);
> --- 1818,1825 ----
>          the new range.  */
>
>         /* Divisions by zero result in a VARYING value.  */
> !       if ((code != MULT_EXPR
> !          && code != RSHIFT_EXPR)
>           && (vr0.type == VR_ANTI_RANGE || range_includes_zero_p (&vr1)))
>         {
>           set_value_range_to_varying (vr);
> Index: testsuite/gcc.dg/torture/pr31115.c
> ===================================================================
> *** testsuite/gcc.dg/torture/pr31115.c  (revision 0)
> --- testsuite/gcc.dg/torture/pr31115.c  (revision 0)
> ***************
> *** 0 ****
> --- 1,22 ----
> + /* { dg-do run } */
> +
> + extern void exit(int);
> + extern void abort();
> + void foo (int e1)
> + {
> +   if (e1 < 0)
> +     {
> +       e1 = -e1;
> +       if (e1 >>= 4)
> +         {
> +           if (e1 >= 1 << 5)
> +             exit(0);
> +         }
> +     }
> + }
> +
> + int main()
> + {
> +   foo(-(1<<9));
> +   abort();
> + }
>