On Wed, Oct 28, 2020 at 10:44 AM Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > This PR shows another problem with calculating value ranges for > POLY_INT_CSTs. We have: > > ivtmp_76 = ASSERT_EXPR <ivtmp_60, ivtmp_60 > POLY_INT_CST [9, 4294967294]> > > where the VQ coefficient is unsigned but is effectively acting > as a negative number. We wrongly give the POLY_INT_CST the range: > > [9, INT_MAX] > > and things go downhill from there: later iterations of the unrolled > epilogue are wrongly removed as dead. > > I guess this is the final nail in the coffin for doing VRP on > POLY_INT_CSTs. For other similarly exotic testcases we could have > overflow for any coefficient, not just those that could be treated > as contextually negative. > > Testing TYPE_OVERFLOW_UNDEFINED doesn't seem like an option because we > couldn't handle warn_strict_overflow properly. At this stage we're > just recording a range that might or might not lead to strict-overflow > assumptions later. > > It still feels like we should be able to do something here, but for > now removing the code seems safest. It's also telling that there > are no testsuite failures on SVE from doing this. > > Tested on aarch64-linux-gnu (with and without SVE) and > x86_64-linux-gnu. OK for trunk and backports?
OK. Richard. > Richard > > > gcc/ > PR tree-optimization/97457 > * value-range.cc (irange::set): Don't decay POLY_INT_CST ranges > to integer ranges. > > gcc/testsuite/ > PR tree-optimization/97457 > * gcc.dg/vect/pr97457.c: New test. > --- > gcc/testsuite/gcc.dg/vect/pr97457.c | 15 +++++++++++++++ > gcc/value-range.cc | 30 +++++------------------------ > 2 files changed, 20 insertions(+), 25 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/pr97457.c > > diff --git a/gcc/testsuite/gcc.dg/vect/pr97457.c > b/gcc/testsuite/gcc.dg/vect/pr97457.c > new file mode 100644 > index 00000000000..506ba249b00 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/pr97457.c > @@ -0,0 +1,15 @@ > +/* { dg-additional-options "-O3" } */ > + > +int a; > +long c; > +signed char d(char e, char f) { return e + f; } > +int main(void) { > + for (; a <= 1; a++) { > + c = -8; > + for (; c != 3; c = d(c, 1)) > + ; > + } > + char b = c; > + if (b != 3) > + __builtin_abort(); > +} > diff --git a/gcc/value-range.cc b/gcc/value-range.cc > index 7847104050c..2319c13388a 100644 > --- a/gcc/value-range.cc > +++ b/gcc/value-range.cc > @@ -248,31 +248,11 @@ irange::set (tree min, tree max, value_range_kind kind) > set_undefined (); > return; > } > - if (kind == VR_RANGE) > - { > - /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST bounds. */ > - if (POLY_INT_CST_P (min)) > - { > - tree type_min = vrp_val_min (TREE_TYPE (min)); > - widest_int lb > - = constant_lower_bound_with_limit (wi::to_poly_widest (min), > - wi::to_widest (type_min)); > - min = wide_int_to_tree (TREE_TYPE (min), lb); > - } > - if (POLY_INT_CST_P (max)) > - { > - tree type_max = vrp_val_max (TREE_TYPE (max)); > - widest_int ub > - = constant_upper_bound_with_limit (wi::to_poly_widest (max), > - wi::to_widest (type_max)); > - max = wide_int_to_tree (TREE_TYPE (max), ub); > - } > - } > - else if (kind != VR_VARYING) > - { > - if (POLY_INT_CST_P (min) || POLY_INT_CST_P (max)) > - kind = VR_VARYING; > - } > + > + if (kind != VR_VARYING > + && (POLY_INT_CST_P (min) || POLY_INT_CST_P (max))) > + kind = VR_VARYING; > + > if (kind == VR_VARYING) > { > set_varying (TREE_TYPE (min));