Fix double_int overflow in VRP PLUS_EXPR

Hello,

when I adapted VRP PLUS_EXPR handling for __int128, I missed one place 
where double_int can overflow. Note that I have no idea if that helps for 
bug 54317, but that's where I noticed the issue.

2012-08-21  Marc Glisse  <marc.glisse@inria.fr>

 	PR tree-optimization/54317

gcc/
 	* tree-vrp.c (extract_range_from_binary_expr_1): Test for
 	double_int overflow.
 	Remove dead tests.

gcc/testsuite/
 	* gcc.dg/tree-ssa/vrp79.c: New testcase.

On Wed, Aug 22, 2012 at 1:55 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> when I adapted VRP PLUS_EXPR handling for __int128, I missed one place where
> double_int can overflow. Note that I have no idea if that helps for bug
> 54317, but that's where I noticed the issue.

Ok.

Thanks,
Richard.

> 2012-08-21  Marc Glisse  <marc.glisse@inria.fr>
>
>         PR tree-optimization/54317
>
> gcc/
>         * tree-vrp.c (extract_range_from_binary_expr_1): Test for
>         double_int overflow.
>         Remove dead tests.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/vrp79.c: New testcase.
>
> --
> Marc Glisse
> Index: testsuite/gcc.dg/tree-ssa/vrp79.c
> ===================================================================
> --- testsuite/gcc.dg/tree-ssa/vrp79.c   (revision 0)
> +++ testsuite/gcc.dg/tree-ssa/vrp79.c   (revision 0)
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +#ifdef __SIZEOF_INT128__
> +typedef unsigned __int128 NT;
> +#else
> +typedef unsigned long long NT;
> +#endif
> +
> +extern void do_not_go_away ();
> +
> +void f (NT x, NT y)
> +{
> +  NT n = 1;
> +  n <<= (8 * sizeof (NT) - 1);
> +  if (x > n) return;
> +  if (y > n) return;
> +  NT z = x + y;
> +  if (z == 42) do_not_go_away ();
> +}
> +
> +/* { dg-final { scan-tree-dump "do_not_go_away" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
>
> Property changes on: testsuite/gcc.dg/tree-ssa/vrp79.c
> ___________________________________________________________________
> Added: svn:keywords
>    + Author Date Id Revision URL
> Added: svn:eol-style
>    + native
>
> Index: tree-vrp.c
> ===================================================================
> --- tree-vrp.c  (revision 190590)
> +++ tree-vrp.c  (working copy)
> @@ -2467,32 +2467,35 @@ extract_range_from_binary_expr_1 (value_
>                        && max_ovf == 1)
>                 {
>                   /* Underflow and overflow, drop to VR_VARYING.  */
>                   set_value_range_to_varying (vr);
>                   return;
>                 }
>               else
>                 {
>                   /* Min underflow or max overflow.  The range kind
>                      changes to VR_ANTI_RANGE.  */
> +                 bool covers = false;
>                   double_int tem = tmin;
>                   gcc_assert ((min_ovf == -1 && max_ovf == 0)
>                               || (max_ovf == 1 && min_ovf == 0));
>                   type = VR_ANTI_RANGE;
>                   tmin = double_int_add (tmax, double_int_one);
> +                 if (double_int_cmp (tmin, tmax, uns) < 0)
> +                   covers = true;
>                   tmax = double_int_add (tem, double_int_minus_one);
> +                 if (double_int_cmp (tmax, tem, uns) > 0)
> +                   covers = true;
>                   /* If the anti-range would cover nothing, drop to varying.
>                      Likewise if the anti-range bounds are outside of the
>                      types values.  */
> -                 if (double_int_cmp (tmin, tmax, uns) > 0
> -                     || double_int_cmp (tmin, type_min, uns) < 0
> -                     || double_int_cmp (tmax, type_max, uns) > 0)
> +                 if (covers || double_int_cmp (tmin, tmax, uns) > 0)
>                     {
>                       set_value_range_to_varying (vr);
>                       return;
>                     }
>                   min = double_int_to_tree (expr_type, tmin);
>                   max = double_int_to_tree (expr_type, tmax);
>                 }
>             }
>           else
>             {
>

On Wed, Aug 22, 2012 at 02:19:19PM +0200, Richard Guenther wrote:
> > 2012-08-21  Marc Glisse  <marc.glisse@inria.fr>
> >
> >         PR tree-optimization/54317
> >
> > gcc/
> >         * tree-vrp.c (extract_range_from_binary_expr_1): Test for
> >         double_int overflow.
> >         Remove dead tests.
> >
> > gcc/testsuite/
> >         * gcc.dg/tree-ssa/vrp79.c: New testcase.
> >
> > --
> > +  n <<= (8 * sizeof (NT) - 1);

Better use __CHAR_BIT__ instead of 8 here...

	Jakub

On Wed, 22 Aug 2012, Jakub Jelinek wrote:

>>> 2012-08-21  Marc Glisse  <marc.glisse@inria.fr>
>>> +  n <<= (8 * sizeof (NT) - 1);
>
> Better use __CHAR_BIT__ instead of 8 here...

Ok, I committed the __CHAR_BIT__ version (I just compiled that one file 
manually with old and new compilers, I didn't restart the testsuite, I 
hope the manual test is enough to detect any typo that could break 
things).

On Wed, Aug 22, 2012 at 02:34:01PM +0200, Marc Glisse wrote:
> On Wed, 22 Aug 2012, Jakub Jelinek wrote:
> 
> >>>2012-08-21  Marc Glisse  <marc.glisse@inria.fr>
> >>>+  n <<= (8 * sizeof (NT) - 1);
> >
> >Better use __CHAR_BIT__ instead of 8 here...
> 
> Ok, I committed the __CHAR_BIT__ version (I just compiled that one
> file manually with old and new compilers, I didn't restart the
> testsuite, I hope the manual test is enough to detect any typo that
> could break things).

Probably.  You could have used:
make check-gcc RUNTESTFLAGS=tree-ssa.exp=vrp79.c
to test just that single testcase (with both old and new compilers).

	Jakub

On Wed, 22 Aug 2012, Jakub Jelinek wrote:

> On Wed, Aug 22, 2012 at 02:34:01PM +0200, Marc Glisse wrote:
>> Ok, I committed the __CHAR_BIT__ version (I just compiled that one
>> file manually with old and new compilers, I didn't restart the
>> testsuite, I hope the manual test is enough to detect any typo that
>> could break things).
>
> Probably.  You could have used:
> make check-gcc RUNTESTFLAGS=tree-ssa.exp=vrp79.c
> to test just that single testcase (with both old and new compilers).

Thanks, I keep forgetting the details of that syntax.

I just ran it for peace of mind, and it was happy.