Hi Pengxuan,

The 08/26/2026 03:42, Pengxuan Zheng wrote:
> 
> 
> On Thu, Jul 30, 2026 at 8:53 AM Tamar Christina 
> <[email protected]<mailto:[email protected]>> wrote:
> Hi Pengxuan,
> 
> I’m very sorry for the delay here. This slipped through the cracks.
> 
> I don’t think this optimization is in general optimal as it depends whether 
> the shuffle is in a loop or not.
> When in a loop the literal load is pulled out of the loop so you only pay for 
> it once.
> 
> Within the loop this would change the permute for 2 to 4 cycles.
> 
> I think to do this the optimization has to be moved to RTL, where you can use 
> a helper similar to aarch64_split_simd_shift_p to check if the instruction 
> executes less often than the function entry point which
> gives us a decent indication if we’re in a loop or not.
> 
> In RTL you can detect the permute sequence similarly to vect_par_cnst_hi_half 
> so I think a split here makes more
> sense.
> 
> Thanks a lot for the review, Tamar!
> 
> I experimented with moving the optimization to RTL as you suggested.
> However, I haven't found a way to remove the constant pool entries created 
> for the TBL.
> 
> For example, here's the split1 dump after applying the optimization.
> 
> (note 1 0 4 NOTE_INSN_DELETED)
> (note 4 1 17 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
> (insn 17 4 2 2 (set (reg:V4SI 108 [ a ])
>         (reg:V4SI 32 v0 [ a ])) 
> "/local/mnt/workspace/pzheng/test/rev64/c.c":11:1 -1
>      (expr_list:REG_DEAD (reg:V4SI 32 v0 [ a ])
>         (nil)))
> (note 2 17 3 2 NOTE_INSN_DELETED)
> (note 3 2 9 2 NOTE_INSN_FUNCTION_BEG)
> (note 9 3 18 2 NOTE_INSN_DELETED)
> (insn 18 9 19 2 (set (reg:V4SI 109)
>         (unspec:V4SI [
>                 (reg:V4SI 108 [ a ])
>             ] UNSPEC_REV64)) 
> "/local/mnt/workspace/pzheng/test/rev64/c.c":13:1 -1
>      (nil))
> (insn 19 18 15 2 (set (reg:V16QI 32 v0)
>         (unspec:V16QI [
>                 (subreg:V16QI (reg:V4SI 109) 0)
>                 (subreg:V16QI (reg:V4SI 109) 0)
>                 (const_int 8 [0x8])
>             ] UNSPEC_EXT)) "/local/mnt/workspace/pzheng/test/rev64/c.c":13:1 
> -1
>      (nil))
> (insn 15 19 0 2 (use (reg/i:V4SI 32 v0)) 
> "/local/mnt/workspace/pzheng/test/rev64/c.c":13:1 -1
>      (nil))
> 
> However, the final assembly still shows the constant pool entries.
> 
>         .arch armv8-a
>         .file   "c.c"
>         .text
>         .align  2
>         .p2align 5,,15
>         .global f2
>         .type   f2, %function
> f2:
> .LFB0:
>         .cfi_startproc
>         rev64   v0.4s, v0.4s
>         ext     v0.16b, v0.16b, v0.16b, #8
>         ret
>         .cfi_endproc
> .LFE0:
>         .size   f2, .-f2
>         .section        .rodata
>         .align  4
>         .set    .LANCHOR0,. + 0
> .LC0:
>         .byte   12
>         .byte   13
>         .byte   14
>         .byte   15
>         .byte   8
>         .byte   9
>         .byte   10
>         .byte   11
>         .byte   4
>         .byte   5
>         .byte   6
>         .byte   7
>         .byte   0
>         .byte   1
>         .byte   2
>         .byte   3
>         .ident  "GCC: (GNU) 17.0.0 20260819 (experimental)"
>         .section        .note.GNU-stack,"",@progbits
> 
> Do you have any suggestions on how to get rid of the constants?
> 

Yeah, it looks like the problem is that the section anchors keep the literal 
entry live
since it seems GCC can't unregister it once all references are gone.  That's 
why -fno-section-anchors
works.

later on combine removes the unused memrefs but by then it's too late.

> Another alternative is to extend my current patch to add a check of 
> bb_loop_depth in aarch64_evpc_rev64_ext.
> This way, we can bail out if it's inside a loop. Any comment on this approach?
>

Yeah I think that's probably the best approach since there doesn't seem to be a 
way to unregister the constant
and the RTL pattern had the benefit of also handling TBL directly

i.e.

int8x16_t
h (int8x16_t a)
{
  return vqtbl1q_s8 (a, (uint8x16_t){ 15, 14, 13, 12,
                                     11, 10, 9, 8,
                                     7, 6, 5, 4,
                                     3, 2, 1, 0 });
}

which at the moment is an UNSPEC but I suppose once work is finished to move 
this to a VEC_PERM_EXPR it'll sort
itself out.

So I'm ok with checking if in a loop at expand time.

Thanks,
Tamar

> Thanks,
> Pengxuan
> 
> 
> Thanks,
> Tamar
> 
> From: Pengxuan Zheng 
> <[email protected]<mailto:[email protected]>>
> Sent: 13 May 2026 21:45
> To: [email protected]<mailto:[email protected]>
> Subject: Re: [PATCH] aarch64: Recognize vector permute patterns which can be 
> optimized as REV64+EXT [PR102055]
> 
> Gentle ping
> 
> On Fri, Apr 17, 2026 at 11:40 AM Pengxuan Zheng 
> <[email protected]<mailto:[email protected]>> 
> wrote:
> Currently, with Advanced SIMD
> 
> vector char
> f (vector char a)
> {
>   return __builtin_shuffle (a, (vector char){ 15, 14, 13, 12, 11, 10, 9, 8,
>                                               7, 6, 5, 4, 3, 2, 1, 0 });
> }
> 
> generates:
> 
> f:
>         adrp    x0, .LANCHOR0
>         ldr     q31, [x0, #:lo12:.LANCHOR0]
>         tbl     v0.16b, {v0.16b}, v31.16b
>         ret
>         .set    .LANCHOR0,. + 0
>         .LC0:
>         .byte   15
>         .byte   14
>         .byte   13
>         .byte   12
>         .byte   11
>         .byte   10
>         .byte   9
>         .byte   8
>         .byte   7
>         .byte   6
>         .byte   5
>         .byte   4
>         .byte   3
>         .byte   2
>         .byte   1
>         .byte   0
> 
> With this patch, it generates REV64 followed by EXT:
> 
> f:
>         rev64   v0.16b, v0.16b
>         ext     v0.16b, v0.16b, v0.16b, #8
>         ret
> 
> Bootstrapped and tested on aarch64_linux_gnu.
> 
>         PR target/102055
> 
> gcc/ChangeLog:
> 
>         * config/aarch64/aarch64.cc (aarch64_evpc_rev64_ext): New.
>         (aarch64_expand_vec_perm_const_1): Call aarch64_evpc_rev64_ext.
> 
> gcc/testsuite/ChangeLog:
> 
>         * gcc.target/aarch64/pr102055.c: New test.
> 
> Signed-off-by: Pengxuan Zheng 
> <[email protected]<mailto:[email protected]>>
> ---
>  gcc/config/aarch64/aarch64.cc               | 32 ++++++++++++++++
>  gcc/testsuite/gcc.target/aarch64/pr102055.c | 42 +++++++++++++++++++++
>  2 files changed, 74 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr102055.c
> 
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 62194b96450..5fdb1a42c99 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -27714,6 +27714,36 @@ aarch64_evpc_rev_global (struct expand_vec_perm_d *d)
>    return true;
>  }
> 
> +/* Recognize patterns for the Advanced SIMD REV64 + EXT insns, which reverse
> +   elements within a full vector.  */
> +
> +static bool
> +aarch64_evpc_rev64_ext (struct expand_vec_perm_d *d)
> +{
> +  poly_uint64 nelt = d->perm.length ();
> +
> +  if (!d->one_vector_p || d->vec_flags != VEC_ADVSIMD)
> +    return false;
> +
> +  if (!d->perm.series_p (0, 1, nelt - 1, -1))
> +    return false;
> +
> +  if (d->testing_p)
> +    return true;
> +
> +  rtx tmp1 = gen_reg_rtx (d->vmode);
> +  rtx tmp2 = gen_reg_rtx (V16QImode);
> +  rtx unspec_rev64
> +      = gen_rtx_UNSPEC (d->vmode, gen_rtvec (1, d->op0), UNSPEC_REV64);
> +  emit_set_insn (tmp1, unspec_rev64);
> +  rtvec vec = gen_rtvec (3, gen_lowpart (V16QImode, tmp1),
> +                        gen_lowpart (V16QImode, tmp1), GEN_INT (8));
> +  rtx unspec_ext = gen_rtx_UNSPEC (V16QImode, vec, UNSPEC_EXT);
> +  emit_set_insn (tmp2, unspec_ext);
> +  emit_set_insn (d->target, gen_lowpart (d->vmode, tmp2));
> +  return true;
> +}
> +
>  static bool
>  aarch64_evpc_dup (struct expand_vec_perm_d *d)
>  {
> @@ -28175,6 +28205,8 @@ aarch64_expand_vec_perm_const_1 (struct 
> expand_vec_perm_d *d)
>             return true;
>           else if (aarch64_evpc_hvla (d))
>             return true;
> +         else if (aarch64_evpc_rev64_ext (d))
> +           return true;
>           else if (aarch64_evpc_reencode (d))
>             return true;
> 
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr102055.c 
> b/gcc/testsuite/gcc.target/aarch64/pr102055.c
> new file mode 100644
> index 00000000000..39b6355fc66
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr102055.c
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +#define vector __attribute__ ((vector_size (16)))
> +
> +/*
> +** f:
> +**     rev64   v([0-9]+).16b, v0.16b
> +**     ext     v0.16b, v\1.16b, v\1.16b, #8
> +**     ret
> +*/
> +vector char
> +f (vector char a)
> +{
> +  return __builtin_shuffle (a, (vector char){ 15, 14, 13, 12, 11, 10, 9, 8,
> +                                             7, 6, 5, 4, 3, 2, 1, 0 });
> +}
> +
> +/*
> +** f1:
> +**     rev64   v([0-9]+).8h, v0.8h
> +**     ext     v0.16b, v\1.16b, v\1.16b, #8
> +**     ret
> +*/
> +vector short
> +f1 (vector short a)
> +{
> +  return __builtin_shuffle (a, (vector short){ 7, 6, 5, 4, 3, 2, 1, 0 });
> +}
> +
> +/*
> +** f2:
> +**     rev64   v([0-9]+).4s, v0.4s
> +**     ext     v0.16b, v\1.16b, v\1.16b, #8
> +**     ret
> +*/
> +vector int
> +f2 (vector int a)
> +{
> +  return __builtin_shuffle (a, (vector int){ 3, 2, 1, 0 });
> +}
> --
> 2.34.1

-- 

Reply via email to