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
I was thinking about following your initial suggestion but then opted for aarch64_za<elem_bits>_imm for a few reasons.+ ;; 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.
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.dVery 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>
