Hi,
The following fixes PR123951. Since r14-3381 the vec_extract/vec_set
idiom used by the vcopy*_lane* intrinsics is folded into a
VEC_PERM_EXPR, so the constant permute expander decides what to emit
for it instead of combine. For two-element vectors an insert
permutation is ambiguous: { 0, 3 } is op0 with element 1 of op1
inserted, but equally op1 with element 0 of op0 inserted.
aarch64_evpc_ins hard-wires the first reading, and because the match.pd
canonicalization swaps the VEC_PERM_EXPR operands whenever the selector
starts with an element of the second vector, half of the lane
combinations tie the result to the wrong input, which the register
allocator can only satisfy with an extra move.
On big endian, arm_neon.h's lane flip puts vcopyq_laneq_u64 (a, 1, b, 1)
in that half, so it regressed from one instruction in GCC 13 to two,
and vect_copy_lane_1.c and vget_set_lane_1.c fail on aarch64_be.
Little endian has the mirror problem for lane 0, which the testsuite
didn't cover. The generated sequence is still functionally correct --
the reporter also confirms the runtime values are fine in the PR -- so
despite the keyword this is a performance and testsuite regression
rather than wrong code, and the PR may be better recategorized from
tree-optimization to target, the fix being entirely within the
backend.
This is the remaining half of what PR112375 covered for little endian:
there the (1,0)/(0,1) combinations became ZIP1/ZIP2 and r14-9137
updated the test, on the reasoning that a three-operand ZIP is
preferable to a destructive INS anyway. For the { 0, 3 } selector no
three-operand instruction exists, and no fixed choice of which input
to tie can work either: the canonical VEC_PERM_EXPR is identical for
both readings up to operand order, each input contributes exactly one
lane, and I verified experimentally that hard-wiring the other
decomposition in aarch64_evpc_ins just moves the failures to
little-endian (0,0)/(1,1). Which input should be tied only becomes
known at register allocation time.
So give the allocator the choice: emit a plain two-input vec_merge
through a new insn that has both tying alternatives. The two
templates insert the live lane of whichever input was not tied, and
the mask stays restricted to the two values for which that is correct.
The first alternative inserts into operand 1, matching what
aarch64_evpc_ins did before, so cases that were already a single INS
are left undisturbed when the allocator has a free choice. This keeps
to standard RTL (no unspec), so combine can also match the form if it
constructs it from other sequences. Both lane combinations of
vcopy_lane_{s,u,f}32 and vcopyq_laneq_{s,u,f}64 now expand to a single
INS on both endiannesses; __builtin_shuffle permutations of the same
shape (pessimized this way since at least GCC 12) improve the same
way, including when the two-element form only appears after
aarch64_evpc_reencode. The ZIP choices for the cross-lane
combinations are deliberately left alone.
The one place where the allocator legitimately makes the opposite
choice is the double add/sub merge in simd/addsub_{1,2}.c: there the
merge feeds a store, both inputs are dead, and the freely allocated
result now inserts into lane 0 rather than lane 1 -- still a single
INS, same value. Their check-function-bodies expectations are updated
to match, as in r14-9137 for the sibling little-endian case.
Bootstrapped and regression-tested on aarch64-linux-gnu (native, in a
single back-to-back session: full 3-stage bootstrap plus `make -k
check` of the unpatched parent and of the fix, same container and
flags). Across gcc, g++, libstdc++, libgomp, libitm and libatomic
there are no PASS->FAIL regressions; the only result changes are the
new gcc.target/aarch64/pr123951_{1,2}.c checks passing. addsub_{1,2}.c
pass on both sides with their respective expectations. The one
apparent g++ tsan execution blip is a scheduling flake that fails and
passes for both the base and the patched compiler on repeated runs.
Also cross-tested with aarch64_be-linux-gnu and aarch64-linux-gnu
compilers over gcc.target/aarch64, and the emitted little-endian
sequences were executed against a scalar reference on aarch64
hardware.
The regression is present on the 14, 15 and 16 branches too. The
backport is the same change minus the addsub_{1,2}.c hunks (those
tests' e1 case does not exist on the branches); it applies cleanly to
15 and 16, and to 14 with a trivial context adjustment in
aarch64-simd.md (the neighbouring copy_lane pattern is still named
*aarch64_simd_vec_copy_lane_... there). OK for trunk, and for the
release branches after a soak period?
Thanks,
Rohith Kapelli
Rohith Kapelli (1):
aarch64: Avoid extra move for two-element vector lane copies
[PR123951]
gcc/config/aarch64/aarch64-simd.md | 31 ++++++++
gcc/config/aarch64/aarch64.cc | 74 ++++++++++++++-----
gcc/testsuite/gcc.target/aarch64/pr123951_1.c | 41 ++++++++++
gcc/testsuite/gcc.target/aarch64/pr123951_2.c | 35 +++++++++
.../gcc.target/aarch64/simd/addsub_1.c | 2 +-
.../gcc.target/aarch64/simd/addsub_2.c | 2 +-
6 files changed, 166 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr123951_1.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr123951_2.c
--
2.53.0
