Ping. https://gcc.gnu.org/pipermail/gcc-patches/2026-October/733600.html
Thanks, Kyrill > On 6 Oct 2026, at 15:55, Kyrylo Tkachov <[email protected]> wrote: > > 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) >
