Re: [PATCH v2 1/1] aarch64: Add SME2.1 structured sparsity outer product intrinsics
Alex Coplan <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
On 17/08/2026 19:12, Claudio Bantaloukas wrote:
> Hellos!
>
> On 17/08/2026 15:46, Alex Coplan wrote:
> > Hi Claudio,
> >
> > Thanks for incorporating the changes from the last review.
> > This is looking much better. A few small comments below.
> >
> > On 28/07/2026 05:08, Claudio Bantaloukas wrote:
> > > This patch adds support for the intrinsics implementing FEAT_SME_TMOP listed
> > > below. All of these require the +sme-tmop arch option.
> > > A new intrinsic shape and a new register constraint is required. This patch
> > > adds these, along with tests. Also added a pair of predicates to tighten the
> > > immediates selecting ZA tiles based on tile width.
> > >
> > > Specifically:
> > > - svtmopa_lane_za16[_mf8_mf8] (requires +sme-f8f16)
> > > - svtmopa_lane_za16[_bf16_bf16] (requires +sme-b16b16)
> > > - svtmopa_lane_za16[_f16_f16] (requires +sme-f16f16)
> > > - svtmopa_lane_za32[_mf8_mf8] (requires +sme-f8f32)
> > > - svtmopa_lane_za32[_bf16_bf16] (requires +sme-b16b16)
> > > - svtmopa_lane_za32[_f16_f16]
> > > - svtmopa_lane_za32[_f32_f32]
> > > - svtmopa_lane_za32[_s16_s16]
> > > - svtmopa_lane_za32[_u16_u16]
> > > - svtmopa_lane_za32[_s8_s8]
> > > - svtmopa_lane_za32[_u8_u8]
> > > - svtmopa_lane_za32[_s8_u8]
> > > - svtmopa_lane_za32[_u8_s8]
> > >
> > <snip>
> > > diff --git a/gcc/config/aarch64/aarch64-sme.md b/gcc/config/aarch64/aarch64-sme.md
> > > index 7091f566ba8..089089eda52 100644
> > > --- a/gcc/config/aarch64/aarch64-sme.md
> > > +++ b/gcc/config/aarch64/aarch64-sme.md
> > > @@ -46,11 +46,13 @@
> > > ;; ---- [INT] Dot product
> > > ;; ---- [INT] Ternary widening arithmetic on ZA slice
> > > ;; ---- [INT] Sum of outer products
> > > +;; ---- [INT] Sparse outer product
> > > ;; ---- [FP] Dot product
> > > ;; ---- [FP8] Dot product
> > > ;; ---- [FP] Ternary arithmetic on ZA slice
> > > ;; ---- [FP] Ternary widening arithmetic on ZA slice
> > > ;; ---- [FP] Sum of outer products
> > > +;; ---- [FP] Sparse outer product
> > > ;;
> > > ;; == Table lookup
> > > ;; ---- Table lookup
> > > @@ -1814,6 +1816,53 @@ (define_insn "@aarch64_sme_<optab><VNx4SI_ONLY:mode><VNx4SI_ONLY:mode>"
> > > "<optab>\tza%0.s, %1/m, %2/m, %3.s, %4.s"
> > > )
> > > +;; -------------------------------------------------------------------------
> > > +;; ---- [INT] Sparse outer product
> > > +;; -------------------------------------------------------------------------
> > > +;; Includes:
> > > +;; - STMOPA
> > > +;; - UTMOPA
> > > +;; - SUTMOPA
> > > +;; - USTMOPA
> > > +;; -------------------------------------------------------------------------
> > > +;; svtmopa_lane_za32[_s16_s16]
> > > +;; svtmopa_lane_za32[_u16_u16]
> > > +;; svtmopa_lane_za32[_s8_s8]
> > > +;; svtmopa_lane_za32[_u8_u8]
> > > +(define_insn "@aarch64_sme_lane_<optab><VNx4SI_ONLY:mode><SVE_FULL_BHI:mode>"
> > > + [(set (reg:VNx4SI_ONLY ZA_REGNUM)
> > > + (unspec:VNx4SI_ONLY
> > > + [(reg:VNx4SI_ONLY ZA_REGNUM)
> > > + (reg:DI SME_STATE_REGNUM)
> > > + (match_operand:DI 0 "aarch64_za32_imm")
> > > + (match_operand:<SVE_FULL_BHI:VDOUBLE> 1 "aligned_register_operand" "Uw2")
> > > + (match_operand:SVE_FULL_BHI 2 "register_operand" "w")
> > > + (match_operand:VNx16QI 3 "register_operand" "Uwo")
> > > + (match_operand:DI 4 "aarch64_imm2")
> > > + ]
> > > + SME_TMOP_INT))]
> > > + "TARGET_STREAMING_SME_TMOP"
> > > + "<optab>\tza%0.s, %1, %2.<SVE_FULL_BHI:Vetype>, %3[%4]"
> > > +)
> > > +
> > > +;; svtmopa_lane_za32[_s8_u8]
> > > +;; svtmopa_lane_za32[_u8_s8]
> > > +(define_insn "@aarch64_sme_lane_<optab><VNx4SI_ONLY:mode><VNx16QI_ONLY:mode>"
> > > + [(set (reg:VNx4SI_ONLY ZA_REGNUM)
> > > + (unspec:VNx4SI_ONLY
> > > + [(reg:VNx4SI_ONLY ZA_REGNUM)
> > > + (reg:DI SME_STATE_REGNUM)
> > > + (match_operand:DI 0 "aarch64_za32_imm")
> > > + (match_operand:<VNx16QI_ONLY:VDOUBLE> 1 "aligned_register_operand" "Uw2")
> > > + (match_operand:VNx16QI_ONLY 2 "register_operand" "w")
> > > + (match_operand:VNx16QI 3 "register_operand" "Uwo")
> > > + (match_operand:DI 4 "aarch64_imm2")
> > > + ]
> > > + SME_TMOP_INT_CROSS))]
> > > + "TARGET_STREAMING_SME_TMOP"
> > > + "<optab>\tza%0.s, %1, %2.<VNx16QI_ONLY:Vetype>, %3[%4]"
> > > +)
> > > +
> > > ;; -------------------------------------------------------------------------
> > > ;; ---- [FP] Dot product
> > > ;; -------------------------------------------------------------------------
> > > @@ -2737,6 +2786,69 @@ (define_insn "@aarch64_sme_<optab><SME_ZA_F8F16_32:mode><VNx16QI_ONLY:mode>"
> > > "<optab>\tza%0.<SME_ZA_F8F16_32:Vetype>, %1/m, %2/m, %3.b, %4.b"
> > > )
> > > +;; -------------------------------------------------------------------------
> > > +;; ---- [FP] Sparse outer product
> > > +;; -------------------------------------------------------------------------
> > > +;; Includes:
> > > +;; - BFTMOPA (SME_TMOP)
> > > +;; - FTMOPA (SME_TMOP)
> > > +;; -------------------------------------------------------------------------
> > > +;; svtmopa_lane_za16[_bf16_bf16]
> > > +;; svtmopa_lane_za16[_f16_f16]
> > > +;; svtmopa_lane_za32[_f32_f32]
> > > +(define_insn "@aarch64_sme_lane_<optab><mode><mode>"
> > > + [(set (reg:SVE_FULL_BHSF ZA_REGNUM)
> >
> > It looks like nothing currently restricts the availability of this
> > pattern. In particular the _f16_f16 overload above should only be
> > available when we have FEAT_SME_F16F16, and similarly the _bf16_bf16
> > variant only when FEAT_SME_B16B16 is enabled.
> >
> > It looks like you maybe meant to use SME_TMOPA_ZA16 here (although it
> > would be missing a mode for the _f32_f32 overload), which AFAICT is
> > currently unused.
> >
> Thanks for catching (facepalm)!
> Patch will follow soon.
>
> > > + (unspec:SVE_FULL_BHSF
> > > + [(reg:SVE_FULL_BHSF ZA_REGNUM)
> > > + (reg:DI SME_STATE_REGNUM)
> > > + (match_operand:DI 0 "aarch64_za<elem_bits>_imm")
> > > + (match_operand:<VDOUBLE> 1 "aligned_register_operand" "Uw2")
> > > + (match_operand:SVE_FULL_BHSF 2 "register_operand" "w")
> > > + (match_operand:VNx16QI 3 "register_operand" "Uwo")
> > > + (match_operand:DI 4 "aarch64_imm2")
> > > + ]
> > > + SME_TMOP_FP))]
> > > + "TARGET_STREAMING_SME_TMOP"
> > > + "<b><optab>\tza%0.<Vetype>, %1, %2.<Vetype>, %3[%4]"
> > > +)
> > > +
> > > +;; svtmopa_lane_za32[_bf16_bf16]
> > > +;; svtmopa_lane_za32[_f16_f16]
> > > +(define_insn "@aarch64_sme_lane_<optab><VNx4SI_ONLY:mode><SVE_FULL_HF:mode>"
> > > + [(set (reg:SVE_FULL_HF ZA_REGNUM)
> > > + (unspec:SVE_FULL_HF
> > > + [(reg:VNx4SI_ONLY ZA_REGNUM)
> > > + (reg:DI SME_STATE_REGNUM)
> > > + (match_operand:DI 0 "aarch64_za32_imm")
> > > + (match_operand:<SVE_FULL_HF:VDOUBLE> 1 "aligned_register_operand" "Uw2")
> > > + (match_operand:SVE_FULL_HF 2 "register_operand" "w")
> > > + (match_operand:VNx16QI 3 "register_operand" "Uwo")
> > > + (match_operand:DI 4 "aarch64_imm2")
> > > + ]
> > > + SME_TMOP_FP))]
> > > + "TARGET_STREAMING_SME_TMOP"
> > > + "<SVE_FULL_HF:b><optab>\tza%0.s, %1, %2.<SVE_FULL_HF:Vetype>, %3[%4]"
> > > +)
> > > +
> > > +;; svtmopa_lane_za16[_mf8_mf8]_fpm
> > > +;; svtmopa_lane_za32[_mf8_mf8]_fpm
> > > +(define_insn "@aarch64_sme_lane_<optab><SME_ZA_TMOP_FP8:mode><VNx16QI_ONLY:mode>"
> > > + [(set (reg:SME_ZA_TMOP_FP8 ZA_REGNUM)
> > > + (unspec:SME_ZA_TMOP_FP8
> > > + [(reg:SME_ZA_TMOP_FP8 ZA_REGNUM)
> > > + (reg:DI SME_STATE_REGNUM)
> > > + (match_operand:DI 0 "aarch64_za<SME_ZA_TMOP_FP8:elem_bits>_imm")
> > > + (match_operand:<VNx16QI_ONLY:VDOUBLE> 1 "aligned_register_operand" "Uw2")
> > > + (match_operand:VNx16QI_ONLY 2 "register_operand" "w")
> > > + (match_operand:VNx16QI 3 "register_operand" "Uwo")
> > > + (match_operand:DI 4 "aarch64_imm2")
> > > + (reg:DI FPM_REGNUM)
> > > + ]
> > > + SME_TMOP_FP8))]
> > > + "TARGET_STREAMING_SME_TMOP"
> > > + "<optab>\tza%0.<SME_ZA_TMOP_FP8:Vetype>, %1, %2.b, %3[%4]"
> > > +)
> > > +
> > <snip>
> > > diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md
> > > index a8b976e4b71..e2ee8caf2a8 100644
> > > --- a/gcc/config/aarch64/iterators.md
> > > +++ b/gcc/config/aarch64/iterators.md
> > > @@ -589,6 +589,10 @@ (define_mode_iterator SVE_FULL_HSI [VNx8HI VNx4SI])
> > > ;; elements.
> > > (define_mode_iterator SVE_FULL_HSF [VNx8HF VNx4SF])
> > > +;; Fully-packed SVE floating-point vector modes that have 16-bit or 32-bit
> > > +;; elements, including brain float.
> > > +(define_mode_iterator SVE_FULL_BHSF [VNx8BF VNx8HF VNx4SF])
> >
> > As mentioned on the patterns, we probably don't want this iterator, instead we
> > probably want to adjust the SME_TMOPA_ZA16 iterator below.
>
> Ack
>
> > > +
> > > ;; Like SVE_FULL_HSF, but selectively enables those modes that are valid
> > > ;; for the variant of the SVE2 FP8 FDOT instruction associated with that
> > > ;; mode.
> > > @@ -847,6 +851,12 @@ (define_mode_iterator SME_MOP_HSDF [VNx4SF
> > > (VNx8HF "TARGET_STREAMING_SME_F16F16")
> > > (VNx8BF "TARGET_STREAMING_SME_B16B16")])
> > > +(define_mode_iterator SME_TMOPA_ZA16 [(VNx8HF "TARGET_STREAMING_SME_F16F16")
> > > + (VNx8BF "TARGET_STREAMING_SME_B16B16")])
> > > +
> > > +(define_mode_iterator SME_ZA_TMOP_FP8 [(VNx8HI "TARGET_STREAMING_SME_F8F16")
> > > + (VNx4SI "TARGET_STREAMING_SME_F8F32")])
> > > +
> > <snip>
> > > diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
> > > index f02486c2d9a..7b4142ea889 100644
> > > --- a/gcc/config/aarch64/predicates.md
> > > +++ b/gcc/config/aarch64/predicates.md
> > > @@ -290,6 +290,18 @@ (define_predicate "aarch64_lane_imm3"
> > > (and (match_code "const_int")
> > > (match_test "UINTVAL (op) <= 7")))
> > > +;; This is a 1-bit field used to select 16 bit sized ZA tiles.
> > > +;; Maps to SME_ZAda_1b fields in binutils
> > > +(define_predicate "aarch64_za16_imm"
> > > + (and (match_code "const_int")
> > > + (match_test "UINTVAL (op) <= 1")))
> >
> > Can we call this aarch64_imm1 instead, please? That way we have a
> > consistent naming scheme (needed for the patterns which have multiple ZA
> > modes) but this still allows us to re-use existing iterators where
> > possible (see below).
> >
> > > +
> > > +;; This is a 2-bit field used to select 32 bit sized ZA tiles
> > > +;; Maps to SME_ZAda_2b fields in binutils
> > > +(define_predicate "aarch64_za32_imm"
> > > + (and (match_code "const_int")
> > > + (match_test "UINTVAL (op) <= 3")))
> >
> > As I suggested in the original review, there's no need to add a new
> > predicate here: aarch64_imm2 works just fine.
> >
> > You'll just need a mode attribute mapping ZA modes to the number of
> > immediate bits needed to address them. So the predicates become
> > "aarch64_imm<za_imm_bits>" or so in the patterns.
> >
> I was thinking about following your initial suggestion but then opted for
> aarch64_za<elem_bits>_imm for a few reasons.
> One is because elem_bits is already there whereas za_imm_bits isn't.
Yes, but given you're going to have to add new preidcates if you don't
add za_imm_bits, this seems a bit moot.
> One that I feel strongly about is that the za in the predicate shows intent.
I think "aarch64_imm<za_imm_bits>" shows the intent equally well.
> It makes it a little bit easier to see at a glance which opearand denotes
> the tile.
> And the one that I think we should discuss more widely is what to do with
> aarch64_imm3. It's the wrong predicate to use for this case because it
> limits the values to 4 or less whereas we would need something like
> aarch64_lane_imm3 which correctly constrains us to 7 or less.
That's a good point, I hadn't realised that aarch64_imm3 doesn't actually
follow the pattern here. However, I don't think it presents an immediate
problem for the purposes of this patch.
We have the following situation:
ZA element size | num ZA tiles | immediate bits
================+==============+====================
8 | 1 | 0
16 | 2 | 1
32 | 4 | 2
64 | 8 | 3
and none of the TMOP instructions have 64-bit ZA tile destintions (only
16- or 32-bit).
I agree, however, that using this approach presents a potential footgun
for future SME patches (which might start using aarch64_imm3), so either
as a pre-patch or follow-up patch, we should probably do the renaming
you suggest below, making the aarch64_immN family of predicates
self-consistent.
FWIW, I think aarch64_shift_imm3 is fine as a replacement name for the
current aarch64_imm3.
>
> If you feel strongly about adding za_imm_bits, would you consider it
> complete if it was limited to all the modes in elem_bits belonging to 8(0),
> 16(1), 32(2), 64(3), 128(4) bit za modes or would you rather I keep 0 out?
I don't think it would need to be implemented for all the modes in
elem_bits. In general, iterators and mode attributes only need
implement what is actually practically needed. In this case I think
only { VNx8HI, VNx4SI, VNx2DI } and corresponding F modes are needed.
If you wanted to be hyper-pragmatic, I think only VNx8HF and VNx4SF are
needed by your patch as it stands, but we might as well implement the
complete set that is likely to be used in practice.
>
> Fields using aarch64_imm3 are described as "Is the left shift amount to be
> applied after extension in the range 0 to 4". I guess we could call it
> aarch64_shift_amount_imm3 and replace it in the extended register versions
> of add, adds etc. Then we could rename aarch64_lane_imm3 to aarch64_imm3 but
> if you look at places where aarch64_lane_imm3 is used, you'd spot the slight
> loss in context this extended name brings.
>
> Let me know where you lean on this and I'll go that way.
As said above, I think we should use aarch64_imm<za_imm_bits>, and do
this renaming (either as a pre-patch or follow-up patch), provided there
are no objections from other reviewers/maintainers.
Thanks,
Alex
>
> > > +
> > > ;; An immediate that fits into 24 bits, but needs splitting.
> > > (define_predicate "aarch64_split_imm24"
> > > (match_code "const_int")
> > <snip>
> > > diff --git a/gcc/testsuite/gcc.target/aarch64/sme2/acle-asm/tmopa_lane_za16_bf16_bf16.c b/gcc/testsuite/gcc.target/aarch64/sme2/acle-asm/tmopa_lane_za16_bf16_bf16.c
> > > new file mode 100644
> > > index 00000000000..381ec046f45
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/aarch64/sme2/acle-asm/tmopa_lane_za16_bf16_bf16.c
> > > @@ -0,0 +1,76 @@
> > > +/* { dg-do assemble { target { aarch64_asm_sme-b16b16_ok && aarch64_asm_sme-tmop_ok} } } */
> > > +/* { dg-do compile { target { ! { aarch64_asm_sme-b16b16_ok && aarch64_asm_sme-tmop_ok } } } } */
> > > +/* { dg-final { check-function-bodies "**" "" "-DCHECK_ASM" } } */
> > > +
> > > +#include "test_sme2_acle.h"
> > > +
> > > +#pragma GCC target "+sme-tmop+sme-b16b16"
> > > +
> > > +/*
> > > +** tmopa_lane_za16_bf16_bf16_0_z0_z4_z20_0:
> > > +** bftmopa za0\.h, {z0\.h - z1\.h}, z4\.h, z20\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z0_z4_z20_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z0, z4, z20, 0),
> > > + svtmopa_lane_za16 (0, z0, z4, z20, 0))
> > > +
> > > +/* ZA slice and offset with different values.
> > > +** tmopa_lane_za16_bf16_bf16_1_z2_z4_z20_3:
> > > +** bftmopa za1\.h, {z2\.h - z3\.h}, z4\.h, z20\[3\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_1_z2_z4_z20_3, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (1, z2, z4, z20, 3),
> > > + svtmopa_lane_za16 (1, z2, z4, z20, 3))
> > > +
> > > +/* The first register on the second argument must be even.
> > > +** tmopa_lane_za16_bf16_bf16_0_z1_z4_z20_0:
> > > +** mov (z\d+)\.d, z1\.d
> > > +** mov (z\d+)\.d, z2\.d
> > > +** bftmopa za0\.h, {\1\.h - \2\.h}, z4\.h, z20\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z1_z4_z20_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z1, z4, z20, 0),
> > > + svtmopa_lane_za16 (0, z1, z4, z20, 0))
> > > +
> > > +/* zk register must be one of Z20-Z23 or Z28-z31.
> > > +** tmopa_lane_za16_bf16_bf16_0_z0_z4_z19_0:
> > > +** mov (z2[0-3]|z2[89]|z3[01]).d, z19.d
> >
> > Very minor nit, but you could factor the z out of the alternation, such
> > that you're only matching the register number (adding it back in again
> > below when you have the backreference). It would make the regex a bit
> > tidier / more readable. Same for the other occurrences.
> >
> ack, will fix :)
>
>
> > OK with those changes, thanks.
> >
> > Alex
> >
> > > +** bftmopa za0\.h, {z0\.h - z1\.h}, z4\.h, \1\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z0_z4_z19_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z0, z4, z19, 0),
> > > + svtmopa_lane_za16 (0, z0, z4, z19, 0))
> > > +
> > > +/* zk register must be one of Z20-Z23 or Z28-z31.
> > > +** tmopa_lane_za16_bf16_bf16_0_z0_z4_z24_0:
> > > +** mov (z2[0-3]|z2[89]|z3[01]).d, z24.d
> > > +** bftmopa za0\.h, {z0\.h - z1\.h}, z4\.h, \1\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z0_z4_z24_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z0, z4, z24, 0),
> > > + svtmopa_lane_za16 (0, z0, z4, z24, 0))
> > > +
> > > +/* zk register must be one of Z20-Z23 or Z28-z31.
> > > +** tmopa_lane_za16_bf16_bf16_0_z0_z4_z27_0:
> > > +** mov (z2[0-3]|z2[89]|z3[01]).d, z27.d
> > > +** bftmopa za0\.h, {z0\.h - z1\.h}, z4\.h, \1\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z0_z4_z27_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z0, z4, z27, 0),
> > > + svtmopa_lane_za16 (0, z0, z4, z27, 0))
> > > +
> > > +/* zk register must be one of Z20-Z23 or Z28-z31.
> > > +** tmopa_lane_za16_bf16_bf16_0_z0_z4_z28_0:
> > > +** bftmopa za0\.h, {z0\.h - z1\.h}, z4\.h, z28\[0\]
> > > +** ret
> > > +*/
> > > +TEST_ZA_TMOP (tmopa_lane_za16_bf16_bf16_0_z0_z4_z28_0, svbfloat16x2_t, svbfloat16_t,
> > > + svtmopa_lane_za16_bf16_bf16 (0, z0, z4, z28, 0),
> > > + svtmopa_lane_za16 (0, z0, z4, z28, 0))
> > > +
> > <snip>
>