> Pengxuan Zheng <quic_pzh...@quicinc.com> writes:
> > This patch optimizes certain vector permute expansion with the FMOV
> > instruction when one of the input vectors is a vector of all zeros and
> > the result of the vector permute is as if the upper lane of the
> > non-zero input vector is set to zero and the lower lane remains
unchanged.
> >
> > Note that the patch also propagates zero_op0_p and zero_op1_p during
> > re-encode now. They will be used by aarch64_evpc_fmov to check if the
> > input vectors are valid candidates.
> >
> > PR target/100165
> >
> > gcc/ChangeLog:
> >
> > * config/aarch64/aarch64-simd.md
> (aarch64_simd_vec_set_zero_fmov<mode>):
> > New define_insn.
> > * config/aarch64/aarch64.cc (aarch64_evpc_reencode): Copy
> zero_op0_p and
> > zero_op1_p.
> > (aarch64_evpc_fmov): New function.
> > (aarch64_expand_vec_perm_const_1): Add call to
> aarch64_evpc_fmov.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/aarch64/vec-set-zero.c: Update test accordingly.
> > * gcc.target/aarch64/fmov.c: New test.
> > * gcc.target/aarch64/fmov-be.c: New test.
>
> Nice! Thanks for doing this. Some comments on the patch below.
> >
> > Signed-off-by: Pengxuan Zheng <quic_pzh...@quicinc.com>
> > ---
> > gcc/config/aarch64/aarch64-simd.md | 14 +++
> > gcc/config/aarch64/aarch64.cc | 74 +++++++++++-
> > gcc/testsuite/gcc.target/aarch64/fmov-be.c | 74 ++++++++++++
> > gcc/testsuite/gcc.target/aarch64/fmov.c | 110 ++++++++++++++++++
> > .../gcc.target/aarch64/vec-set-zero.c | 6 +-
> > 5 files changed, 275 insertions(+), 3 deletions(-) create mode
> > 100644 gcc/testsuite/gcc.target/aarch64/fmov-be.c
> > create mode 100644 gcc/testsuite/gcc.target/aarch64/fmov.c
> >
> > diff --git a/gcc/config/aarch64/aarch64-simd.md
> > b/gcc/config/aarch64/aarch64-simd.md
> > index e456f693d2f..543126948e7 100644
> > --- a/gcc/config/aarch64/aarch64-simd.md
> > +++ b/gcc/config/aarch64/aarch64-simd.md
> > @@ -1190,6 +1190,20 @@ (define_insn "aarch64_simd_vec_set<mode>"
> > [(set_attr "type" "neon_ins<q>, neon_from_gp<q>,
> > neon_load1_one_lane<q>")]
> > )
> >
> > +(define_insn "aarch64_simd_vec_set_zero_fmov<mode>"
> > + [(set (match_operand:VP_2E 0 "register_operand" "=w")
> > + (vec_merge:VP_2E
> > + (match_operand:VP_2E 1 "aarch64_simd_imm_zero" "Dz")
> > + (match_operand:VP_2E 3 "register_operand" "w")
> > + (match_operand:SI 2 "immediate_operand" "i")))]
> > + "TARGET_SIMD
> > + && (ENDIAN_LANE_N (<nunits>, exact_log2 (INTVAL (operands[2]))) ==
> 1)"
> > + {
> > + return "fmov\\t%<Vetype>0, %<Vetype>3";
> > + }
> > + [(set_attr "type" "fmov")]
> > +)
> > +
>
> I think this shows that target-independent code is missing some
> canonicalisation of vec_merge. combine has:
>
> unsigned n_elts = 0;
> if (GET_CODE (x) == VEC_MERGE
> && CONST_INT_P (XEXP (x, 2))
> && GET_MODE_NUNITS (GET_MODE (x)).is_constant (&n_elts)
> && (swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1))
> /* Two operands have same precedence, then
> first bit of mask select first operand. */
> || (!swap_commutative_operands_p (XEXP (x, 1), XEXP (x, 0))
> && !(UINTVAL (XEXP (x, 2)) & 1))))
> {
> rtx temp = XEXP (x, 0);
> unsigned HOST_WIDE_INT sel = UINTVAL (XEXP (x, 2));
> unsigned HOST_WIDE_INT mask = HOST_WIDE_INT_1U;
> if (n_elts == HOST_BITS_PER_WIDE_INT)
> mask = -1;
> else
> mask = (HOST_WIDE_INT_1U << n_elts) - 1;
> SUBST (XEXP (x, 0), XEXP (x, 1));
> SUBST (XEXP (x, 1), temp);
> SUBST (XEXP (x, 2), GEN_INT (~sel & mask));
> }
>
> which AFAICT would prefer to put the immediate second, not first. I think
we
> should be doing the same canonicalisation in simplify_ternary_operation,
and
> possibly elsewhere, so that the .md pattern only needs to match the
canonical
> form (i.e. register, immedate, mask).
Thanks for the suggestion. I've added the canonicalization in a separate
patch.
https://gcc.gnu.org/pipermail/gcc-patches/2025-February/676105.html
>
> On:
>
> > + && (ENDIAN_LANE_N (<nunits>, exact_log2 (INTVAL (operands[2]))) ==
> 1)"
>
> it seems dangerous to pass exact_log2 to ENDIAN_LANE_N when we haven't
> checked whether it is a power of 2. (0b00 or 0b11 ought to get
simplified, but
> I don't think we can ignore the possibility.)
>
> Rather than restrict the pattern to pairs, could we instead handle
> VALL_F16 minus the QI elements, with the 16-bit elements restricted to
> TARGET_F16? E.g. we should be able to handle V4SI using an FMOV of S
> registers if only the low element is nonzero.
Good point! I've addressed these in the latest version. Please let me know
if I missed anything.
https://gcc.gnu.org/pipermail/gcc-patches/2025-February/676106.html
Thanks,
Pengxuan
>
> Part of me thinks that this should just be described as a plain old AND,
but I
> suppose that doesn't work well for FP modes. Still, handling ANDs might
be
> an interesting follow-up :)
>
> Thanks,
> Richard
>
> > (define_insn "aarch64_simd_vec_set_zero<mode>"
> > [(set (match_operand:VALL_F16 0 "register_operand" "=w")
> > (vec_merge:VALL_F16
> > diff --git a/gcc/config/aarch64/aarch64.cc
> > b/gcc/config/aarch64/aarch64.cc index a6cc00e74ab..64756920eda
> 100644
> > --- a/gcc/config/aarch64/aarch64.cc
> > +++ b/gcc/config/aarch64/aarch64.cc
> > @@ -25950,6 +25950,8 @@ aarch64_evpc_reencode (struct
> expand_vec_perm_d *d)
> > newd.target = d->target ? gen_lowpart (new_mode, d->target) : NULL;
> > newd.op0 = d->op0 ? gen_lowpart (new_mode, d->op0) : NULL;
> > newd.op1 = d->op1 ? gen_lowpart (new_mode, d->op1) : NULL;
> > + newd.zero_op0_p = d->zero_op0_p;
> > + newd.zero_op1_p = d->zero_op1_p;
> > newd.testing_p = d->testing_p;
> > newd.one_vector_p = d->one_vector_p;
> >
> > @@ -26434,6 +26436,74 @@ aarch64_evpc_ins (struct
> expand_vec_perm_d *d)
> > return true;
> > }
> >
> > +/* Recognize patterns suitable for the FMOV instructions. */ static
> > +bool aarch64_evpc_fmov (struct expand_vec_perm_d *d) {
> > + machine_mode mode = d->vmode;
> > + unsigned HOST_WIDE_INT nelt;
> > +
> > + if (d->vec_flags != VEC_ADVSIMD)
> > + return false;
> > +
> > + /* to_constant is safe since this routine is specific to Advanced
SIMD
> > + vectors. */
> > + nelt = d->perm.length ().to_constant ();
> > +
> > + /* Either d->op0 or d->op1 should be a vector of all zeros. */ if
> > + (nelt != 2 || d->one_vector_p || (!d->zero_op0_p && !d->zero_op1_p))
> > + return false;
> > +
> > + HOST_WIDE_INT elt0, elt1;
> > + rtx in0 = d->op0;
> > + rtx in1 = d->op1;
> > +
> > + if (!d->perm[0].is_constant (&elt0))
> > + return false;
> > +
> > + if (!d->perm[1].is_constant (&elt1))
> > + return false;
> > +
> > + if (!BYTES_BIG_ENDIAN)
> > + {
> > + /* Lane 0 of the output vector should come from lane 0 of the
non-zero
> > + vector. */
> > + if (elt0 != (d->zero_op0_p ? 2 : 0))
> > + return false;
> > +
> > + /* Lane 1 of the output vector should come from any lane of the
zero
> > + vector. */
> > + if (elt1 != (d->zero_op0_p ? 0 : 2) && elt1 != (d->zero_op0_p ? 1
: 3))
> > + return false;
> > + }
> > + else
> > + {
> > + /* Lane 0 of the output vector should come from any lane of the
zero
> > + vector. */
> > + if (elt0 != (d->zero_op0_p ? 0 : 2) && elt0 != (d->zero_op0_p ? 1
: 3))
> > + return false;
> > +
> > + /* Lane 1 of the output vector should come from lane 1 of the
non-zero
> > + vector. */
> > + if (elt1 != (d->zero_op0_p ? 3 : 1))
> > + return false;
> > + }
> > +
> > + if (d->testing_p)
> > + return true;
> > +
> > + insn_code icode = code_for_aarch64_simd_vec_copy_lane (mode);
> > + expand_operand ops[5]; create_output_operand (&ops[0], d->target,
> > + mode); create_input_operand (&ops[1], d->zero_op0_p ? in1 : in0,
> > + mode); create_integer_operand (&ops[2], BYTES_BIG_ENDIAN ? 1 : 2);
> > + create_input_operand (&ops[3], d->zero_op0_p ? in0 : in1, mode);
> > + create_integer_operand (&ops[4], 0); expand_insn (icode, 5, ops);
> > +
> > + return true;
> > +}
> > +
> > static bool
> > aarch64_expand_vec_perm_const_1 (struct expand_vec_perm_d *d) { @@
> > -26457,7 +26527,9 @@ aarch64_expand_vec_perm_const_1 (struct
> expand_vec_perm_d *d)
> > {
> > if (d->vmode == d->op_mode)
> > {
> > - if (aarch64_evpc_rev_local (d))
> > + if (aarch64_evpc_fmov (d))
> > + return true;
> > + else if (aarch64_evpc_rev_local (d))
> > return true;
> > else if (aarch64_evpc_rev_global (d))
> > return true;
> > diff --git a/gcc/testsuite/gcc.target/aarch64/fmov-be.c
> > b/gcc/testsuite/gcc.target/aarch64/fmov-be.c
> > new file mode 100644
> > index 00000000000..f864d474a7a
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/fmov-be.c
> > @@ -0,0 +1,74 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -mbig-endian" } */
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +typedef int v2si __attribute__ ((vector_size (8))); typedef long v2di
> > +__attribute__ ((vector_size (16))); typedef int v4si __attribute__
> > +((vector_size (16))); typedef float v4sf __attribute__ ((vector_size
> > +(16)));
> > +
> > +/*
> > +** f_v2si:
> > +** fmov s0, s0
> > +** ret
> > +*/
> > +v2si
> > +f_v2si (v2si x)
> > +{
> > + return __builtin_shuffle (x, (v2si){ 0, 0 }, (v2si){ 3, 1 }); }
> > +
> > +/*
> > +** g_v2si:
> > +** fmov s0, s0
> > +** ret
> > +*/
> > +v2si
> > +g_v2si (v2si x)
> > +{
> > + return __builtin_shuffle ((v2si){ 0, 0 }, x, (v2si){ 0, 3 }); }
> > +
> > +/*
> > +** f_v2di:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v2di
> > +f_v2di (v2di x)
> > +{
> > + return __builtin_shuffle (x, (v2di){ 0, 0 }, (v2di){ 2, 1 }); }
> > +
> > +/*
> > +** g_v2di:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v2di
> > +g_v2di (v2di x)
> > +{
> > + return __builtin_shuffle ((v2di){ 0, 0 }, x, (v2di){ 0, 3 }); }
> > +
> > +/*
> > +** f_v4si:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v4si
> > +f_v4si (v4si x)
> > +{
> > + return __builtin_shuffle (x, (v4si){ 0, 0, 0, 0 }, (v4si){ 6, 7, 2,
> > +3 }); }
> > +
> > +/*
> > +** g_v4si:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v4si
> > +g_v4si (v4si x)
> > +{
> > + return __builtin_shuffle ((v4si){ 0, 0, 0, 0 }, x, (v4si){ 2, 3, 6,
> > +7 }); }
> > diff --git a/gcc/testsuite/gcc.target/aarch64/fmov.c
> > b/gcc/testsuite/gcc.target/aarch64/fmov.c
> > new file mode 100644
> > index 00000000000..f40ec56c5f8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/fmov.c
> > @@ -0,0 +1,110 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-final { check-function-bodies "**" "" "" } } */
> > +
> > +typedef int v2si __attribute__ ((vector_size (8))); typedef float
> > +v2sf __attribute__ ((vector_size (8))); typedef long v2di
> > +__attribute__ ((vector_size (16))); typedef double v2df __attribute__
> > +((vector_size (16))); typedef int v4si __attribute__ ((vector_size
> > +(16))); typedef float v4sf __attribute__ ((vector_size (16)));
> > +
> > +
> > +/*
> > +** f_v2si:
> > +** fmov s0, s0
> > +** ret
> > +*/
> > +v2si
> > +f_v2si (v2si x)
> > +{
> > + return __builtin_shuffle (x, (v2si){ 0, 0 }, (v2si){ 0, 3 }); }
> > +
> > +/*
> > +** g_v2si:
> > +** fmov s0, s0
> > +** ret
> > +*/
> > +v2si
> > +g_v2si (v2si x)
> > +{
> > + return __builtin_shuffle ((v2si){ 0, 0 }, x, (v2si){ 2, 0 }); }
> > +
> > +/*
> > +** f_v2sf:
> > +** fmov s0, s0
> > +** ret
> > +*/
> > +v2sf
> > +f_v2sf (v2sf x)
> > +{
> > + return __builtin_shuffle (x, (v2sf){ 0, 0 }, (v2si){ 0, 2 }); }
> > +
> > +/*
> > +** f_v2di:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v2di
> > +f_v2di (v2di x)
> > +{
> > + return __builtin_shuffle (x, (v2di){ 0, 0 }, (v2di){ 0, 3 }); }
> > +
> > +/*
> > +** g_v2di:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v2di
> > +g_v2di (v2di x)
> > +{
> > + return __builtin_shuffle ((v2di){ 0, 0 }, x, (v2di){ 2, 1 }); }
> > +
> > +/*
> > +** f_v2df:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v2df
> > +f_v2df (v2df x)
> > +{
> > + return __builtin_shuffle (x, (v2df){ 0, 0 }, (v2di){ 0, 2 }); }
> > +
> > +/*
> > +** f_v4si:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v4si
> > +f_v4si (v4si x)
> > +{
> > + return __builtin_shuffle (x, (v4si){ 0, 0, 0, 0 }, (v4si){ 0, 1, 4,
> > +5 }); }
> > +
> > +/*
> > +** g_v4si:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v4si
> > +g_v4si (v4si x)
> > +{
> > + return __builtin_shuffle ((v4si){ 0, 0, 0, 0 }, x, (v4si){ 4, 5, 2,
> > +3 }); }
> > +
> > +/*
> > +** f_v4sf:
> > +** fmov d0, d0
> > +** ret
> > +*/
> > +v4sf
> > +f_v4sf (v4sf x)
> > +{
> > + return __builtin_shuffle (x, (v4sf){ 0, 0, 0, 0 }, (v4si){ 0, 1, 6,
> > +7 }); }
> > diff --git a/gcc/testsuite/gcc.target/aarch64/vec-set-zero.c
> > b/gcc/testsuite/gcc.target/aarch64/vec-set-zero.c
> > index b34b902cf27..9040839931f 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/vec-set-zero.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/vec-set-zero.c
> > @@ -28,8 +28,10 @@ FOO(float64x2_t)
> >
> > /* { dg-final { scan-assembler-times {ins\tv[0-9]+\.b\[1\], wzr} 2 {
> > target aarch64_little_endian } } } */
> > /* { dg-final { scan-assembler-times {ins\tv[0-9]+\.h\[1\], wzr} 4 {
> > target aarch64_little_endian } } } */
> > -/* { dg-final { scan-assembler-times {ins\tv[0-9]+\.s\[1\], wzr} 4 {
> > target aarch64_little_endian } } } */
> > -/* { dg-final { scan-assembler-times {ins\tv[0-9]+\.d\[1\], xzr} 2 {
> > target aarch64_little_endian } } } */
> > +/* { dg-final { scan-assembler-times {ins\tv[0-9]+\.s\[1\], wzr} 2 {
> > +target aarch64_little_endian } } } */
> > +/* { dg-final { scan-assembler-times {ins\tv[0-9]+\.d\[1\], xzr} 0 {
> > +target aarch64_little_endian } } } */
> > +/* { dg-final { scan-assembler-times {fmov\ts0, s0} 2 { target
> > +aarch64_little_endian } } } */
> > +/* { dg-final { scan-assembler-times {fmov\td0, d0} 2 { target
> > +aarch64_little_endian } } } */
> >
> > /* { dg-final { scan-assembler-times {ins\tv[0-9]+\.b\[6\], wzr} 1 {
> > target aarch64_big_endian } } } */
> > /* { dg-final { scan-assembler-times {ins\tv[0-9]+\.b\[14\], wzr} 1 {
> > target aarch64_big_endian } } } */
