Re: Add a constant_range_value_p function (PR 92033)

[Christophe Lyon <christophe.lyon@linaro.org>]

On Tue, 15 Oct 2019 at 12:36, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
> >>Richard Biener <richard.guenther@gmail.com> writes:
> >>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
> >>> <richard.sandiford@arm.com> wrote:
> >>>>
> >>>> The range-tracking code has a pretty hard-coded assumption that
> >>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
> >>>> ADDR_EXPR".  It seems better to add a predicate specifically for
> >>>> that rather than contiually fight cases in which it can't handle
> >>>> other invariants.
> >>>>
> >>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >>>
> >>> ICK.  Nobody is going to remember this new restriction and
> >>> constant_range_value_p reads like constant_value_range_p ;)
> >>>
> >>> Btw, is_gimple_invariant_address shouldn't have been exported,
> >>> it's only use could have used is_gimple_min_invariant...
> >>
> >>What do you think we should do instead?
> >
> > Just handle POLY_INT_CST in a few place to quickly enough drop to varying.
>
> OK, how about this?  Aldy's suggestion would be fine by me too,
> but I thought I'd try this first given Aldy's queasiness about
> allowing POLY_INT_CSTs further in.
>
> The main case in which this gives useful ranges is a lower bound
> of A + B * X becoming A when B >= 0.  E.g.:
>
>   (1) [32 + 16X, 100] -> [32, 100]
>   (2) [32 + 16X, 32 + 16X] -> [32, MAX]
>
> But the same thing can be useful for the upper bound with negative
> X coefficients.
>
> We can revisit this later if keeping a singleton range for (2)
> would be better.
>
> Tested as before.
>
> Richard
>
>
Hi Richard,

This patch did improve aarch64 results quite a lot, however, there are
still a few failures that used to pass circa r276650:
    gcc.target/aarch64/sve/loop_add_6.c -march=armv8.2-a+sve
scan-assembler \\tfsub\\tz[0-9]+\\.d, p[0-7]/m
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tfsub\\tz[0-9]+\\.d, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tfsub\\tz[0-9]+\\.s, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.b, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.d, p[0-7]/m 2
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.h, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.s, p[0-7]/m 2

Just to make sure you are aware of these :-)

Thanks,

Christophe

> 2019-10-15  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR middle-end/92033
>         * poly-int.h (constant_lower_bound_with_limit): New function.
>         (constant_upper_bound_with_limit): Likewise.
>         * doc/poly-int.texi: Document them.
>         * tree-vrp.c (value_range_base::set): Convert POLY_INT_CST bounds
>         into the worst-case INTEGER_CST bounds.
>
> Index: gcc/poly-int.h
> ===================================================================
> --- gcc/poly-int.h      2019-07-10 19:41:26.395898027 +0100
> +++ gcc/poly-int.h      2019-10-15 11:30:14.099625553 +0100
> @@ -1528,6 +1528,29 @@ constant_lower_bound (const poly_int_pod
>    return a.coeffs[0];
>  }
>
> +/* Return the constant lower bound of A, given that it is no less than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_lower_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_ge (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
> +/* Return the constant upper bound of A, given that it is no greater
> +   than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_upper_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_le (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
>  /* Return a value that is known to be no greater than A and B.  This
>     will be the greatest lower bound for some indeterminate values but
>     not necessarily for all.  */
> Index: gcc/doc/poly-int.texi
> ===================================================================
> --- gcc/doc/poly-int.texi       2019-03-08 18:14:25.333011645 +0000
> +++ gcc/doc/poly-int.texi       2019-10-15 11:30:14.099625553 +0100
> @@ -803,6 +803,18 @@ the assertion is known to hold.
>  @item constant_lower_bound (@var{a})
>  Assert that @var{a} is nonnegative and return the smallest value it can have.
>
> +@item constant_lower_bound_with_limit (@var{a}, @var{b})
> +Return the least value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no less than @var{b}.
> +In other words, the caller is asserting that @var{a} is greater than or
> +equal to @var{b} even if @samp{known_ge (@var{a}, @var{b})} doesn't hold.
> +
> +@item constant_upper_bound_with_limit (@var{a}, @var{b})
> +Return the greatest value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no greater than @var{b}.
> +In other words, the caller is asserting that @var{a} is less than or equal
> +to @var{b} even if @samp{known_le (@var{a}, @var{b})} doesn't hold.
> +
>  @item lower_bound (@var{a}, @var{b})
>  Return a value that is always less than or equal to both @var{a} and @var{b}.
>  It will be the greatest such value for some indeterminate values
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      2019-10-14 09:04:54.515259320 +0100
> +++ gcc/tree-vrp.c      2019-10-15 11:30:14.099625553 +0100
> @@ -727,6 +727,24 @@ value_range_base::set (enum value_range_
>        return;
>      }
>
> +  /* 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), true);
> +      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), true);
> +      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);
> +    }
> +
>    /* Nothing to canonicalize for symbolic ranges.  */
>    if (TREE_CODE (min) != INTEGER_CST
>        || TREE_CODE (max) != INTEGER_CST)