Skip to content

[Clang] Refactor diagnostics for SME builtins. #78258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

sdesmalen-arm
Copy link
Collaborator

The arm_sme.td file was still using IsSharedZA and IsPreservesZA, which should be changed to match the new state attributes added in #76971.

This patch adds IsInZA, IsOutZA and IsInOutZA as the state for the Clang builtins and fixes up the code in SemaChecking and SveEmitter to match.

Note that the code is written in such a way that it can be easily extended with ZT0 state (to follow in a future patch).

The arm_sme.td file was still using `IsSharedZA` and `IsPreservesZA`,
which should be changed to match the new state attributes added in llvm#76971.

This patch adds `IsInZA`, `IsOutZA` and `IsInOutZA` as the state for the
Clang builtins and fixes up the code in SemaChecking and SveEmitter to match.

Note that the code is written in such a way that it can be easily extended
with ZT0 state (to follow in a future patch).
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

@llvm/pr-subscribers-clang

Author: Sander de Smalen (sdesmalen-arm)

Changes

The arm_sme.td file was still using IsSharedZA and IsPreservesZA, which should be changed to match the new state attributes added in #76971.

This patch adds IsInZA, IsOutZA and IsInOutZA as the state for the Clang builtins and fixes up the code in SemaChecking and SveEmitter to match.

Note that the code is written in such a way that it can be easily extended with ZT0 state (to follow in a future patch).


Patch is 100.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78258.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/arm_sme.td (+286-286)
  • (modified) clang/include/clang/Basic/arm_sve_sme_incl.td (+7-6)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+19-16)
  • (modified) clang/test/Sema/aarch64-incompat-sm-builtin-calls.c (+5)
  • (modified) clang/utils/TableGen/SveEmitter.cpp (+19-11)
diff --git a/clang/include/clang/Basic/arm_sme.td b/clang/include/clang/Basic/arm_sme.td
index aac3bd486de9223..4fb50b8e4e4e565 100644
--- a/clang/include/clang/Basic/arm_sme.td
+++ b/clang/include/clang/Basic/arm_sme.td
@@ -21,19 +21,19 @@ include "arm_sve_sme_incl.td"
 multiclass ZALoad<string n_suffix, string t, string i_prefix, list<ImmCheck> ch> {
   let TargetGuard = "sme" in {
     def NAME # _H : MInst<"svld1_hor_" # n_suffix, "vimPQ", t,
-                          [IsLoad, IsOverloadNone, IsStreaming, IsSharedZA],
+                          [IsLoad, IsOverloadNone, IsStreaming, IsInOutZA],
                           MemEltTyDefault, i_prefix # "_horiz", ch>;
 
     def NAME # _H_VNUM : MInst<"svld1_hor_vnum_" # n_suffix, "vimPQl", t,
-                               [IsLoad, IsOverloadNone, IsStreaming, IsSharedZA],
+                               [IsLoad, IsOverloadNone, IsStreaming, IsInOutZA],
                                MemEltTyDefault, i_prefix # "_horiz", ch>;
 
     def NAME # _V : MInst<"svld1_ver_" # n_suffix, "vimPQ", t,
-                          [IsLoad, IsOverloadNone, IsStreaming, IsSharedZA],
+                          [IsLoad, IsOverloadNone, IsStreaming, IsInOutZA],
                           MemEltTyDefault, i_prefix # "_vert", ch>;
 
     def NAME # _V_VNUM : MInst<"svld1_ver_vnum_" # n_suffix, "vimPQl", t,
-                               [IsLoad, IsOverloadNone, IsStreaming, IsSharedZA],
+                               [IsLoad, IsOverloadNone, IsStreaming, IsInOutZA],
                                MemEltTyDefault, i_prefix # "_vert", ch>;
   }
 }
