On 18/08/2026 12:19, Alex Coplan wrote:
(snip)>>> You'll just need a mode attribute mapping ZA modes to the number ofOn 17/08/2026 19:12, Claudio Bantaloukas wrote:Hellos!
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.
Absolutely!
(match_operand:DI 0 "aarch64_za<elem_bits>_imm")
vs
(match_operand:DI 0 "aarch64_imm<za_imm_bits>")
are equally readable and convey what arg 0 is very clearly.
But I find
(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")
slightly better to
(match_operand:DI 0 "aarch64_imm2")
(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")
Nevertheless, I'll go ahead with the change set out below unless told
otherwise.
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.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>
