From 1b5f74e8be4dd7abe5624ff60adceff19ca71bda Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Wed, 31 Mar 2021 19:34:00 +0100 Subject: Handle CONST_POLY_INTs in CONST_VECTORs [PR97141, PR98726] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is caused by POLY_INT_CSTs being (necessarily) valid in tree-level VECTOR_CSTs but CONST_POLY_INTs not being valid in RTL CONST_VECTORs. I can't tell/remember how deliberate that was, but I'm guessing not very. In particular, valid_for_const_vector_p was added to guard against symbolic constants rather than CONST_POLY_INTs. I did briefly consider whether we should maintain the current status anyway. However, that would then require a way of constructing variable-length vectors from individiual elements if, say, we have: { [2, 2], [3, 2], [4, 2], … } So I'm chalking this up to an oversight. I think the intention (and certainly the natural thing) is to have the same rules for both trees and RTL. The SVE CONST_VECTOR code should already be set up to handle CONST_POLY_INTs. However, we need to add support for Advanced SIMD CONST_VECTORs that happen to contain SVE-based values. The patch does that by expanding such CONST_VECTORs in the same way as variable vectors. gcc/ PR rtl-optimization/97141 PR rtl-optimization/98726 * emit-rtl.c (valid_for_const_vector_p): Return true for CONST_POLY_INT_P. * rtx-vector-builder.h (rtx_vector_builder::step): Return a poly_wide_int instead of a wide_int. (rtx_vector_builder::apply_set): Take a poly_wide_int instead of a wide_int. * rtx-vector-builder.c (rtx_vector_builder::apply_set): Likewise. * config/aarch64/aarch64.c (aarch64_legitimate_constant_p): Return false for CONST_VECTORs that cannot be forced to memory. * config/aarch64/aarch64-simd.md (mov): If a CONST_VECTOR is too complex to force to memory, build it up from individual elements instead. gcc/testsuite/ PR rtl-optimization/97141 PR rtl-optimization/98726 * gcc.c-torture/compile/pr97141.c: New test. * gcc.c-torture/compile/pr98726.c: Likewise. * gcc.target/aarch64/sve/pr97141.c: Likewise. * gcc.target/aarch64/sve/pr98726.c: Likewise. --- gcc/config/aarch64/aarch64-simd.md | 11 +++++++++++ gcc/config/aarch64/aarch64.c | 24 ++++++++++++++---------- gcc/emit-rtl.c | 1 + gcc/rtx-vector-builder.c | 6 +++--- gcc/rtx-vector-builder.h | 10 +++++----- gcc/testsuite/gcc.c-torture/compile/pr97141.c | 8 ++++++++ gcc/testsuite/gcc.c-torture/compile/pr98726.c | 7 +++++++ gcc/testsuite/gcc.target/aarch64/sve/pr97141.c | 10 ++++++++++ gcc/testsuite/gcc.target/aarch64/sve/pr98726.c | 9 +++++++++ 9 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr97141.c create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr98726.c create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr97141.c create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr98726.c (limited to 'gcc') diff --git a/gcc/config/aarch64/aarch64-simd.md b/gcc/config/aarch64/aarch64-simd.md index d86e8e72..4edee99 100644 --- a/gcc/config/aarch64/aarch64-simd.md +++ b/gcc/config/aarch64/aarch64-simd.md @@ -35,6 +35,17 @@ && aarch64_mem_pair_operand (operands[0], DImode)) || known_eq (GET_MODE_SIZE (mode), 8)))) operands[1] = force_reg (mode, operands[1]); + + /* If a constant is too complex to force to memory (e.g. because it + contains CONST_POLY_INTs), build it up from individual elements instead. + We should only need to do this before RA; aarch64_legitimate_constant_p + should ensure that we don't try to rematerialize the constant later. */ + if (GET_CODE (operands[1]) == CONST_VECTOR + && targetm.cannot_force_const_mem (mode, operands[1])) + { + aarch64_expand_vector_init (operands[0], operands[1]); + DONE; + } " ) diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index f878721..994fafc 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -17925,10 +17925,22 @@ aarch64_legitimate_constant_p (machine_mode mode, rtx x) { /* Support CSE and rematerialization of common constants. */ if (CONST_INT_P (x) - || (CONST_DOUBLE_P (x) && GET_MODE_CLASS (mode) == MODE_FLOAT) - || GET_CODE (x) == CONST_VECTOR) + || (CONST_DOUBLE_P (x) && GET_MODE_CLASS (mode) == MODE_FLOAT)) return true; + /* Only accept variable-length vector constants if they can be + handled directly. + + ??? It would be possible (but complex) to handle rematerialization + of other constants via secondary reloads. */ + if (!GET_MODE_SIZE (mode).is_constant ()) + return aarch64_simd_valid_immediate (x, NULL); + + /* Otherwise, accept any CONST_VECTOR that, if all else fails, can at + least be forced to memory and loaded from there. */ + if (GET_CODE (x) == CONST_VECTOR) + return !targetm.cannot_force_const_mem (mode, x); + /* Do not allow vector struct mode constants for Advanced SIMD. We could support 0 and -1 easily, but they need support in aarch64-simd.md. */ @@ -17936,14 +17948,6 @@ aarch64_legitimate_constant_p (machine_mode mode, rtx x) if (vec_flags == (VEC_ADVSIMD | VEC_STRUCT)) return false; - /* Only accept variable-length vector constants if they can be - handled directly. - - ??? It would be possible to handle rematerialization of other - constants via secondary reloads. */ - if (vec_flags & VEC_ANY_SVE) - return aarch64_simd_valid_immediate (x, NULL); - if (GET_CODE (x) == HIGH) x = XEXP (x, 0); diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index b23284e..1113c58 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -5949,6 +5949,7 @@ bool valid_for_const_vector_p (machine_mode, rtx x) { return (CONST_SCALAR_INT_P (x) + || CONST_POLY_INT_P (x) || CONST_DOUBLE_AS_FLOAT_P (x) || CONST_FIXED_P (x)); } diff --git a/gcc/rtx-vector-builder.c b/gcc/rtx-vector-builder.c index 58b3e10..cf9b3dd 100644 --- a/gcc/rtx-vector-builder.c +++ b/gcc/rtx-vector-builder.c @@ -46,11 +46,11 @@ rtx_vector_builder::build (rtvec v) rtx rtx_vector_builder::apply_step (rtx base, unsigned int factor, - const wide_int &step) const + const poly_wide_int &step) const { scalar_int_mode int_mode = as_a (GET_MODE_INNER (m_mode)); - return immed_wide_int_const (wi::add (rtx_mode_t (base, int_mode), - factor * step), + return immed_wide_int_const (wi::to_poly_wide (base, int_mode) + + factor * step, int_mode); } diff --git a/gcc/rtx-vector-builder.h b/gcc/rtx-vector-builder.h index ea32208..30a91e8 100644 --- a/gcc/rtx-vector-builder.h +++ b/gcc/rtx-vector-builder.h @@ -44,8 +44,8 @@ private: bool equal_p (rtx, rtx) const; bool allow_steps_p () const; bool integral_p (rtx) const; - wide_int step (rtx, rtx) const; - rtx apply_step (rtx, unsigned int, const wide_int &) const; + poly_wide_int step (rtx, rtx) const; + rtx apply_step (rtx, unsigned int, const poly_wide_int &) const; bool can_elide_p (rtx) const { return true; } void note_representative (rtx *, rtx) {} @@ -115,11 +115,11 @@ rtx_vector_builder::integral_p (rtx elt) const /* Return the value of element ELT2 minus the value of element ELT1. Both elements are known to be CONST_SCALAR_INT_Ps. */ -inline wide_int +inline poly_wide_int rtx_vector_builder::step (rtx elt1, rtx elt2) const { - return wi::sub (rtx_mode_t (elt2, GET_MODE_INNER (m_mode)), - rtx_mode_t (elt1, GET_MODE_INNER (m_mode))); + return (wi::to_poly_wide (elt2, GET_MODE_INNER (m_mode)) + - wi::to_poly_wide (elt1, GET_MODE_INNER (m_mode))); } #endif diff --git a/gcc/testsuite/gcc.c-torture/compile/pr97141.c b/gcc/testsuite/gcc.c-torture/compile/pr97141.c new file mode 100644 index 0000000..1a9ff83 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr97141.c @@ -0,0 +1,8 @@ +int a; +short b, c; +short d(short e, short f) { return e + f; } +void g(void) { + a = -9; + for (; a != 51; a = d(a, 5)) + b |= c; +} diff --git a/gcc/testsuite/gcc.c-torture/compile/pr98726.c b/gcc/testsuite/gcc.c-torture/compile/pr98726.c new file mode 100644 index 0000000..ce24b18 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr98726.c @@ -0,0 +1,7 @@ +int a, c; +char b; +int d() { + a = 0; + for (; a <= 21; a = (short)a + 1) + b &= c; +} diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr97141.c b/gcc/testsuite/gcc.target/aarch64/sve/pr97141.c new file mode 100644 index 0000000..942e4a4 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr97141.c @@ -0,0 +1,10 @@ +/* { dg-options "-O3" } */ + +int a; +short b, c; +short d(short e, short f) { return e + f; } +void g(void) { + a = -9; + for (; a != 51; a = d(a, 5)) + b |= c; +} diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr98726.c b/gcc/testsuite/gcc.target/aarch64/sve/pr98726.c new file mode 100644 index 0000000..2395cab --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr98726.c @@ -0,0 +1,9 @@ +/* { dg-options "-O3" } */ + +int a, c; +char b; +int d() { + a = 0; + for (; a <= 21; a = (short)a + 1) + b &= c; +} -- cgit v1.1