@@ -45,11 +45,11 @@ defm SVLD1_ZA64 : ZALoad<"za64", "l", "aarch64_sme_ld1d", [ImmCheck<0, ImmCheck0
 defm SVLD1_ZA128 : ZALoad<"za128", "q", "aarch64_sme_ld1q", [ImmCheck<0, ImmCheck0_15>]>;
 
 def SVLDR_VNUM_ZA : MInst<"svldr_vnum_za", "vmQl", "",
-                          [IsOverloadNone, IsStreamingCompatible, IsSharedZA],
+                          [IsOverloadNone, IsStreamingCompatible, IsInOutZA],
                           MemEltTyDefault, "aarch64_sme_ldr">;
 
 def SVLDR_ZA : MInst<"svldr_za", "vmQ", "",
-                          [IsOverloadNone, IsStreamingCompatible, IsSharedZA],
+                          [IsOverloadNone, IsStreamingCompatible, IsInOutZA],
                           MemEltTyDefault, "aarch64_sme_ldr", []>;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -58,19 +58,19 @@ def SVLDR_ZA : MInst<"svldr_za", "vmQ", "",
 multiclass ZAStore<string n_suffix, string t, string i_prefix, list<ImmCheck> ch> {
   let TargetGuard = "sme" in {
     def NAME # _H : MInst<"svst1_hor_" # n_suffix, "vimP%", t,
-                          [IsStore, IsOverloadNone, IsStreaming, IsSharedZA, IsPreservesZA],
+                          [IsStore, IsOverloadNone, IsStreaming, IsInZA],
                           MemEltTyDefault, i_prefix # "_horiz", ch>;
 
     def NAME # _H_VNUM : MInst<"svst1_hor_vnum_" # n_suffix, "vimP%l", t,
-                               [IsStore, IsOverloadNone, IsStreaming, IsSharedZA, IsPreservesZA],
+                               [IsStore, IsOverloadNone, IsStreaming, IsInZA],
                                MemEltTyDefault, i_prefix # "_horiz", ch>;
 
     def NAME # _V : MInst<"svst1_ver_" # n_suffix, "vimP%", t,
-                          [IsStore, IsOverloadNone, IsStreaming, IsSharedZA, IsPreservesZA],
+                          [IsStore, IsOverloadNone, IsStreaming, IsInZA],
                           MemEltTyDefault, i_prefix # "_vert", ch>;
 
     def NAME # _V_VNUM : MInst<"svst1_ver_vnum_" # n_suffix, "vimP%l", t,
-                               [IsStore, IsOverloadNone, IsStreaming, IsSharedZA, IsPreservesZA],
+                               [IsStore, IsOverloadNone, IsStreaming, IsInZA],
                                MemEltTyDefault, i_prefix # "_vert", ch>;
   }
 }
@@ -82,11 +82,11 @@ defm SVST1_ZA64 : ZAStore<"za64", "l", "aarch64_sme_st1d", [ImmCheck<0, ImmCheck
 defm SVST1_ZA128 : ZAStore<"za128", "q", "aarch64_sme_st1q", [ImmCheck<0, ImmCheck0_15>]>;
 
 def SVSTR_VNUM_ZA : MInst<"svstr_vnum_za", "vm%l", "",
-                          [IsOverloadNone, IsStreamingCompatible, IsSharedZA, IsPreservesZA],
+                          [IsOverloadNone, IsStreamingCompatible, IsInZA],
                           MemEltTyDefault, "aarch64_sme_str">;
 
 def SVSTR_ZA : MInst<"svstr_za", "vm%", "",
-                      [IsOverloadNone, IsStreamingCompatible, IsSharedZA, IsPreservesZA],
+                      [IsOverloadNone, IsStreamingCompatible, IsInZA],
                       MemEltTyDefault, "aarch64_sme_str", []>;
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -96,11 +96,11 @@ multiclass ZARead<string n_suffix, string t, string i_prefix, list<ImmCheck> ch>
   let TargetGuard = "sme" in {
     def NAME # _H : SInst<"svread_hor_" # n_suffix # "[_{d}]", "ddPim", t,
                           MergeOp1, i_prefix # "_horiz",
-                          [IsReadZA, IsStreaming, IsSharedZA, IsPreservesZA], ch>;
+                          [IsReadZA, IsStreaming, IsInZA], ch>;
 
     def NAME # _V : SInst<"svread_ver_" # n_suffix # "[_{d}]", "ddPim", t,
                           MergeOp1, i_prefix # "_vert",
-                          [IsReadZA, IsStreaming, IsSharedZA, IsPreservesZA], ch>;
+                          [IsReadZA, IsStreaming, IsInZA], ch>;
   }
 }
 
@@ -117,11 +117,11 @@ multiclass ZAWrite<string n_suffix, string t, string i_prefix, list<ImmCheck> ch
   let TargetGuard = "sme" in {
     def NAME # _H : SInst<"svwrite_hor_" # n_suffix # "[_{d}]", "vimPd", t,
                           MergeOp1, i_prefix # "_horiz",
-                          [IsWriteZA, IsStreaming, IsSharedZA], ch>;
+                          [IsWriteZA, IsStreaming, IsInOutZA], ch>;
 
     def NAME # _V : SInst<"svwrite_ver_" # n_suffix # "[_{d}]", "vimPd", t,
                           MergeOp1, i_prefix # "_vert",
-                          [IsWriteZA, IsStreaming, IsSharedZA], ch>;
+                          [IsWriteZA, IsStreaming, IsInOutZA], ch>;
   }
 }
 
