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]> |
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.
> + (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.
> +
> ;; 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.
> +
> ;; 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.
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>