On Wed, Aug 26, 2026 at 1:33 AM Tamar Christina <[email protected]> wrote:
> 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 for explaining the constant issue, Tamar. I've updated the patch accordingly. Please let me know if you have any comments. https://gcc.gnu.org/pipermail/gcc-patches/2026-August/729054.html Thanks, Pengxuan > > 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 > > -- >