@@ -136,10 +136,10 @@ defm SVWRITE_ZA128 : ZAWrite<"za128", "csilUcUsUiUlhbfd", "aarch64_sme_writeq",
 
 let TargetGuard = "sme" in {
   def SVZERO_MASK_ZA : SInst<"svzero_mask_za", "vi", "", MergeNone, "aarch64_sme_zero",
-                             [IsOverloadNone, IsStreamingCompatible, IsSharedZA],
+                             [IsOverloadNone, IsStreamingCompatible, IsInOutZA],
                              [ImmCheck<0, ImmCheck0_255>]>;
   def SVZERO_ZA      : SInst<"svzero_za", "v", "", MergeNone, "aarch64_sme_zero",
-                             [IsOverloadNone, IsStreamingCompatible, IsSharedZA]>;
+                             [IsOverloadNone, IsStreamingCompatible, IsOutZA]>;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -149,7 +149,7 @@ multiclass ZACount<string n_suffix> {
   let TargetGuard = "sme" in {
     def NAME : SInst<"sv" # n_suffix, "nv", "", MergeNone,
                       "aarch64_sme_" # n_suffix,
-                      [IsOverloadNone, IsStreamingCompatible, IsPreservesZA]>;
+                      [IsOverloadNone, IsStreamingCompatible]>;
   }
 }
 
@@ -164,13 +164,13 @@ defm SVCNTSD : ZACount<"cntsd">;
 multiclass ZAAdd<string n_suffix> {
   let TargetGuard = "sme" in {
     def NAME # _ZA32: SInst<"sv" # n_suffix # "_za32[_{d}]", "viPPd", "iUi", MergeOp1,
-                      "aarch64_sme_" # n_suffix, [IsStreaming, IsSharedZA],
+                      "aarch64_sme_" # n_suffix, [IsStreaming, IsInOutZA],
                       [ImmCheck<0, ImmCheck0_3>]>;
   }
 
   let TargetGuard = "sme-i16i64" in {
     def NAME # _ZA64: SInst<"sv" # n_suffix # "_za64[_{d}]", "viPPd", "lUl", MergeOp1,
-                     "aarch64_sme_" # n_suffix, [IsStreaming, IsSharedZA],
+                     "aarch64_sme_" # n_suffix, [IsStreaming, IsInOutZA],
                      [ImmCheck<0, ImmCheck0_7>]>;
   }
 }
@@ -186,7 +186,7 @@ multiclass ZAIntOuterProd<string n_suffix1, string n_suffix2> {
     def NAME # _ZA32_B: SInst<"sv" # n_suffix2 # "_za32[_{d}]",
                               "viPPdd", !cond(!eq(n_suffix1, "s") : "", true: "U") # "c",
                               MergeOp1, "aarch64_sme_" # n_suffix1 # n_suffix2 # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_3>]>;
   }
 
@@ -194,7 +194,7 @@ multiclass ZAIntOuterProd<string n_suffix1, string n_suffix2> {
     def NAME # _ZA64_H: SInst<"sv" # n_suffix2 # "_za64[_{d}]",
                               "viPPdd", !cond(!eq(n_suffix1, "s") : "", true: "U") # "s",
                               MergeOp1, "aarch64_sme_" # n_suffix1 # n_suffix2 # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_7>]>;
   }
 }
