| Message ID | 75068b7a-f6f0-7a59-0fb7-d78fafa92d25@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Check for overflow in tree-switch-conversion (PR middle-end/90478). | expand |
On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote: > > Hi. > > The patch is about handling of overflow in jump_table_cluster::can_be_handled. > I calculate 100 * range and compare it to range. If it's smaller, then > overflow happens. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? Looks good, but can the RHS overflow as well? I suppose changing the compute/compare to uint64_t would fix the overflow but downstream the issue would re-appear? Richard. > Thanks, > Martin > > gcc/ChangeLog: > > 2019-05-15 Martin Liska <mliska@suse.cz> > > PR middle-end/90478 > * tree-switch-conversion.c (jump_table_cluster::can_be_handled): > Check for overflow. > > gcc/testsuite/ChangeLog: > > 2019-05-15 Martin Liska <mliska@suse.cz> > > PR middle-end/90478 > * gcc.dg/tree-ssa/pr90478-2.c: New test. > * gcc.dg/tree-ssa/pr90478.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++ > gcc/testsuite/gcc.dg/tree-ssa/pr90478.c | 18 ++++++++++++++++++ > gcc/tree-switch-conversion.c | 6 +++++- > 3 files changed, 40 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c > >
On 5/15/19 1:21 PM, Richard Biener wrote: > On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote: >> >> Hi. >> >> The patch is about handling of overflow in jump_table_cluster::can_be_handled. >> I calculate 100 * range and compare it to range. If it's smaller, then >> overflow happens. >> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. >> >> Ready to be installed? > > Looks good, but can the RHS overflow as well? That should be fine as maximal value of the PARAM is 1<32. And if I'm correct HOST_WIDE_INT should have 64-bits? Martin > I suppose changing > the compute/compare to uint64_t would fix the overflow but downstream > the issue would re-appear? > > Richard. > >> Thanks, >> Martin >> >> gcc/ChangeLog: >> >> 2019-05-15 Martin Liska <mliska@suse.cz> >> >> PR middle-end/90478 >> * tree-switch-conversion.c (jump_table_cluster::can_be_handled): >> Check for overflow. >> >> gcc/testsuite/ChangeLog: >> >> 2019-05-15 Martin Liska <mliska@suse.cz> >> >> PR middle-end/90478 >> * gcc.dg/tree-ssa/pr90478-2.c: New test. >> * gcc.dg/tree-ssa/pr90478.c: New test. >> --- >> gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++ >> gcc/testsuite/gcc.dg/tree-ssa/pr90478.c | 18 ++++++++++++++++++ >> gcc/tree-switch-conversion.c | 6 +++++- >> 3 files changed, 40 insertions(+), 1 deletion(-) >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c >> >>
On Wed, May 15, 2019 at 1:30 PM Martin Liška <mliska@suse.cz> wrote: > > On 5/15/19 1:21 PM, Richard Biener wrote: > > On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote: > >> > >> Hi. > >> > >> The patch is about handling of overflow in jump_table_cluster::can_be_handled. > >> I calculate 100 * range and compare it to range. If it's smaller, then > >> overflow happens. > >> > >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > >> > >> Ready to be installed? > > > > Looks good, but can the RHS overflow as well? > > That should be fine as maximal value of the PARAM is 1<32. And if I'm correct > HOST_WIDE_INT should have 64-bits? Yes. > Martin > > > I suppose changing > > the compute/compare to uint64_t would fix the overflow but downstream > > the issue would re-appear? > > > > Richard. > > > >> Thanks, > >> Martin > >> > >> gcc/ChangeLog: > >> > >> 2019-05-15 Martin Liska <mliska@suse.cz> > >> > >> PR middle-end/90478 > >> * tree-switch-conversion.c (jump_table_cluster::can_be_handled): > >> Check for overflow. > >> > >> gcc/testsuite/ChangeLog: > >> > >> 2019-05-15 Martin Liska <mliska@suse.cz> > >> > >> PR middle-end/90478 > >> * gcc.dg/tree-ssa/pr90478-2.c: New test. > >> * gcc.dg/tree-ssa/pr90478.c: New test. > >> --- > >> gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++ > >> gcc/testsuite/gcc.dg/tree-ssa/pr90478.c | 18 ++++++++++++++++++ > >> gcc/tree-switch-conversion.c | 6 +++++- > >> 3 files changed, 40 insertions(+), 1 deletion(-) > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c > >> > >> >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c new file mode 100644 index 00000000000..f0fc103a888 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-Os --param jump-table-max-growth-ratio-for-size=2147483647" } */ + +long +foo (long x, long y) +{ + x = x & y; + switch (y) + { + case 63L: x >>= 0; break; + case 4032L: x >>= 6; break; + case 258048L: x >>= 12; break; + case 16515072L: x >>= 18; break; + default: __builtin_unreachable (); + } + return x; +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c b/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c new file mode 100644 index 00000000000..f19808d2941 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ + +typedef struct { + long a; +} c; + +void e(); + +void d() { + c *b; + switch (b->a) + case 8: + case 2: + case 2057594037927936: + case 0: + case 4611686018427387904: + e(); +} diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c index bedeb2fd865..5f8ed46f496 100644 --- a/gcc/tree-switch-conversion.c +++ b/gcc/tree-switch-conversion.c @@ -1284,7 +1284,11 @@ jump_table_cluster::can_be_handled (const vec<cluster *> &clusters, comparison_count += sc->m_range_p ? 2 : 1; } - return 100 * range <= max_ratio * comparison_count; + unsigned HOST_WIDE_INT lhs = 100 * range; + if (lhs < range) + return false; + + return lhs <= max_ratio * comparison_count; } /* Return true if cluster starting at START and ending at END (inclusive)