Re: [PATCH v2 1/1] aarch64: Add SME2.1 structured sparsity outer product intrinsics
Claudio Bantaloukas <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
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.
One that I feel strongly about is that the za in the predicate shows
intent. 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.
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?
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.
>> +
>> ;; 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>