@@ -213,7 +213,7 @@ multiclass ZAIntOuterProdMixedSigns<string n_suffix1, string n_suffix2> {
                               "viPPd" # !cond(!eq(n_suffix1, "su") : "u", true: "x"),
                               !cond(!eq(n_suffix1, "su") : "", true: "U") # "c",
                               MergeOp1, "aarch64_sme_" # n_suffix1 # n_suffix2 # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_3>]>;
   }
 
@@ -222,7 +222,7 @@ multiclass ZAIntOuterProdMixedSigns<string n_suffix1, string n_suffix2> {
                               "viPPd" # !cond(!eq(n_suffix1, "su") : "u", true: "x"),
                               !cond(!eq(n_suffix1, "su") : "", true: "U") # "s",
                               MergeOp1, "aarch64_sme_" # n_suffix1 # n_suffix2 # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_7>]>;
   }
 }
@@ -239,24 +239,24 @@ multiclass ZAFPOuterProd<string n_suffix> {
   let TargetGuard = "sme" in {
     def NAME # _ZA32_B: SInst<"sv" # n_suffix # "_za32[_{d}]", "viPPdd", "h",
                               MergeOp1, "aarch64_sme_" # n_suffix # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_3>]>;
 
     def NAME # _ZA32_H: SInst<"sv" # n_suffix # "_za32[_{d}]", "viPPdd", "b",
                               MergeOp1, "aarch64_sme_" # n_suffix # "_wide",
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_3>]>;
 
     def NAME # _ZA32_S: SInst<"sv" # n_suffix # "_za32[_{d}]", "viPPdd", "f",
                               MergeOp1, "aarch64_sme_" # n_suffix,
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_3>]>;
   }
 
   let TargetGuard = "sme-f64f64" in {
     def NAME # _ZA64_D: SInst<"sv" # n_suffix # "_za64[_{d}]", "viPPdd", "d",
                               MergeOp1, "aarch64_sme_" # n_suffix,
-                              [IsStreaming, IsSharedZA],
+                              [IsStreaming, IsInOutZA],
                               [ImmCheck<0, ImmCheck0_7>]>;
   }
 }
