Message ID | alpine.DEB.2.02.1208221346210.21614@stedding.saclay.inria.fr |
---|---|
State | New |
Headers | show |
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.
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 {