RE: [PATCH v3 1/1] aarch64: Avoid extra move for two-element vector lane copies [PR123951]

> -----Original Message-----
> From: Rohith Kapelli <[email protected]>
> Sent: 07 August 2026 19:34
> To: [email protected]
> Cc: Richard Sandiford <[email protected]>; Tamar Christina
> <[email protected]>; Kyrylo Tkachov <[email protected]>;
> Richard Earnshaw <[email protected]>
> Subject: [PATCH v3 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 to a VEC_PERM_EXPR, so the constant permute
> expander decides what to emit for it.  For a two-element vector the
> result takes one lane from each input, but the patterns that matched it
> tied the result to one particular input, and for half the lane
> combinations that is the wrong one, so the register allocator has to add
> a 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 to two and vect_copy_lane_1.c and vget_set_lane_1.c fail;
> little endian has the same problem on lane 0.
> 
> Add two patterns for the two-element modes, each offering a second
> alternative that ties the destination to the other input, so that the
> register allocator can insert into whichever input already occupies the
> destination.  The first handles a lane copy whose source and destination
> lanes are the same; the second handles a lane insert from a scalar,
> which always sits in the low part of its register, so it is restricted
> to the case where the destination is architectural lane 0.  Both sit
> before the corresponding general patterns, which continue to handle
> every other case, including cross-lane copies.
> 
> 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.
> The double add/sub merge in addsub_{1,2}.c uses the same two-element
> merge; with the tie left to the allocator its e1 case now inserts into
> lane 0 rather than lane 1, still a single INS, so the expected output is
> updated.
> 
> 	PR tree-optimization/123951
> 
> gcc/ChangeLog:
> 
> 	* config/aarch64/aarch64-simd.md
> 	(*aarch64_simd_vec_set_lane0<mode>): New pattern.
> 	(*aarch64_simd_vec_copy_lane_same<mode>): New pattern.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/aarch64/pr123951_1.c: New test.
> 	* gcc.target/aarch64/pr123951_2.c: New test.
> 	* gcc.target/aarch64/simd/addsub_1.c: Update e1 lane insert from
> 	d[1] to d[0].
> 	* gcc.target/aarch64/simd/addsub_2.c: Likewise.
> 
> Signed-off-by: Rohith Kapelli <[email protected]>
> ---
> 
> > It looks like both patterns could use the "@..." syntax to specify the
> > alternatives, rather than switching on which_alternative.  There isn't
> > the ENDIAN_LANE_N stuff that makes some of the other patterns require
> > C++ code.
> >
> > The patterns could then use the new alternative syntax, where the
> > constraints and attributes are on the same line as the asm.
> 
> Done, both of them.  You're right that nothing here needs the C++: the
> lane numbers in the templates are constants, and the only ENDIAN_LANE_N
> use is in the insn condition, which is unchanged.  v3 converts both
> patterns to the "@" form with the constraints and attributes alongside
> each alternative, which also lets the per-operand constraint strings and
> the separate set_attr blocks go.  54 added lines rather than 68.
> 
> One thing worth flagging: the cons: list names operands 0, 1 and 3, so
> the two immediates lose their "i".  That is deliberate.  Comparing the
> genoutput data for v2 and v3, every register and memory constraint is
> byte-identical and only the two immediates change to "", which is what
> *cmov<mode>_insn already does for the operands its own cons: list skips.
> Their predicate is immediate_operand, so "i" was not adding anything.
> 
> The generated code is unchanged.  I compared the assembly out of the v2
> and v3 compilers for the two new tests and for a file covering every
> VP_2E mode (s64/u64/f64 and s32/u32/f32, both lane pairs and both
> directions) plus the vec_set paths (scalar from an FP register, from a
> general register and from memory), on aarch64-linux-gnu and
> aarch64_be-linux-gnu: byte-identical in all six comparisons.
> 
> Bootstrapped and regression tested on aarch64-unknown-linux-gnu, patched
> and unpatched built from clean trees in the same environment, both
> stage2 == stage3.  Comparing every result line across gcc, g++,
> libstdc++, libgomp, libitm and libatomic, the two runs are identical
> except for 11 new PASSes, which are the two new tests.  No PASS->FAIL.
> 
> addsub_{1,2}.c do still need the expectation change: with the patch, e1
> comes out as ins v.d[0], v.d[0], so the updated files pass and the
> d[1] versions fail on that one function and nothing else.
> 
> Also tested aarch64.exp and simd/simd.exp as crosses on both
> endiannesses, v2 against v3: all four .sum files identical.
> 
> > Otherwise this LGTM, but I'm not sure that I should approve.
> 
> Thanks.  Tamar, since the two-pattern shape is your suggestion, would you
> be happy to confirm the approval?
> 

OK.

Thanks,
Tamar

> Thanks,
> Rohith
> 
>  gcc/config/aarch64/aarch64-simd.md            | 54 +++++++++++++++++++
>  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 +-
>  5 files changed, 132 insertions(+), 2 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..80d8474b474 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -1330,6 +1330,32 @@
>    [(set_attr "type" "neon_logic<q>")]
>  )
> 
> +;; Lane insert for the two-element modes.  A scalar source always occupies
> +;; the low part of its register, architectural lane 0.  When the inserted
> +;; element is that same lane, the destination can equally be tied to the
> +;; source register and the other lane brought in from operand 3, so offer
> +;; that as a second alternative and let the register allocator pick
> +;; whichever input already occupies the destination.  The remaining
> +;; alternatives are those of the general pattern below, so that a scalar
> +;; from a general register or from memory is unaffected.
> +(define_insn "*aarch64_simd_vec_set_lane0<mode>"
> +  [(set (match_operand:VP_2E 0 "register_operand")
> +	(vec_merge:VP_2E
> +	    (vec_duplicate:VP_2E
> +	      (match_operand:<VEL> 1
> "aarch64_simd_nonimmediate_operand"))
> +	    (match_operand:VP_2E 3 "register_operand")
> +	    (match_operand:SI 2 "immediate_operand")))]
> +  "TARGET_SIMD && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 2 : 1)"
> +  ;; In the second alternative the destination is the scalar's own register,
> +  ;; which already holds it in lane 0, so the other lane comes from operand 3.
> +  {@ [ cons: =0 , 1   , 3 ; attrs: type            ]
> +     [ w        , w   , 0 ; neon_ins<q>            ] ins\t%0.<Vetype>[0], %1.<Vetype>[0]
> +     [ w        , 0   , w ; neon_ins<q>            ] ins\t%0.<Vetype>[1], %3.<Vetype>[1]
> +     [ w        , ?r  , 0 ; neon_from_gp<q>        ] ins\t%0.<Vetype>[0], %<vwcore>1
> +     [ w        , Utv , 0 ; neon_load1_one_lane<q> ] ld1\t{%0.<Vetype>}[0], %1
> +  }
> +)
> +
>  (define_insn "@aarch64_simd_vec_set<mode>"
>    [(set (match_operand:VALL_F16 0 "register_operand" "=w,w,w")
>  	(vec_merge:VALL_F16
> @@ -1401,6 +1427,34 @@
>    }
>  )
> 
> +;; Lane copy between two two-element vectors.  When the source and
> +;; destination lanes are the same, the copy reads one lane from each input,
> +;; so it can equally be done by inserting the live lane of either input into
> +;; the other.  Offer both directions and let the register allocator tie
> +;; whichever input already occupies the destination, so that neither lane
> +;; needs an extra move.  Other lane combinations are left to the general
> +;; pattern below, where only the destination can be tied.
> +(define_insn "*aarch64_simd_vec_copy_lane_same<mode>"
> +  [(set (match_operand:VP_2E 0 "register_operand")
> +	(vec_merge:VP_2E
> +	    (vec_duplicate:VP_2E
> +	      (vec_select:<VEL>
> +		(match_operand:VP_2E 3 "register_operand")
> +		(parallel
> +		  [(match_operand:SI 4 "immediate_operand")])))
> +	    (match_operand:VP_2E 1 "register_operand")
> +	    (match_operand:SI 2 "immediate_operand")))]
> +  "TARGET_SIMD
> +   && ENDIAN_LANE_N (2, INTVAL (operands[4])) == 1
> +   && INTVAL (operands[2]) == (BYTES_BIG_ENDIAN ? 1 : 2)"
> +  ;; In the second alternative the destination is operand 3, which already
> +  ;; holds lane 1, so lane 0 comes from operand 1.
> +  {@ [ cons: =0 , 1 , 3 ; attrs: type ]
> +     [ w        , 0 , w ; neon_ins<q> ] ins\t%0.<Vetype>[1], %3.<Vetype>[1]
> +     [ w        , w , 0 ; neon_ins<q> ] ins\t%0.<Vetype>[0], %1.<Vetype>[0]
> +  }
> +)
> +
>  (define_insn "@aarch64_simd_vec_copy_lane<mode>"
>    [(set (match_operand:VALL_F16 0 "register_operand" "=w")
>  	(vec_merge:VALL_F16
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr123951_1.c
> b/gcc/testsuite/gcc.target/aarch64/pr123951_1.c
> new file mode 100644
> index 00000000000..3c195505a89
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr123951_1.c
> @@ -0,0 +1,41 @@
> +/* PR tree-optimization/123951.  Copying a lane between two vectors must
> +   remain a single INS (or ZIP) whichever lane pair is used, on both
> +   endiannesses.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +#include <arm_neon.h>
> +
> +#define BUILD_TEST(TYPE, Q1, Q2, SUFFIX, INDEX1, INDEX2)		\
> +TYPE __attribute__((noinline,noclone))
> 	\
> +test_copy##Q1##_lane##Q2##_##SUFFIX##_##INDEX1##INDEX2 (TYPE a,
> TYPE b) \
> +{									\
> +  return vcopy##Q1##_lane##Q2##_##SUFFIX (a, INDEX1, b, INDEX2);	\
> +}
> +
> +BUILD_TEST (uint64x2_t,  q, q, u64, 0, 0)
> +BUILD_TEST (int64x2_t,   q, q, s64, 0, 0)
> +BUILD_TEST (float64x2_t, q, q, f64, 0, 0)
> +/* { dg-final { scan-assembler-times "ins\\tv0.d\\\[0\\\], v1.d\\\[0\\\]" 3 } }
> */
> +BUILD_TEST (uint64x2_t,  q, q, u64, 1, 1)
> +BUILD_TEST (int64x2_t,   q, q, s64, 1, 1)
> +BUILD_TEST (float64x2_t, q, q, f64, 1, 1)
> +/* { dg-final { scan-assembler-times "ins\\tv0.d\\\[1\\\], v1.d\\\[1\\\]" 3 } }
> */
> +BUILD_TEST (uint64x2_t,  q, q, u64, 1, 0)
> +BUILD_TEST (int64x2_t,   q, q, s64, 1, 0)
> +BUILD_TEST (float64x2_t, q, q, f64, 1, 0)
> +/* { dg-final { scan-assembler-times "zip1\\tv0.2d, v0.2d, v1.2d" 3 } } */
> +BUILD_TEST (uint64x2_t,  q, q, u64, 0, 1)
> +BUILD_TEST (int64x2_t,   q, q, s64, 0, 1)
> +BUILD_TEST (float64x2_t, q, q, f64, 0, 1)
> +/* { dg-final { scan-assembler-times "zip2\\tv0.2d, v1.2d, v0.2d" 3 } } */
> +BUILD_TEST (uint32x2_t,  , , u32, 0, 0)
> +BUILD_TEST (int32x2_t,   , , s32, 0, 0)
> +BUILD_TEST (float32x2_t, , , f32, 0, 0)
> +/* { dg-final { scan-assembler-times "ins\\tv0.s\\\[0\\\], v1.s\\\[0\\\]" 3 } }
> */
> +BUILD_TEST (uint32x2_t,  , , u32, 1, 1)
> +BUILD_TEST (int32x2_t,   , , s32, 1, 1)
> +BUILD_TEST (float32x2_t, , , f32, 1, 1)
> +/* { dg-final { scan-assembler-times "ins\\tv0.s\\\[1\\\], v1.s\\\[1\\\]" 3 } }
> */
> +
> +/* { dg-final { scan-assembler-not "\\tmov\\t" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr123951_2.c
> b/gcc/testsuite/gcc.target/aarch64/pr123951_2.c
> new file mode 100644
> index 00000000000..59770b6b000
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr123951_2.c
> @@ -0,0 +1,35 @@
> +/* PR tree-optimization/123951.  Like pr123951_1.c, but for generic vector
> +   shuffles, including ones that only become lane inserts after being
> +   re-encoded to a wider element mode.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +typedef unsigned long long v2di __attribute__((vector_size (16)));
> +typedef unsigned int v4si __attribute__((vector_size (16)));
> +
> +v2di
> +shuffle_03 (v2di a, v2di b)
> +{
> +  return __builtin_shuffle (a, b, (v2di) { 0, 3 });
> +}
> +
> +v2di
> +shuffle_21 (v2di a, v2di b)
> +{
> +  return __builtin_shuffle (a, b, (v2di) { 2, 1 });
> +}
> +
> +v4si
> +shuffle_0167 (v4si a, v4si b)
> +{
> +  return __builtin_shuffle (a, b, (v4si) { 0, 1, 6, 7 });
> +}
> +
> +v4si
> +shuffle_4523 (v4si a, v4si b)
> +{
> +  return __builtin_shuffle (a, b, (v4si) { 4, 5, 2, 3 });
> +}
> +
> +/* { dg-final { scan-assembler-times "\\tins\\t" 4 } } */
> +/* { dg-final { scan-assembler-not "\\tmov\\t" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_1.c
> b/gcc/testsuite/gcc.target/aarch64/simd/addsub_1.c
> index 1fb91a34c42..5acfe33a576 100644
> --- a/gcc/testsuite/gcc.target/aarch64/simd/addsub_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_1.c
> @@ -43,7 +43,7 @@ void d1 (_Float16 *restrict a, _Float16 *restrict b,
> _Float16 *res, int n)
>  ** ...
>  ** 	fadd	v[0-9]+.2d, v[0-9]+.2d, v[0-9]+.2d
>  ** 	fsub	v[0-9]+.2d, v[0-9]+.2d, v[0-9]+.2d
> -** 	ins	v[0-9]+.d\[1\], v[0-9]+.d\[1\]
> +** 	ins	v[0-9]+.d\[0\], v[0-9]+.d\[0\]
>  ** ...
>  */
>  void e1 (double *restrict a, double *restrict b, double *res, int n)
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> index 87424c94f24..023adb46ece 100644
> --- a/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/addsub_2.c
> @@ -43,7 +43,7 @@ void d1 (_Float16 *restrict a, _Float16 *restrict b,
> _Float16 *res, int n)
>  ** ...
>  ** 	fsub	v[0-9]+.2d, v[0-9]+.2d, v[0-9]+.2d
>  ** 	fadd	v[0-9]+.2d, v[0-9]+.2d, v[0-9]+.2d
> -** 	ins	v[0-9]+.d\[1\], v[0-9]+.d\[1\]
> +** 	ins	v[0-9]+.d\[0\], v[0-9]+.d\[0\]
>  ** ...
>  */
>  void e1 (double *restrict a, double *restrict b, double *res, int n)
> --
> 2.53.0