@@ -269,29 +269,29 @@ defm SVMOPS : ZAFPOuterProd<"mops">;
 
 multiclass ZAAddSub<string n_suffix> {
   let TargetGuard = "sme2" in {
-    def NAME # _WRITE_SINGLE_ZA32_VG1X2_I32 : Inst<"sv" # n_suffix # "_write[_single]_za32[_{d}]_vg1x2", "vm2d", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x2", [IsStreaming, IsSharedZA], []>;
-    def NAME # _WRITE_SINGLE_ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_write[_single]_za32[_{d}]_vg1x4", "vm4d", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x4", [IsStreaming, IsSharedZA], []>;
+    def NAME # _WRITE_SINGLE_ZA32_VG1X2_I32 : Inst<"sv" # n_suffix # "_write[_single]_za32[_{d}]_vg1x2", "vm2d", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x2", [IsStreaming, IsInOutZA], []>;
+    def NAME # _WRITE_SINGLE_ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_write[_single]_za32[_{d}]_vg1x4", "vm4d", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x4", [IsStreaming, IsInOutZA], []>;
 
-    def NAME # _WRITE_ZA32_VG1X2_I32 : Inst<"sv" # n_suffix # "_write_za32[_{d}]_vg1x2", "vm22", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x2", [IsStreaming, IsSharedZA], []>;
-    def NAME # _WRITE_ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_write_za32[_{d}]_vg1x4", "vm44", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x4", [IsStreaming, IsSharedZA], []>;
+    def NAME # _WRITE_ZA32_VG1X2_I32 : Inst<"sv" # n_suffix # "_write_za32[_{d}]_vg1x2", "vm22", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x2", [IsStreaming, IsInOutZA], []>;
+    def NAME # _WRITE_ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_write_za32[_{d}]_vg1x4", "vm44", "iUi", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x4", [IsStreaming, IsInOutZA], []>;
 
-    def NAME # _ZA32_VG1x2_I32 : Inst<"sv" # n_suffix # "_za32[_{d}]_vg1x2", "vm2", "iUif", MergeNone, "aarch64_sme_" # n_suffix # "_za32_vg1x2", [IsStreaming, IsSharedZA], []>;
-    def NAME # _ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_za32[_{d}]_vg1x4", "vm4", "iUif", MergeNone, "aarch64_sme_" # n_suffix # "_za32_vg1x4", [IsStreaming, IsSharedZA], []>;
+    def NAME # _ZA32_VG1x2_I32 : Inst<"sv" # n_suffix # "_za32[_{d}]_vg1x2", "vm2", "iUif", MergeNone, "aarch64_sme_" # n_suffix # "_za32_vg1x2", [IsStreaming, IsInOutZA], []>;
+    def NAME # _ZA32_VG1X4_I32 : Inst<"sv" # n_suffix # "_za32[_{d}]_vg1x4", "vm4", "iUif", MergeNone, "aarch64_sme_" # n_suffix # "_za32_vg1x4", [IsStreaming, IsInOutZA], []>;
 
     let TargetGuard = "sme-i16i64" in {
-      def NAME # _WRITE_SINGLE_ZA64_VG1X2_I64 : Inst<"sv" # n_suffix # "_write[_single]_za64[_{d}]_vg1x2", "vm2d", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x2", [IsStreaming, IsSharedZA], []>;
-      def NAME # _WRITE_SINGLE_ZA64_VG1X4_I64 : Inst<"sv" # n_suffix # "_write[_single]_za64[_{d}]_vg1x4", "vm4d", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x4", [IsStreaming, IsSharedZA], []>;
+      def NAME # _WRITE_SINGLE_ZA64_VG1X2_I64 : Inst<"sv" # n_suffix # "_write[_single]_za64[_{d}]_vg1x2", "vm2d", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x2", [IsStreaming, IsInOutZA], []>;
+      def NAME # _WRITE_SINGLE_ZA64_VG1X4_I64 : Inst<"sv" # n_suffix # "_write[_single]_za64[_{d}]_vg1x4", "vm4d", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_single_za_vg1x4", [IsStreaming, IsInOutZA], []>;
 
-      def NAME # _WRITE_ZA64_VG1x2_I64 : Inst<"sv" # n_suffix # "_write_za64[_{d}]_vg1x2", "vm22", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x2", [IsStreaming, IsSharedZA], []>;
-      def NAME # _WRITE_ZA64_VG1x4_I64 : Inst<"sv" # n_suffix # "_write_za64[_{d}]_vg1x4", "vm44", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x4", [IsStreaming, IsSharedZA], []>;
+      def NAME # _WRITE_ZA64_VG1x2_I64 : Inst<"sv" # n_suffix # "_write_za64[_{d}]_vg1x2", "vm22", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x2", [IsStreaming, IsInOutZA], []>;
+      def NAME # _WRITE_ZA64_VG1x4_I64 : Inst<"sv" # n_suffix # "_write_za64[_{d}]_vg1x4", "vm44", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_write_za_vg1x4", [IsStreaming, IsInOutZA], []>;
 
-      def NAME # _ZA64_VG1X2_I64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x2", "vm2", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x2", [IsStreaming, IsSharedZA], []>;
-      def NAME # _ZA64_VG1X4_I64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x4", "vm4", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x4", [IsStreaming, IsSharedZA], []>;
+      def NAME # _ZA64_VG1X2_I64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x2", "vm2", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x2", [IsStreaming, IsInOutZA], []>;
+      def NAME # _ZA64_VG1X4_I64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x4", "vm4", "lUl", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x4", [IsStreaming, IsInOutZA], []>;
     }
 
     let TargetGuard = "sme-f64f64" in {
-      def NAME # _ZA64_VG1X2_F64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x2", "vm2", "d", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x2", [IsStreaming, IsSharedZA], []>;
-      def NAME # _ZA64_VG1X4_F64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x4", "vm4", "d", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x4", [IsStreaming, IsSharedZA], []>;
+      def NAME # _ZA64_VG1X2_F64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x2", "vm2", "d", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x2", [IsStreaming, IsInOutZA], []>;
+      def NAME # _ZA64_VG1X4_F64 : Inst<"sv" # n_suffix # "_za64[_{d}]_vg1x4", "vm4", "d", MergeNone, "aarch64_sme_" # n_suffix # "_za64_vg1x4", [IsStreaming, IsInOutZA], []>;
     }
   }
 }
