From: Kyrylo Tkachov <[email protected]>
The ADDP patterns describe their result with vec_select and vec_concat,
which use GCC's lane numbering. On big-endian targets this numbering is
the reverse of the architectural one, so the RTL says that the first
input gives the low half of the result, while the instruction puts it in
the high half. The fix for PR114890 removed the big-endian operand swap
from the vpadd expander. That made the intrinsics correct but left the
RTL wrong, so RTL simplification can compute wrong results. With
-mbig-endian -O2:
int32_t
f (int32x4_t a, int32x4_t b)
{
return vgetq_lane_s32 (vpaddq_s32 (a, b), 0);
}
Before:
addp v1.4s, v1.4s, v1.4s
umov x0, v1.d[0]
After:
addp v0.4s, v0.4s, v0.4s
umov x0, v0.d[0]
Print the operands in the other order on big-endian targets, and swap
the intrinsic operands in the expander again. The same-register split
now also applies on big-endian targets.
The new runtime test fails under big-endian QEMU without this patch and
passes with it.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
Cc: Tamar Christina <[email protected]>
Cc: Wilco Dijkstra <[email protected]>
gcc/ChangeLog:
* config/aarch64/aarch64-simd.md (aarch64_addp<mode>_insn): Swap
the input operands on big-endian targets.
(aarch64_addp<mode><vczle><vczbe>_insn): Likewise.
(aarch64_addp<mode>): Swap the input operands on big-endian
targets.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/simd/addp-same-low_1.c: Also run on big-endian
targets.
* gcc.target/aarch64/simd/vpadd_lane_1.c: New test.
* gcc.target/aarch64/simd/vpadd_lane_2.c: Likewise.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/config/aarch64/aarch64-simd.md | 20 ++++-
.../gcc.target/aarch64/simd/addp-same-low_1.c | 1 -
.../gcc.target/aarch64/simd/vpadd_lane_1.c | 75 +++++++++++++++++++
.../gcc.target/aarch64/simd/vpadd_lane_2.c | 31 ++++++++
4 files changed, 123 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_1.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_2.c
diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index 98c2da034c5..f928c5958cd 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -8310,6 +8310,8 @@
;; a pairwise addition on the result. For 128-bit input modes represent this
;; as a concatenation of the pairwise addition results of the two input
;; registers. This allow us to avoid using intermediate 256-bit modes.
+;; The RTL uses GCC's lane numbering, so on big-endian targets operand 1
+;; provides the architecturally high half of the result.
(define_insn "aarch64_addp<mode>_insn"
[(set (match_operand:VQ_I 0 "register_operand" "=w")
(vec_concat:VQ_I
@@ -8328,14 +8330,19 @@
(match_dup 2)
(match_dup 4)))))]
"TARGET_SIMD && !rtx_equal_p (operands[3], operands[4])"
- "addp\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>"
+ {
+ if (BYTES_BIG_ENDIAN)
+ return "addp\t%<v>0<Vmtype>, %<v>2<Vmtype>, %<v>1<Vmtype>";
+ return "addp\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>";
+ }
[(set_attr "type" "neon_reduc_add<q>")]
)
;; For 64-bit input modes an ADDP is represented as a concatenation
;; of the input registers into an 128-bit register which is then fed
;; into a pairwise add. That way we avoid having to create intermediate
-;; 32-bit vector modes.
+;; 32-bit vector modes. As above, operand 1 provides the architecturally
+;; high half of the result on big-endian targets.
(define_insn "aarch64_addp<mode><vczle><vczbe>_insn"
[(set (match_operand:VD_BHSI 0 "register_operand" "=w")
(plus:VD_BHSI
@@ -8350,7 +8357,11 @@
(match_dup 2))
(match_operand:<VDBL> 4 "vect_par_cnst_even_or_odd_half"))))]
"TARGET_SIMD && !rtx_equal_p (operands[3], operands[4])"
- "addp\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>"
+ {
+ if (BYTES_BIG_ENDIAN)
+ return "addp\t%<v>0<Vmtype>, %<v>2<Vmtype>, %<v>1<Vmtype>";
+ return "addp\t%<v>0<Vmtype>, %<v>1<Vmtype>, %<v>2<Vmtype>";
+ }
[(set_attr "type" "neon_reduc_add<q>")]
)
@@ -8395,6 +8406,9 @@
nunits /= 2;
rtx par_even = aarch64_gen_stepped_int_parallel (nunits, 0, 2);
rtx par_odd = aarch64_gen_stepped_int_parallel (nunits, 1, 2);
+ /* The intrinsic uses architectural lane numbering. */
+ if (BYTES_BIG_ENDIAN)
+ std::swap (operands[1], operands[2]);
emit_insn (gen_aarch64_addp<mode>_insn (operands[0], operands[1],
operands[2], par_even, par_odd));
DONE;
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addp-same-low_1.c
b/gcc/testsuite/gcc.target/aarch64/simd/addp-same-low_1.c
index 42d8db77f1c..fb182db8e8d 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd/addp-same-low_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd/addp-same-low_1.c
@@ -1,6 +1,5 @@
/* { dg-do compile } */
/* { dg-additional-options "-O" } */
-/* { dg-require-effective-target aarch64_little_endian } */
/* { dg-final { check-function-bodies "**" "" "" } } */
#include <arm_neon.h>
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_1.c
b/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_1.c
new file mode 100644
index 00000000000..5b679018c2b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_1.c
@@ -0,0 +1,75 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include <arm_neon.h>
+
+__attribute__ ((noipa)) int32_t
+lane0_q (int32x4_t a, int32x4_t b)
+{
+ return vgetq_lane_s32 (vpaddq_s32 (a, b), 0);
+}
+
+__attribute__ ((noipa)) int32_t
+lane3_q (int32x4_t a, int32x4_t b)
+{
+ return vgetq_lane_s32 (vpaddq_s32 (a, b), 3);
+}
+
+__attribute__ ((noipa)) int32x2_t
+low_q (int32x4_t a, int32x4_t b)
+{
+ return vget_low_s32 (vpaddq_s32 (a, b));
+}
+
+__attribute__ ((noipa)) int32x2_t
+high_q (int32x4_t a, int32x4_t b)
+{
+ return vget_high_s32 (vpaddq_s32 (a, b));
+}
+
+__attribute__ ((noipa)) int16_t
+lane0_d (int16x4_t a, int16x4_t b)
+{
+ return vget_lane_s16 (vpadd_s16 (a, b), 0);
+}
+
+__attribute__ ((noipa)) int16_t
+lane3_d (int16x4_t a, int16x4_t b)
+{
+ return vget_lane_s16 (vpadd_s16 (a, b), 3);
+}
+
+__attribute__ ((noipa)) int64_t
+lane1_q64 (int64x2_t a, int64x2_t b)
+{
+ return vgetq_lane_s64 (vpaddq_s64 (a, b), 1);
+}
+
+int
+main (void)
+{
+ int32_t a32[] = { 1, 2, 4, 8 };
+ int32_t b32[] = { 16, 32, 64, 128 };
+ int16_t a16[] = { 1, 2, 4, 8 };
+ int16_t b16[] = { 16, 32, 64, 128 };
+ int64_t a64[] = { 1, 2 };
+ int64_t b64[] = { 4, 8 };
+ int32x4_t a = vld1q_s32 (a32);
+ int32x4_t b = vld1q_s32 (b32);
+ int32_t res[2];
+
+ if (lane0_q (a, b) != 3 || lane3_q (a, b) != 192)
+ __builtin_abort ();
+ vst1_s32 (res, low_q (a, b));
+ if (res[0] != 3 || res[1] != 12)
+ __builtin_abort ();
+ vst1_s32 (res, high_q (a, b));
+ if (res[0] != 48 || res[1] != 192)
+ __builtin_abort ();
+ if (lane0_d (vld1_s16 (a16), vld1_s16 (b16)) != 3
+ || lane3_d (vld1_s16 (a16), vld1_s16 (b16)) != 192)
+ __builtin_abort ();
+ if (lane1_q64 (vld1q_s64 (a64), vld1q_s64 (b64)) != 12)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_2.c
b/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_2.c
new file mode 100644
index 00000000000..e100e282a7d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vpadd_lane_2.c
@@ -0,0 +1,31 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target aarch64_mbig_endian } */
+/* Avoid a dependence on big-endian C library headers. */
+/* { dg-options "-O2 -mbig-endian -ffreestanding" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+#include <arm_neon.h>
+
+/* The architectural lanes of the low half depend only on A. */
+
+/*
+** lane0_q:
+** addp (v[0-9]+)\.4s, v0\.4s, v0\.4s
+** ...
+*/
+int32_t
+lane0_q (int32x4_t a, int32x4_t b)
+{
+ return vgetq_lane_s32 (vpaddq_s32 (a, b), 0);
+}
+
+/*
+** low_q:
+** addp v0\.4s, v0\.4s, v0\.4s
+** ret
+*/
+int32x2_t
+low_q (int32x4_t a, int32x4_t b)
+{
+ return vget_low_s32 (vpaddq_s32 (a, b));
+}
--
2.50.1 (Apple Git-155)
