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));

Reply via email to