On Wed, Oct 28, 2020 at 10:44 AM Richard Sandiford via Gcc-patches
<[email protected]> 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));
