> -----Original Message-----
> From: Tamar Christina
> Sent: 31 July 2026 14:51
> To: Richard Sandiford <[email protected]>
> Cc: Rohith Kapelli <[email protected]>; [email protected];
> Kyrylo Tkachov <[email protected]>; Richard Earnshaw
> <[email protected]>
> Subject: RE: [PATCH 1/1] aarch64: Avoid extra move for two-element vector
> lane copies [PR123951]
>
> > -----Original Message-----
> > From: Tamar Christina
> > Sent: 31 July 2026 14:47
> > To: 'Richard Sandiford' <[email protected]>
> > Cc: Rohith Kapelli <[email protected]>; [email protected];
> > Kyrylo Tkachov <[email protected]>; Richard Earnshaw
> > <[email protected]>
> > Subject: RE: [PATCH 1/1] aarch64: Avoid extra move for two-element vector
> > lane copies [PR123951]
> >
> > > -----Original Message-----
> > > From: Richard Sandiford <[email protected]>
> > > Sent: 31 July 2026 14:41
> > > To: Tamar Christina <[email protected]>
> > > Cc: Rohith Kapelli <[email protected]>; [email protected];
> > > Kyrylo Tkachov <[email protected]>; Richard Earnshaw
> > > <[email protected]>
> > > Subject: Re: [PATCH 1/1] aarch64: Avoid extra move for two-element
> vector
> > > lane copies [PR123951]
> > >
> > > Tamar Christina <[email protected]> writes:
> > > > Hi Rohith,
> > > >
> > > > Sorry for the delay.
> > > >
> > > >> -----Original Message-----
> > > >> From: Rohith Kapelli <[email protected]>
> > > >> Sent: 19 July 2026 11:44
> > > >> To: [email protected]
> > > >> Cc: Richard Sandiford <[email protected]>; Kyrylo Tkachov
> > > >> <[email protected]>; Richard Earnshaw
> > > <[email protected]>;
> > > >> Rohith Kapelli <[email protected]>
> > > >> Subject: [PATCH 1/1] aarch64: Avoid extra move for two-element vector
> > > lane
> > > >> copies [PR123951]
> > > >>
> > > >> Since r14-3381 the vec_extract/vec_set idiom used by the vcopy*_lane*
> > > >> intrinsics is folded into a VEC_PERM_EXPR by forwprop, so the constant
> > > >> permute expander rather than combine now decides what to emit for it.
> > > >>
> > > >> For two-element vectors, an insert permutation matches two different
> > > >> decompositions at the same time: { 0, 3 } is op0 with element 1 of op1
> > > >> inserted at index 1, but equally op1 with element 0 of op0 inserted at
> > > >> index 0. aarch64_evpc_ins always picks the first form, tying the
> > > >> result to op0. The match.pd canonicalization of VEC_PERM_EXPR
> swaps
> > > >> the operands whenever the selector starts with an element of the
> > > >> second vector, so for half of the lane combinations the vector that
> > > >> the intrinsic inserts into arrives as op1, and the register allocator
> > > >> then has to satisfy the tie with an extra move.
> > > >>
> > > >> On big endian the lane numbering flip in arm_neon.h makes
> > > >> vcopyq_laneq_u64 (a, 1, b, 1) take exactly that path, so what used to
> > > >> be a single INS in GCC 13 became INS plus MOV and vect_copy_lane_1.c
> > > >> started failing. Little endian has the same problem for
> > > >> vcopyq_laneq_u64 (a, 0, b, 0), and __builtin_shuffle permutations of
> > > >> this shape have always been pessimized this way, even before the
> > > >> intrinsics started using this path.
> > > >>
> > > >> The expander cannot make the right choice: each input contributes
> > > >> exactly one lane, the canonical VEC_PERM_EXPR is the same for both
> > > >> readings, and which form is cheaper only becomes known during
> register
> > > >> allocation. Emit a plain two-input vec_merge instead and give it an
> > > >> insn with both tying alternatives, so that the allocator resolves the
> > > >> tie and no move is needed for either lane combination, on either
> > > >> endianness.
> > > >>
> > > >
> > > > I don't think you actually need the expander changes as the majority
> > > > of the heavy lifting here is being done by the new VP_2E pattern you
> > > > added.
> > > >
> > > > So I think what you want is to change aarch64_simd_vec_copy_lane and
> > > > to exclude VP_2E modes and then make a new pattern to cover these
> > having
> > > the same
> > > > operand swap as your new pattern. So
> > > >
> > > > something like
> > > >
> > > > (define_insn "@aarch64_simd_vec_copy_lane<mode>"
> > > > [(set (match_operand:VP_2E 0 "register_operand" "=w,w")
> > > > (vec_merge:VP_2E
> > > > (vec_duplicate:VP_2E
> > > > (vec_select:<VEL>
> > > > (match_operand:VP_2E 3 "register_operand" "w,0")
> > > > (parallel
> > > > [(match_operand:SI 4 "immediate_operand" "i,i")])))
> > > > (match_operand:VP_2E 1 "register_operand" "0,w")
> > > > (match_operand:SI 2 "immediate_operand" "i,i")))]
> > > > "TARGET_SIMD
> > > > && exact_log2 (INTVAL (operands[2])) >= 0
> > > > && INTVAL (operands[4]) == exact_log2 (INTVAL (operands[2]))"
> > > > {
> > > >
> > > >> The addsub_1.c and addsub_2.c body checks match the same
> > > >> addend/subtrahend merge; with the tie left to the allocator the freely
> > > >> allocated case now inserts into lane 0 rather than lane 1, still a
> > > >> single INS, so their expected output is updated accordingly.
> > > >>
> > > >> gcc/ChangeLog:
> > > >>
> > > >> PR tree-optimization/123951
> > > >> * config/aarch64/aarch64-simd.md
> > > >> (@aarch64_simd_vec_merge<mode>):
> > > >> New insn.
> > > >> * config/aarch64/aarch64.cc (aarch64_evpc_ins): Look for both
> > > >> single-insert decompositions of the permutation and emit the new
> > > >> insn when both match.
> > > >>
> > > >> gcc/testsuite/ChangeLog:
> > > >>
> > > >> PR tree-optimization/123951
> > > >> * gcc.target/aarch64/pr123951_1.c: New test.
> > > >> * gcc.target/aarch64/pr123951_2.c: New test.
> > > >> * gcc.target/aarch64/simd/addsub_1.c: Update the e1 lane insert
> > > >> from d[1] to d[0].
> > > >> * gcc.target/aarch64/simd/addsub_2.c: Likewise.
> > > >>
> > > >> Signed-off-by: Rohith Kapelli <[email protected]>
> > > >> ---
> > > >> 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
> > > >>
> > > >> diff --git a/gcc/config/aarch64/aarch64-simd.md
> > > >> b/gcc/config/aarch64/aarch64-simd.md
> > > >> index ce51e24da36..a46629763c3 100644
> > > >> --- a/gcc/config/aarch64/aarch64-simd.md
> > > >> +++ b/gcc/config/aarch64/aarch64-simd.md
> > > >> @@ -1422,6 +1422,37 @@
> > > >> [(set_attr "type" "neon_ins<q>")]
> > > >> )
> > > >>
> > > >> +;; A two-element vec_merge takes one lane from each input, so it can
> be
> > > >> +;; carried out by inserting the live lane of either input into the
> > > >> other.
> > > >> +;; Offer both forms and leave the register allocator to pick the
> > > >> input to
> > > >> +;; tie to the destination, so that no extra move is needed.
> > > >> +(define_insn "@aarch64_simd_vec_merge<mode>"
> > > >> + [(set (match_operand:VP_2E 0 "register_operand" "=w,w")
> > > >> + (vec_merge:VP_2E
> > > >> + (match_operand:VP_2E 1 "register_operand" "w,0")
> > > >> + (match_operand:VP_2E 2 "register_operand" "0,w")
> > > >> + (match_operand:SI 3 "immediate_operand" "i,i")))]
> > > >> + "TARGET_SIMD
> > > >> + && (INTVAL (operands[3]) == 1 || INTVAL (operands[3]) == 2)"
> > > >> + {
> > > >> + int elt = INTVAL (operands[3]) == 1 ? 0 : 1;
> > > >> + if (which_alternative == 0)
> > > >> + {
> > > >> + /* The destination already holds the value of operand 2; insert
> > > >> + the live lane of operand 1. */
> > > >> + operands[3] = GEN_INT (ENDIAN_LANE_N (<nunits>, elt));
> > > >> + return "ins\t%0.<Vetype>[%3], %1.<Vetype>[%3]";
> > > >> + }
> > > >> + else
> > > >> + {
> > > >> + /* Conversely, insert the live lane of operand 2. */
> > > >> + operands[3] = GEN_INT (ENDIAN_LANE_N (<nunits>, 1 - elt));
> > > >> + return "ins\t%0.<Vetype>[%3], %2.<Vetype>[%3]";
> > > >> + }
> > > >> + }
> > > >> + [(set_attr "type" "neon_ins<q>")]
> > > >> +)
> > > >
> > > > This pattern then becomes nearly identical to the one I suggested in the
> > > cover letter
> > > > but with
> > > >
> > > > (define_insn "*aarch64_simd_vec_copy_lane_same<mode>_subreg"
> > > > [(set (match_operand:VP_2E 0 "register_operand" "=w,w")
> > > > (vec_merge:VP_2E
> > > > (vec_duplicate:VP_2E
> > > > (match_operand:<VEL> 2 "register_operand" "w,0"))
> > > > (match_operand:VP_2E 1 "register_operand" "0,w")
> > > > (match_operand:SI 3 "immediate_operand" "i,i")))]
> > > > "TARGET_SIMD
> > > > && INTVAL (operands[3]) == 2
> > > > && SUBREG_P (operands[2])
> > > > && known_eq (SUBREG_BYTE (operands[2]), GET_MODE_SIZE
> > > (<VEL>mode))"
> > > > {
> > > >
> > > > This catches the subreg case where the vec_select is folded to a subreg
> > > > by
> > > > Simplify RTX. Before reload it'll be a subreg but after reload the
> > > > subreg
> gets
> > > > removed and replaced by the hardreg.
> > >
> > > Yeah, but that's what I think makes the above problematic. The condition
> > > should pass even after RA. It's a major blind spot that we don't have
> > > RTL checkers to verify things like that.
> > >
> > > Also, any subreg used in place of vec_select will be a lowpart subreg,
> > > so if we do still end up testing for subregs, I think it should be based
> > > on that. The condition above doesn't look correct, since any
> > > little-endian
> > > subreg would have a SUBREG_BYTE of 0.
> >
> > Yes, I'm aware of that. I'm also aware that we don't actually recheck the
> > condition
> > after reload. Once the pattern is selected it stays valid. So since the
> > RTL
> shape
> > didn't
> > change it still matches.
> >
> > This is also why I mentioned because the byte offset isn't lane offset it
> already
> > account
> > for the correct location, so the lanes don't have to be endian swapped.
> >
> > So yes. I am aware of this.
>
> And to follow up, that concern is easily addressed with
>
> || (reload_completed && REG_P (operands[2])) added to the condition.
>
> Then the condition and the RTL match. So this is still perfectly fine.
Since Richard had an objection, he's an alternate approach. These operations
are essentially a vec_concat rather than a vec_merge. The issues can be avoided
if we actually treat them as such.
For LE the first pattern stays the same. We also add these
(define_insn "*aarch64_vec_concat_lane<mode>"
[(set (match_operand:VP_2E 0 "register_operand" "=w,w")
(vec_concat:VP_2E
(vec_select:<VEL>
(match_operand:VP_2E 1 "register_operand" "0,w")
(parallel
[(match_operand:SI 3 "immediate_operand" "i,i")]))
(vec_select:<VEL>
(match_operand:VP_2E 2 "register_operand" "w,0")
(parallel
[(match_operand:SI 4 "immediate_operand" "i,i")]))))]
"TARGET_SIMD
&& INTVAL (operands[3]) == 0
&& INTVAL (operands[4]) == 1"
{
...
Which essentially is saying concat two vectors together from a vec_select.
This then becomes your INS.
Then for BE you rewrite the vec_merge into the vec_concat using e.g.
(define_insn_and_split "*aarch64_simd_vec_copy_lane_same<mode>_be_lowpart"
[(set (match_operand:VP_2E 0 "register_operand" "=w,w")
(vec_merge:VP_2E
(vec_duplicate:VP_2E
(match_operand:<VEL> 2 "register_operand" "w,0"))
(match_operand:VP_2E 1 "register_operand" "0,w")
(match_operand:SI 3 "immediate_operand" "i,i")))]
"TARGET_SIMD
&& BYTES_BIG_ENDIAN
&& !reload_completed
&& INTVAL (operands[3]) == 2
&& SUBREG_P (operands[2])
&& subreg_lowpart_p (operands[2])"
"#"
"&& true"
...
This changes the codegen to target
(vec_concat:V2DF
(vec_select:DF (reg:V2DF x) [(const_int 0)])
(vec_select:DF (reg:V2DF y) [(const_int 1)]))
Which represents the INS without needing the subreg lanes and the subreg
goes away way before reload.
Hopefully Richard is happy with this one.
Tamar
>
> Thanks,
> Tamar
>
> >
> > Thanks,
> > Tamar
> >
> > >
> > > Thanks,
> > > Richard