@@ -306,12 +306,12 @@ defm SVSUB : ZAAddSub<"sub">;
 //
 
 multiclass ZAWrite_VG<string n, string t, string i, list<ImmCheck> checks> {
-  def NAME # _VG2_H : Inst<"svwrite_hor_" # n # "[_{d}]_vg2",   "vim2", t, MergeNone, i # "_hor_vg2", [IsSharedZA, IsStreaming], checks>;
-  def NAME # _VG2_V : Inst<"svwrite_ver_" # n # "[_{d}]_vg2",   "vim2", t, MergeNone, i # "_ver_vg2", [IsSharedZA, IsStreaming], checks>;
-  def NAME # _VG4_H : Inst<"svwrite_hor_" # n # "[_{d}]_vg4",   "vim4", t, MergeNone, i # "_hor_vg4", [IsSharedZA, IsStreaming], checks>;
-  def NAME # _VG4_V : Inst<"svwrite_ver_" # n # "[_{d}]_vg4",   "vim4", t, MergeNone, i # "_ver_vg4", [IsSharedZA, IsStreaming], checks>;
-  def NAME # _VG1x2 : Inst<"svwrite_"     # n # "[_{d}]_vg1x2", "vm2",  t, MergeNone, i # "_vg1x2",   [IsSharedZA, IsStreaming], []>;
-  def NAME # _VG1x4 : Inst<"svwrite_"     # n # "[_{d}]_vg1x4", "vm4",  t, MergeNone, i # "_vg1x4",   [IsSharedZA, IsStreaming], []>;
+  def NAME # _VG2_H : Inst<"svwrite_hor_" # n # "[_{d}]_vg2",   "vim2", t, MergeNone, i # "_hor_vg2", [IsInOutZA, IsStreaming], checks>;
+  def NAME # _VG2_V : Inst<"svwrite_ver_" # n # "[_{d}]_vg2",   "vim2", t, MergeNone, i # "_ver_vg2", [IsInOutZA, IsStreaming], checks>;
+  def NAME # _VG4_H : Inst<"svwrite_hor_" # n # "[_{d}]_vg4",   "vim4", t, MergeNone, i # "_hor_vg4", [IsInOutZA, IsStreaming], checks>;
+  def NAME # _VG4_V : Inst<"svwrite_ver_" # n # "[_{d}]_vg4",   "vim4", t, MergeNone, i # "_ver_vg4", [IsInOutZA, IsStreaming], checks>;
+  def NAME # _VG1x2 : Inst<"svwrite_"     # n # "[_{d}]_vg1x2", "vm2",  t, MergeNone, i # "_vg1x2",   [IsInOutZA, IsStreaming], []>;
+  def NAME # _VG1x4 : Inst<"svwrite_"     # n # "[_{d}]_vg1x4", "vm4",  t, MergeNone, i # "_vg1x4",   [IsInOutZA, IsStreaming], []>;
 }
 
 let TargetGuard = "sme2" in {
@@ -322,12 +322,12 @@ let TargetGuard = "sme2" in {
 }
 
 multiclass ZARead_VG<string n, string t, string i, list<ImmCheck> checks> {
-  def NAME # _VG2_H : Inst<"svread_hor_" # n # "_{d}_vg2",   "2im", t, MergeNone, i # "_hor_vg2", [IsSharedZA, IsPreservesZA, IsStreaming], checks>;
-  def NAME # _VG2_V : Inst<"svread_ver_" # n # "_{d}_vg2",   "2im", t, MergeNone, i # "_ver_vg2", [IsSharedZA, IsPreservesZA, IsStreaming], checks>;
-  def NAME # _VG4_H : Inst<"svread_hor_" # n # "_{d}_vg4",   "4im", t, MergeNone, i # "_hor_vg4", [IsSharedZA, IsPreservesZA, IsStreaming], checks>;
-  def NAME # _VG4_V : Inst<"svread_ver_" # n # "_{d}_vg4",   "4im", t, MergeNone, i # "_ver_vg4", [IsSharedZA, IsPreservesZA, IsStreaming], checks>;
-  def NAME # _VG1x2 : Inst<"svread_"     # n # "_{d}_vg1x2", "2m",  t, MergeNone, i # "_vg1x2",   [IsSharedZA, IsPreservesZA, IsStreaming], []>;
-  def NAME # _VG1x4 : Inst<"svread_"     # n # "_{d}_vg1x4", "4m",  t, MergeNone, i # "_vg1x4",   [IsSharedZA, IsPreservesZA, IsStreaming], []>;
+  def NAME # _VG2_H : Inst<"svread_hor_" # n # "_{d}_vg2",   "2im", t, MergeNone, i # "_hor_vg2", [IsInZA, IsStreaming], checks>;
+  def NAME # _VG2_V : Inst<"svread_ver_" # n # "_{d}_vg2",   "2im", t, MergeNone, i # "_ver_vg2", [IsInZA, IsStreaming], checks>;
+  def NAME # _VG4_H : Inst<"svread_hor_" # n # "_{d}_vg4",   "...
[truncated]

def SVLDR_ZT : Inst<"svldr_zt", "viQ", "", MergeNone, "aarch64_sme_ldr_zt", [IsOverloadNone, IsStreamingCompatible, IsSharedZA], [ImmCheck<0, ImmCheck0_0>]>;
def SVSTR_ZT : Inst<"svstr_zt", "vi%", "", MergeNone, "aarch64_sme_str_zt", [IsOverloadNone, IsStreamingCompatible, IsSharedZA, IsPreservesZA], [ImmCheck<0, ImmCheck0_0>]>;
def SVLDR_ZT : Inst<"svldr_zt", "viQ", "", MergeNone, "aarch64_sme_ldr_zt", [IsOverloadNone, IsStreamingCompatible], [ImmCheck<0, ImmCheck0_0>]>;
def SVSTR_ZT : Inst<"svstr_zt", "vi%", "", MergeNone, "aarch64_sme_str_zt", [IsOverloadNone, IsStreamingCompatible], [ImmCheck<0, ImmCheck0_0>]>;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These builtins (and the ones below) have had the IsSharedZA & IsPreservesZA attributes removed, should they have been replaced with IsInOutZA or IsInZA?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a follow-up patch lined up that adds the IsOutZT0 (and related attributes), but thought I'd hold off until #77941 has landed.

def SVLDR_ZT : Inst<"svldr_zt", "viQ", "", MergeNone, "aarch64_sme_ldr_zt", [IsOverloadNone, IsStreamingCompatible, IsSharedZA], [ImmCheck<0, ImmCheck0_0>]>;
def SVSTR_ZT : Inst<"svstr_zt", "vi%", "", MergeNone, "aarch64_sme_str_zt", [IsOverloadNone, IsStreamingCompatible, IsSharedZA, IsPreservesZA], [ImmCheck<0, ImmCheck0_0>]>;
def SVLDR_ZT : Inst<"svldr_zt", "viQ", "", MergeNone, "aarch64_sme_ldr_zt", [IsOverloadNone, IsStreamingCompatible], [ImmCheck<0, ImmCheck0_0>]>;
def SVSTR_ZT : Inst<"svstr_zt", "vi%", "", MergeNone, "aarch64_sme_str_zt", [IsOverloadNone, IsStreamingCompatible], [ImmCheck<0, ImmCheck0_0>]>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why haven't this & these below been replaced with IsInZA? Is it because the zt0 follow-up is planned? It seems weird to just remove them with no replacement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's right. I have a follow-up patch lined up, but thought I'd hold off until #77941 has landed.

[ImmCheck<0, ImmCheck0_255>]>;
def SVZERO_ZA : SInst<"svzero_za", "v", "", MergeNone, "aarch64_sme_zero",
[IsOverloadNone, IsStreamingCompatible, IsSharedZA]>;
[IsOverloadNone, IsStreamingCompatible, IsOutZA]>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused about this change. IsOutZA seems to make sense since the instruction creates a new zeroed ZA state, but doesn't this also mean the old state is input and destroyed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SVZERO_MASK_ZA zeroes a ZA tile, not the whole ZA array. It takes part of ZA as input and zeroes only part of that, to return the combined result as output.

SVZERO_ZA only writes to ZA, because it zeroes the whole of ZA. It discards any values in ZA that came in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was the case. Thanks for clearing it up.


__arm_new("za")
svint8_t new_za(svint8_t zd, svbool_t pg, uint32_t slice_base) __arm_streaming {
return svread_hor_za8_s8_m(zd, pg, 0, slice_base); // no warning
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: every other test in this file does

// expected-no-warning
return sv...

instead of

return sv... // no warning

Comment on lines 1725 to 1738
std::string Key;
auto AddToKey = [&Key](std::string S) -> void {
Key = Key.empty() ? S : (Key + " | " + S);
};

if (Def->isFlagSet(getEnumValueForFlag("IsInZA")))
AddToKey("ArmInZA");
else if (Def->isFlagSet(getEnumValueForFlag("IsOutZA")))
AddToKey("ArmOutZA");
else if (Def->isFlagSet(getEnumValueForFlag("IsInOutZA")))
AddToKey("ArmInOutZA");

if (!Key.empty())
IntrinsicsPerState[Key].insert(Def->getMangledName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I reading this correctly? std::string Key; will create an empty string Key at the start of each for loop iteration. The only way to add to this string is via 3 else if's which means the Key.empty() check in the lambda will always be true and the else case will never fire.

Therefore you can just replace everything in this for loop with

    if (Def->isFlagSet(getEnumValueForFlag("IsInZA")))
      IntrinsicsPerState["ArmInZA"].insert(Def->getMangledName());
    else if (Def->isFlagSet(getEnumValueForFlag("IsOutZA")))
      IntrinsicsPerState["ArmOutZA"].insert(Def->getMangledName());
    else if (Def->isFlagSet(getEnumValueForFlag("IsInOutZA")))
      IntrinsicsPerState["ArmInOutZA"].insert(Def->getMangledName());

I'm assuming this is meant to be used to output or checks in clang/include/clang/Basic/arm_sme_builtins_za_state.inc like return ArmInZA | ArmOutZA? These aren't being emitted for me when I build this so I don't think this is doing what it was made for correctly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote this with the follow-up patch in mind, but I agree it doesn't make much sense without the follow-up. I'll simplify it.

FWIW, the idea was to support return ArmInZA | ArmOutZT0. ArmInZA/ArmOutZA/ArmInOutZA are all mutually exclusive, so we can't return ArmInZA | ArmOutZA.

bool HasZAState = Def->isFlagSet(IsSharedZAFlag);
DefsZAState[HasZAState].insert(Def->getMangledName());
std::string Key;
auto AddToKey = [&Key](std::string S) -> void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use const std::string& to pass string literals/read-only std::strings as parameters in C++ to avoid copying memory

ArmInZA = 0b01,
ArmOutZA = 0b10,
ArmInOutZA = 0b11,
ArmZAMask = 0b11,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ArmZAMask = 0b11,
ArmZAMask = ArmInOutZA,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really convinced that's better because they're two different things. One is an enum value for the state of ZA, and the other is a bitmask. If we'd want to change the encoding of the ZA attributes, we don't necessarily have to update the mask.

[ImmCheck<0, ImmCheck0_255>]>;
def SVZERO_ZA : SInst<"svzero_za", "v", "", MergeNone, "aarch64_sme_zero",
[IsOverloadNone, IsStreamingCompatible, IsSharedZA]>;
[IsOverloadNone, IsStreamingCompatible, IsOutZA]>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was the case. Thanks for clearing it up.

@sdesmalen-arm sdesmalen-arm merged commit 40a631f into llvm:main Jan 19, 2024
@sdesmalen-arm sdesmalen-arm deleted the sme-new-attributes-za-diagnostics-refactor branch January 19, 2024 16:02
rupprecht pushed a commit to rupprecht/llvm-project that referenced this pull request Jan 19, 2024
The arm_sme.td file was still using `IsSharedZA` and `IsPreservesZA`,
which should be changed to match the new state attributes added in
llvm#76971.

This patch adds `IsInZA`, `IsOutZA` and `IsInOutZA` as the state for the
Clang builtins and fixes up the code in SemaChecking and SveEmitter to
match.

Note that the code is written in such a way that it can be easily
extended with ZT0 state (to follow in a future patch).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants