Handle CONST_POLY_INTs in CONST_VECTORs [PR97141, PR98726]
authorRichard Sandiford <richard.sandiford@arm.com>
Wed, 31 Mar 2021 18:34:00 +0000 (19:34 +0100)
committerRichard Sandiford <richard.sandiford@arm.com>
Wed, 31 Mar 2021 18:34:00 +0000 (19:34 +0100)
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<mode>): 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
gcc/config/aarch64/aarch64.c
gcc/emit-rtl.c
gcc/rtx-vector-builder.c
gcc/rtx-vector-builder.h
gcc/testsuite/gcc.c-torture/compile/pr97141.c [new file with mode: 0644]
gcc/testsuite/gcc.c-torture/compile/pr98726.c [new file with mode: 0644]
gcc/testsuite/gcc.target/aarch64/sve/pr97141.c [new file with mode: 0644]
gcc/testsuite/gcc.target/aarch64/sve/pr98726.c [new file with mode: 0644]

index d86e8e7..4edee99 100644 (file)
                && aarch64_mem_pair_operand (operands[0], DImode))
               || known_eq (GET_MODE_SIZE (<MODE>mode), 8))))
       operands[1] = force_reg (<MODE>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>mode, operands[1]))
+    {
+      aarch64_expand_vector_init (operands[0], operands[1]);
+      DONE;
+    }
   "
 )
 
index f878721..994fafc 100644 (file)
@@ -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);
 
index b23284e..1113c58 100644 (file)
@@ -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));
 }
index 58b3e10..cf9b3dd 100644 (file)
@@ -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 <scalar_int_mode> (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);
 }
 
index ea32208..30a91e8 100644 (file)
@@ -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 (file)
index 0000000..1a9ff83
--- /dev/null
@@ -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 (file)
index 0000000..ce24b18
--- /dev/null
@@ -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 (file)
index 0000000..942e4a4
--- /dev/null
@@ -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 (file)
index 0000000..2395cab
--- /dev/null
@@ -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;
+}