[v2,1/2] fold-const: Do not fold MIN of possibly wrapping unsigned POLY_INT_CSTs
Commit Message
can_min_p compares POLY_INT_CSTs as poly_widest_ints, which zero-extends
the coefficients of an unsigned type. 33 - svcntb () in uint64_t is
POLY_INT_CST [17, 18446744073709551600] (17 - 16x), so
MIN (33 - svcntb (), svcntb ()) folds to svcntb () although the result
is 1 at VL 256. Likewise (uint8_t) svcntb () is POLY_INT_CST [16, 16],
so MIN of it and 5 folds to 5 although it is 0 at VL 2048. Punt when
an unsigned POLY_INT_CST may exceed its type for the bounds given by
TARGET_POLY_INT_INDETERMINATE_BOUND. Without the hook, as on RISC-V,
punt only when a coefficient is negative when read as signed, so
wraps past the top of the type still fold there.
Bootstrapped and regression tested on x86_64-pc-linux-gnu; regression
tested on aarch64-linux-gnu under qemu (the SVE ACLE, vect, aarch64-sve
and execute tests at VL 128, and at VL 2048 with -march=armv8.2-a+sve).
Assisted-by: Claude Opus 5 (Anthropic)
gcc/ChangeLog:
* fold-const.cc (poly_int_cst_bounds): New function.
(maybe_wrapping_poly_p): Likewise.
(can_min_p): Use maybe_wrapping_poly_p to punt on unsigned
POLY_INT_CST operands that may wrap.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/sve/acle/general/min_poly_1.c: New test.
* gcc.target/aarch64/sve/acle/general/min_poly_4.c: New test.
Signed-off-by: Dominic P <gcc@gcc.dp11.uk>
---
Changes in v2: the check is now a static helper, as you suggested. It
also catches unsigned POLY_INT_CSTs that wrap past the top of their
type, such as (uint8_t) svcntb (), using
TARGET_POLY_INT_INDETERMINATE_BOUND (poly_int_cst_bounds also returns
the lower bound, for 2/2); without the hook it behaves as v1.
min_poly_4.c checks the code at any VL; min_poly_1.c runs it. GCC 16
is affected too, but it lacks the hook, so a backport would catch only
the negative coefficients.
gcc/fold-const.cc | 41 ++++++++++++++
.../aarch64/sve/acle/general/min_poly_1.c | 55 +++++++++++++++++++
.../aarch64/sve/acle/general/min_poly_4.c | 51 +++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_1.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_4.c
Comments
Dominic P <gcc@gcc.dp11.uk> writes:
> can_min_p compares POLY_INT_CSTs as poly_widest_ints, which zero-extends
> the coefficients of an unsigned type. 33 - svcntb () in uint64_t is
> POLY_INT_CST [17, 18446744073709551600] (17 - 16x), so
> MIN (33 - svcntb (), svcntb ()) folds to svcntb () although the result
> is 1 at VL 256. Likewise (uint8_t) svcntb () is POLY_INT_CST [16, 16],
> so MIN of it and 5 folds to 5 although it is 0 at VL 2048. Punt when
> an unsigned POLY_INT_CST may exceed its type for the bounds given by
> TARGET_POLY_INT_INDETERMINATE_BOUND. Without the hook, as on RISC-V,
> punt only when a coefficient is negative when read as signed, so
> wraps past the top of the type still fold there.
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu; regression
> tested on aarch64-linux-gnu under qemu (the SVE ACLE, vect, aarch64-sve
> and execute tests at VL 128, and at VL 2048 with -march=armv8.2-a+sve).
>
> Assisted-by: Claude Opus 5 (Anthropic)
>
> gcc/ChangeLog:
>
> * fold-const.cc (poly_int_cst_bounds): New function.
> (maybe_wrapping_poly_p): Likewise.
> (can_min_p): Use maybe_wrapping_poly_p to punt on unsigned
> POLY_INT_CST operands that may wrap.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/sve/acle/general/min_poly_1.c: New test.
> * gcc.target/aarch64/sve/acle/general/min_poly_4.c: New test.
>
> Signed-off-by: Dominic P <gcc@gcc.dp11.uk>
> ---
> Changes in v2: the check is now a static helper, as you suggested. It
> also catches unsigned POLY_INT_CSTs that wrap past the top of their
> type, such as (uint8_t) svcntb (), using
> TARGET_POLY_INT_INDETERMINATE_BOUND (poly_int_cst_bounds also returns
> the lower bound, for 2/2); without the hook it behaves as v1.
> min_poly_4.c checks the code at any VL; min_poly_1.c runs it. GCC 16
> is affected too, but it lacks the hook, so a backport would catch only
> the negative coefficients.
>
> gcc/fold-const.cc | 41 ++++++++++++++
> .../aarch64/sve/acle/general/min_poly_1.c | 55 +++++++++++++++++++
> .../aarch64/sve/acle/general/min_poly_4.c | 51 +++++++++++++++++
> 3 files changed, 147 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_1.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_4.c
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 113558973f6..6795c15b906 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -1144,11 +1144,52 @@ wide_int_binop (wide_int &res,
> return true;
> }
>
> +/* If the target defines TARGET_POLY_INT_INDETERMINATE_BOUND, set *LO and
> + *HI to the smallest and largest values of POLY_INT_CST ARG over those
> + bounds, computed in infinite precision, and return true. Otherwise
> + return false. */
> +
> +static bool
> +poly_int_cst_bounds (const_tree arg, widest_int *lo, widest_int *hi)
> +{
> + if (!targetm.poly_int_indeterminate_bound)
> + return false;
> + poly_uint64 bound = targetm.poly_int_indeterminate_bound ();
> + *lo = *hi = wi::to_widest (POLY_INT_CST_COEFF (arg, 0));
> + for (unsigned int i = 1; i < NUM_POLY_INT_COEFFS; ++i)
> + {
> + auto term = (wi::to_widest (POLY_INT_CST_COEFF (arg, i))
> + * bound.coeffs[i]);
> + (wi::neg_p (term) ? *lo : *hi) += term;
> + }
> + return true;
> +}
> +
> +/* Return true if ARG is an unsigned POLY_INT_CST that may wrap for some
> + runtime value of its indeterminates. Without
> + TARGET_POLY_INT_INDETERMINATE_BOUND, only catch coefficients that are
> + negative when read as signed. */
> +
> +static bool
> +maybe_wrapping_poly_p (const_tree arg)
> +{
> + if (!POLY_INT_CST_P (arg) || !TYPE_UNSIGNED (TREE_TYPE (arg)))
Not a full review, but: this looks like it should use
TYPE_OVERFLOW_WRAPS (and then also check whether lo fits the type).
> + return false;
> + tree type = TREE_TYPE (arg);
> + widest_int lo, hi;
> + if (!poly_int_cst_bounds (arg, &lo, &hi))
> + return maybe_lt (tree_to_poly_int64 (arg), 0);
IMO it doesn't make sense to accept cases here that would be rejected
were TARGET_POLY_INT_INDETERMINATE_BOUND defined. T_P_I_I_B is just
an optimisation; it shouldn't be needed for correctness.
If coefficients 1 and above are the same for both values and are all
powers of 2, we can still calculate the minimum based on coefficient 0.
That should be a somewhat common case.
Thanks,
Richard
> + return !wi::fits_to_tree_p (hi, type);
> +}
> +
> /* Returns true if we know who is smaller or equal, ARG1 or ARG2, and set the
> min value to RES. */
> bool
> can_min_p (const_tree arg1, const_tree arg2, poly_wide_int &res)
> {
> + if (maybe_wrapping_poly_p (arg1) || maybe_wrapping_poly_p (arg2))
> + return false;
> +
> if (known_le (wi::to_poly_widest (arg1), wi::to_poly_widest (arg2)))
> {
> res = wi::to_poly_wide (arg1);
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_1.c
> new file mode 100644
> index 00000000000..896c6169c02
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_1.c
> @@ -0,0 +1,55 @@
> +/* { dg-do run { target aarch64_sve_hw } } */
> +/* { dg-options "-O2" } */
> +
> +#include <arm_sve.h>
> +
> +#define TEST(NAME, T, K, CNT) \
> + T __attribute__ ((noipa)) \
> + NAME (void) \
> + { \
> + T n = (T) K - (T) CNT (); \
> + return n < (T) CNT () ? n : (T) CNT (); \
> + }
> +
> +#define TEST_MUL(NAME, T, K, CNT) \
> + T __attribute__ ((noipa)) \
> + NAME (void) \
> + { \
> + T n = CNT () * K; \
> + return n < 5 ? n : 5; \
> + }
> +
> +TEST (f1, uint64_t, 33, svcntb)
> +TEST (f2, uint32_t, 33, svcnth)
> +TEST (f3, uint64_t, 130, svcntb)
> +TEST (f4, uint64_t, 300, svcntb)
> +TEST_MUL (g1, uint16_t, 1024, svcntb)
> +TEST_MUL (g2, uint8_t, 4, svcnth)
> +TEST_MUL (g3, uint32_t, 0x4000000, svcntb)
> +TEST_MUL (g4, uint8_t, 1, svcntb)
> +
> +int __attribute__ ((optimize (0)))
> +main (void)
> +{
> +#define CHECK(NAME, T, K, CNT) \
> + { \
> + T c = CNT (), n = (T) K - c; \
> + if (NAME () != (n < c ? n : c)) \
> + __builtin_abort (); \
> + }
> +#define CHECK_MUL(NAME, T, K, CNT) \
> + { \
> + T n = CNT () * K; \
> + if (NAME () != (n < 5 ? n : 5)) \
> + __builtin_abort (); \
> + }
> + CHECK (f1, uint64_t, 33, svcntb)
> + CHECK (f2, uint32_t, 33, svcnth)
> + CHECK (f3, uint64_t, 130, svcntb)
> + CHECK (f4, uint64_t, 300, svcntb)
> + CHECK_MUL (g1, uint16_t, 1024, svcntb)
> + CHECK_MUL (g2, uint8_t, 4, svcnth)
> + CHECK_MUL (g3, uint32_t, 0x4000000, svcntb)
> + CHECK_MUL (g4, uint8_t, 1, svcntb)
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_4.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_4.c
> new file mode 100644
> index 00000000000..7ff76a29bb6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/min_poly_4.c
> @@ -0,0 +1,51 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +#include <arm_sve.h>
> +
> +#define TEST(NAME, T, K, CNT) \
> + T NAME (void) \
> + { \
> + T n = (T) K - (T) CNT (); \
> + return n < (T) CNT () ? n : (T) CNT (); \
> + }
> +
> +#define TEST_MUL(NAME, T, K, CNT) \
> + T NAME (void) \
> + { \
> + T n = CNT () * K; \
> + return n < 5 ? n : 5; \
> + }
> +
> +/*
> +** f1:
> +** ...
> +** csel [^\n]+
> +** ret
> +*/
> +TEST (f1, uint64_t, 33, svcntb)
> +
> +/*
> +** f2:
> +** ...
> +** csel [^\n]+
> +** ret
> +*/
> +TEST (f2, uint32_t, 33, svcnth)
> +
> +/*
> +** g1:
> +** ...
> +** csel [^\n]+
> +** ret
> +*/
> +TEST_MUL (g1, uint16_t, 1024, svcntb)
> +
> +/*
> +** g4:
> +** ...
> +** csel [^\n]+
> +** ret
> +*/
> +TEST_MUL (g4, uint8_t, 1, svcntb)
@@ -1144,11 +1144,52 @@ wide_int_binop (wide_int &res,
return true;
}
+/* If the target defines TARGET_POLY_INT_INDETERMINATE_BOUND, set *LO and
+ *HI to the smallest and largest values of POLY_INT_CST ARG over those
+ bounds, computed in infinite precision, and return true. Otherwise
+ return false. */
+
+static bool
+poly_int_cst_bounds (const_tree arg, widest_int *lo, widest_int *hi)
+{
+ if (!targetm.poly_int_indeterminate_bound)
+ return false;
+ poly_uint64 bound = targetm.poly_int_indeterminate_bound ();
+ *lo = *hi = wi::to_widest (POLY_INT_CST_COEFF (arg, 0));
+ for (unsigned int i = 1; i < NUM_POLY_INT_COEFFS; ++i)
+ {
+ auto term = (wi::to_widest (POLY_INT_CST_COEFF (arg, i))
+ * bound.coeffs[i]);
+ (wi::neg_p (term) ? *lo : *hi) += term;
+ }
+ return true;
+}
+
+/* Return true if ARG is an unsigned POLY_INT_CST that may wrap for some
+ runtime value of its indeterminates. Without
+ TARGET_POLY_INT_INDETERMINATE_BOUND, only catch coefficients that are
+ negative when read as signed. */
+
+static bool
+maybe_wrapping_poly_p (const_tree arg)
+{
+ if (!POLY_INT_CST_P (arg) || !TYPE_UNSIGNED (TREE_TYPE (arg)))
+ return false;
+ tree type = TREE_TYPE (arg);
+ widest_int lo, hi;
+ if (!poly_int_cst_bounds (arg, &lo, &hi))
+ return maybe_lt (tree_to_poly_int64 (arg), 0);
+ return !wi::fits_to_tree_p (hi, type);
+}
+
/* Returns true if we know who is smaller or equal, ARG1 or ARG2, and set the
min value to RES. */
bool
can_min_p (const_tree arg1, const_tree arg2, poly_wide_int &res)
{
+ if (maybe_wrapping_poly_p (arg1) || maybe_wrapping_poly_p (arg2))
+ return false;
+
if (known_le (wi::to_poly_widest (arg1), wi::to_poly_widest (arg2)))
{
res = wi::to_poly_wide (arg1);
new file mode 100644
@@ -0,0 +1,55 @@
+/* { dg-do run { target aarch64_sve_hw } } */
+/* { dg-options "-O2" } */
+
+#include <arm_sve.h>
+
+#define TEST(NAME, T, K, CNT) \
+ T __attribute__ ((noipa)) \
+ NAME (void) \
+ { \
+ T n = (T) K - (T) CNT (); \
+ return n < (T) CNT () ? n : (T) CNT (); \
+ }
+
+#define TEST_MUL(NAME, T, K, CNT) \
+ T __attribute__ ((noipa)) \
+ NAME (void) \
+ { \
+ T n = CNT () * K; \
+ return n < 5 ? n : 5; \
+ }
+
+TEST (f1, uint64_t, 33, svcntb)
+TEST (f2, uint32_t, 33, svcnth)
+TEST (f3, uint64_t, 130, svcntb)
+TEST (f4, uint64_t, 300, svcntb)
+TEST_MUL (g1, uint16_t, 1024, svcntb)
+TEST_MUL (g2, uint8_t, 4, svcnth)
+TEST_MUL (g3, uint32_t, 0x4000000, svcntb)
+TEST_MUL (g4, uint8_t, 1, svcntb)
+
+int __attribute__ ((optimize (0)))
+main (void)
+{
+#define CHECK(NAME, T, K, CNT) \
+ { \
+ T c = CNT (), n = (T) K - c; \
+ if (NAME () != (n < c ? n : c)) \
+ __builtin_abort (); \
+ }
+#define CHECK_MUL(NAME, T, K, CNT) \
+ { \
+ T n = CNT () * K; \
+ if (NAME () != (n < 5 ? n : 5)) \
+ __builtin_abort (); \
+ }
+ CHECK (f1, uint64_t, 33, svcntb)
+ CHECK (f2, uint32_t, 33, svcnth)
+ CHECK (f3, uint64_t, 130, svcntb)
+ CHECK (f4, uint64_t, 300, svcntb)
+ CHECK_MUL (g1, uint16_t, 1024, svcntb)
+ CHECK_MUL (g2, uint8_t, 4, svcnth)
+ CHECK_MUL (g3, uint32_t, 0x4000000, svcntb)
+ CHECK_MUL (g4, uint8_t, 1, svcntb)
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,51 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+#include <arm_sve.h>
+
+#define TEST(NAME, T, K, CNT) \
+ T NAME (void) \
+ { \
+ T n = (T) K - (T) CNT (); \
+ return n < (T) CNT () ? n : (T) CNT (); \
+ }
+
+#define TEST_MUL(NAME, T, K, CNT) \
+ T NAME (void) \
+ { \
+ T n = CNT () * K; \
+ return n < 5 ? n : 5; \
+ }
+
+/*
+** f1:
+** ...
+** csel [^\n]+
+** ret
+*/
+TEST (f1, uint64_t, 33, svcntb)
+
+/*
+** f2:
+** ...
+** csel [^\n]+
+** ret
+*/
+TEST (f2, uint32_t, 33, svcnth)
+
+/*
+** g1:
+** ...
+** csel [^\n]+
+** ret
+*/
+TEST_MUL (g1, uint16_t, 1024, svcntb)
+
+/*
+** g4:
+** ...
+** csel [^\n]+
+** ret
+*/
+TEST_MUL (g4, uint8_t, 1, svcntb)