Skip to content
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

[Clang] Amend SME attributes with support for ZT0. #77941

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

sdesmalen-arm
Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm commented Jan 12, 2024

This patch builds on top of #76971 and implements support for:

  • __arm_new("zt0")
  • __arm_in("zt0")
  • __arm_out("zt0")
  • __arm_inout("zt0")
  • __arm_preserves("zt0")

The patch is ready for review but won't be able to land until LLVM
implements support for handling ZT0 state.

Copy link

github-actions bot commented Jan 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@rsandifo-arm
Copy link
Collaborator

LGTM from a spec point of view, although it might be worth having some tests that have different ZA state from ZT0. (But maybe not, since the code to handle ZA and ZT0 is mostly orthogonal.)

I won't approve because of the growth in FunctionProtoType — someone more qualified than me should sign off on that.

sdesmalen-arm added a commit that referenced this pull request Jan 15, 2024
#76971)

This patch replaces the `__arm_new_za`, `__arm_shared_za` and
`__arm_preserves_za` attributes in favour of:
* `__arm_new("za")`
* `__arm_in("za")`
* `__arm_out("za")`
* `__arm_inout("za")`
* `__arm_preserves("za")`

As described in ARM-software/acle#276.

One change is that `__arm_in/out/inout/preserves(S)` are all mutually
exclusive, whereas previously it was fine to write `__arm_shared_za
__arm_preserves_za`. This case is now represented with `__arm_in("za")`.

The current implementation uses the same LLVM attributes under the hood,
since `__arm_in/out/inout` are all variations of "shared ZA", so can use
the existing `aarch64_pstate_za_shared` attribute in LLVM.

#77941 will add support for the new "zt0" state as introduced
with SME2.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Jan 15, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-clang-codegen

Author: Sander de Smalen (sdesmalen-arm)

Changes

This patch builds on top of #76971 and implements support for:

  • __arm_new("zt0")
  • __arm_in("zt0")
  • __arm_out("zt0")
  • __arm_inout("zt0")
  • __arm_preserves("zt0")

I'll rebase this patch after I land #76971, as this is currently a
stacked pull-request on top of #76971.

The patch is ready for review but won't be able to land until LLVM
implements support for handling ZT0 state.


Full diff: https://github.com/llvm/llvm-project/pull/77941.diff

13 Files Affected:

  • (modified) clang/include/clang/AST/Type.h (+11-4)
  • (modified) clang/include/clang/Basic/Attr.td (+3)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
  • (modified) clang/lib/AST/TypePrinter.cpp (+8)
  • (modified) clang/lib/CodeGen/CGCall.cpp (+10)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+22)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+11)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+8)
  • (modified) clang/lib/Sema/SemaType.cpp (+3)
  • (added) clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp (+57)
  • (modified) clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp (+2)
  • (modified) clang/test/Sema/aarch64-sme-func-attrs.c (+45)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a7efe78591635e0..a56401f9da0450b 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -4040,9 +4040,12 @@ class FunctionType : public Type {
     // Describes the value of the state using ArmStateValue.
     SME_ZAShift = 2,
     SME_ZAMask = 0b111 << SME_ZAShift,
+    SME_ZT0Shift = 5,
+    SME_ZT0Mask = 0b111 << SME_ZT0Shift,
 
-    SME_AttributeMask = 0b111'111 // We only support maximum 6 bits because of
-                                  // the bitmask in FunctionTypeExtraBitfields.
+    SME_AttributeMask =
+        0b111'111'11 // We can't support more than 8 bits because of
+                     // the bitmask in FunctionTypeExtraBitfields.
   };
 
   enum ArmStateValue : unsigned {
@@ -4057,6 +4060,10 @@ class FunctionType : public Type {
     return (ArmStateValue)((AttrBits & SME_ZAMask) >> SME_ZAShift);
   }
 
+  static ArmStateValue getArmZT0State(unsigned AttrBits) {
+    return (ArmStateValue)((AttrBits & SME_ZT0Mask) >> SME_ZT0Shift);
+  }
+
   /// A simple holder for various uncommon bits which do not fit in
   /// FunctionTypeBitfields. Aligned to alignof(void *) to maintain the
   /// alignment of subsequent objects in TrailingObjects.
@@ -4068,7 +4075,7 @@ class FunctionType : public Type {
 
     /// Any AArch64 SME ACLE type attributes that need to be propagated
     /// on declarations and function pointers.
-    unsigned AArch64SMEAttributes : 6;
+    unsigned AArch64SMEAttributes : 8;
     FunctionTypeExtraBitfields()
         : NumExceptionType(0), AArch64SMEAttributes(SME_NormalFunction) {}
   };
@@ -4253,7 +4260,7 @@ class FunctionProtoType final
     FunctionType::ExtInfo ExtInfo;
     unsigned Variadic : 1;
     unsigned HasTrailingReturn : 1;
-    unsigned AArch64SMEAttributes : 6;
+    unsigned AArch64SMEAttributes : 8;
     Qualifiers TypeQuals;
     RefQualifierKind RefQualifier = RQ_None;
     ExceptionSpecInfo ExceptionSpec;
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b9ec720dd9e199c..174d4e14bfa0ecb 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2549,6 +2549,9 @@ def ArmNew : InheritableAttr, TargetSpecificAttr<TargetAArch64> {
     bool isNewZA() const {
       return llvm::is_contained(newArgs(), "za");
     }
+    bool isNewZT0() const {
+      return llvm::is_contained(newArgs(), "zt0");
+    }
   }];
 }
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c50b188a1039acb..cca4a4419d0bf94 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3696,10 +3696,14 @@ def err_sme_call_in_non_sme_target : Error<
   "call to a streaming function requires 'sme'">;
 def err_sme_za_call_no_za_state : Error<
   "call to a shared ZA function requires the caller to have ZA state">;
+def err_sme_zt0_call_no_zt0_state : Error<
+  "call to a shared ZT0 function requires the caller to have ZT0 state">;
 def err_sme_definition_using_sm_in_non_sme_target : Error<
   "function executed in streaming-SVE mode requires 'sme'">;
 def err_sme_definition_using_za_in_non_sme_target : Error<
   "function using ZA state requires 'sme'">;
+def err_sme_definition_using_zt0_in_non_sme2_target : Error<
+  "function using ZT0 state requires 'sme2'">;
 def err_conflicting_attributes_arm_state : Error<
   "conflicting attributes for state '%0'">;
 def err_unknown_arm_state : Error<
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 1baf895ebaec2cd..80b42c8f84a00ae 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -951,6 +951,14 @@ void TypePrinter::printFunctionProtoAfter(const FunctionProtoType *T,
     OS << " __arm_out(\"za\")";
   if (FunctionType::getArmZAState(SMEBits) == FunctionType::ARM_InOut)
     OS << " __arm_inout(\"za\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Preserves)
+    OS << " __arm_preserves(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_In)
+    OS << " __arm_in(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Out)
+    OS << " __arm_out(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_InOut)
+    OS << " __arm_inout(\"zt0\")";
 
   printFunctionAfter(Info, OS);
 
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index acf6cbad1c74809..0f2c80974b1f353 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1782,6 +1782,16 @@ static void AddAttributesFromFunctionProtoType(ASTContext &Ctx,
     FuncAttrs.addAttribute("aarch64_pstate_za_shared");
     FuncAttrs.addAttribute("aarch64_pstate_za_preserved");
   }
+
+  // ZT0
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Preserves)
+    FuncAttrs.addAttribute("aarch64_zt0_preserved");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_In)
+    FuncAttrs.addAttribute("aarch64_zt0_in");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Out)
+    FuncAttrs.addAttribute("aarch64_zt0_out");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_InOut)
+    FuncAttrs.addAttribute("aarch64_zt0_inout");
 }
 
 static void AddAttributesFromAssumes(llvm::AttrBuilder &FuncAttrs,
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 01b042ce5dd1343..7c7b825ae3a6b9a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2383,6 +2383,8 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   if (auto *Attr = D->getAttr<ArmNewAttr>()) {
     if (Attr->isNewZA())
       B.addAttribute("aarch64_pstate_za_new");
+    if (Attr->isNewZT0())
+      B.addAttribute("aarch64_zt0_new");
   }
 
   // Track whether we need to add the optnone LLVM attribute,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ace3e386988f005..02d6f1b4d2586b3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7544,6 +7544,28 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
       if (!CallerHasZAState)
         Diag(Loc, diag::err_sme_za_call_no_za_state);
     }
+
+    // If the callee uses AArch64 SME ZT0 state but the caller doesn't define
+    // any, then this is an error.
+    FunctionType::ArmStateValue ArmZT0State =
+        FunctionType::getArmZT0State(ExtInfo.AArch64SMEAttributes);
+    if (ArmZT0State != FunctionType::ARM_None) {
+      bool CallerHasZT0State = false;
+      if (const auto *CallerFD = dyn_cast<FunctionDecl>(CurContext)) {
+        auto *Attr = CallerFD->getAttr<ArmNewAttr>();
+        if (Attr && Attr->isNewZT0())
+          CallerHasZT0State = true;
+        else if (const auto *FPT =
+                     CallerFD->getType()->getAs<FunctionProtoType>())
+          CallerHasZT0State =
+              FunctionType::getArmZT0State(
+                  FPT->getExtProtoInfo().AArch64SMEAttributes) !=
+              FunctionType::ARM_None;
+      }
+
+      if (!CallerHasZT0State)
+        Diag(Loc, diag::err_sme_zt0_call_no_zt0_state);
+    }
   }
 
   if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4e7049571eeb7a3..e2377bb986195eb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12182,12 +12182,15 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     const auto *Attr = NewFD->getAttr<ArmNewAttr>();
     bool UsesSM = NewFD->hasAttr<ArmLocallyStreamingAttr>();
     bool UsesZA = Attr && Attr->isNewZA();
+    bool UsesZT0 = Attr && Attr->isNewZT0();
     if (const auto *FPT = NewFD->getType()->getAs<FunctionProtoType>()) {
       FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
       UsesSM |=
           EPI.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask;
       UsesZA |= FunctionType::getArmZAState(EPI.AArch64SMEAttributes) !=
                 FunctionType::ARM_None;
+      UsesZT0 |= FunctionType::getArmZT0State(EPI.AArch64SMEAttributes) !=
+                 FunctionType::ARM_None;
     }
 
     if (UsesSM || UsesZA) {
@@ -12202,6 +12205,14 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
                diag::err_sme_definition_using_za_in_non_sme_target);
       }
     }
+    if (UsesZT0) {
+      llvm::StringMap<bool> FeatureMap;
+      Context.getFunctionFeatureMap(FeatureMap, NewFD);
+      if (!FeatureMap.contains("sme2")) {
+        Diag(NewFD->getLocation(),
+             diag::err_sme_definition_using_zt0_in_non_sme2_target);
+      }
+    }
   }
 
   return Redeclaration;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 7e6881049d8d953..946c4503e20b898 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8988,6 +8988,7 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   bool HasZA = false;
+  bool HasZT0 = false;
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
     StringRef StateName;
     SourceLocation LiteralLoc;
@@ -8996,6 +8997,8 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
     if (StateName == "za")
       HasZA = true;
+    else if (StateName == "zt0")
+      HasZT0 = true;
     else {
       S.Diag(LiteralLoc, diag::err_unknown_arm_state) << StateName;
       AL.setInvalid();
@@ -9014,6 +9017,11 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     if (HasZA && ZAState != FunctionType::ARM_None &&
         checkArmNewAttrMutualExclusion(S, AL, FPT, ZAState, "za"))
       return;
+    FunctionType::ArmStateValue ZT0State =
+        FunctionType::getArmZT0State(FPT->getAArch64SMEAttributes());
+    if (HasZT0 && ZT0State != FunctionType::ARM_None &&
+        checkArmNewAttrMutualExclusion(S, AL, FPT, ZT0State, "zt0"))
+      return;
   }
 
   D->dropAttr<ArmNewAttr>();
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 78702b41ab82005..26b5d7da59b0a6e 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7899,6 +7899,9 @@ static bool handleArmStateAttribute(Sema &S,
     if (StateName == "za") {
       Shift = FunctionType::SME_ZAShift;
       ExistingState = FunctionType::getArmZAState(EPI.AArch64SMEAttributes);
+    } else if (StateName == "zt0") {
+      Shift = FunctionType::SME_ZT0Shift;
+      ExistingState = FunctionType::getArmZT0State(EPI.AArch64SMEAttributes);
     } else {
       S.Diag(LiteralLoc, diag::err_unknown_arm_state) << StateName;
       Attr.setInvalid();
diff --git a/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp b/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp
new file mode 100644
index 000000000000000..f0a5ea265920ced
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 \
+// RUN:   -S -disable-O0-optnone -Werror -emit-llvm -o - %s \
+// RUN: | opt -S -passes=mem2reg \
+// RUN: | opt -S -passes=inline \
+// RUN: | FileCheck %s
+
+// Test the attributes for ZT0 and their mappings to LLVM IR.
+
+extern "C" {
+
+// CHECK-LABEL: @in_zt0()
+// CHECK-SAME: #[[ZT0_IN:[0-9]+]]
+void in_zt0() __arm_in("zt0") { }
+
+// CHECK-LABEL: @out_zt0()
+// CHECK-SAME: #[[ZT0_OUT:[0-9]+]]
+void out_zt0() __arm_out("zt0") { }
+
+// CHECK-LABEL: @inout_zt0()
+// CHECK-SAME: #[[ZT0_INOUT:[0-9]+]]
+void inout_zt0() __arm_inout("zt0") { }
+
+// CHECK-LABEL: @preserves_zt0()
+// CHECK-SAME: #[[ZT0_PRESERVED:[0-9]+]]
+void preserves_zt0() __arm_preserves("zt0") { }
+
+// CHECK-LABEL: @new_zt0()
+// CHECK-SAME: #[[ZT0_NEW:[0-9]+]]
+__arm_new("zt0") void new_zt0() { }
+
+// CHECK-LABEL: @in_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_IN:[0-9]+]]
+void in_za_zt0() __arm_in("za", "zt0") { }
+
+// CHECK-LABEL: @out_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_OUT:[0-9]+]]
+void out_za_zt0() __arm_out("za", "zt0") { }
+
+// CHECK-LABEL: @inout_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_INOUT:[0-9]+]]
+void inout_za_zt0() __arm_inout("za", "zt0") { }
+
+// CHECK-LABEL: @preserves_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_PRESERVED:[0-9]+]]
+void preserves_za_zt0() __arm_preserves("za", "zt0") { }
+
+// CHECK-LABEL: @new_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_NEW:[0-9]+]]
+__arm_new("za", "zt0") void new_za_zt0() { }
+
+}
+
+// CHECK: attributes #[[ZT0_IN]] = {{{.*}} "aarch64_zt0_in" {{.*}}}
+// CHECK: attributes #[[ZT0_OUT]] = {{{.*}} "aarch64_zt0_out" {{.*}}}
+// CHECK: attributes #[[ZT0_INOUT]] = {{{.*}} "aarch64_zt0_inout" {{.*}}}
+// CHECK: attributes #[[ZT0_PRESERVED]] = {{{.*}} "aarch64_zt0_preserved" {{.*}}}
+// CHECK: attributes #[[ZT0_NEW]] = {{{.*}} "aarch64_zt0_new" {{.*}}}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp b/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
index 0a54a94f408b795..ec6bb6f5035784f 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
+++ b/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
@@ -8,6 +8,8 @@ void shared_za_def() __arm_inout("za") { } // expected-error {{function using ZA
 __arm_new("za") void new_za_def() { } // expected-error {{function using ZA state requires 'sme'}}
 __arm_locally_streaming void locally_streaming_def() { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
 void streaming_shared_za_def() __arm_streaming __arm_inout("za") { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+void inout_za_def() __arm_inout("za") { } // expected-error {{function using ZA state requires 'sme'}}
+void inout_zt0_def() __arm_inout("zt0") { } // expected-error {{function using ZT0 state requires 'sme2'}}
 
 // It should work fine when we explicitly add the target("sme") attribute.
 __attribute__((target("sme"))) void streaming_compatible_def_sme_attr() __arm_streaming_compatible {} // OK
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index b986b0b3de2e14d..b8a1d91a4b7e093 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -353,6 +353,11 @@ void invalid_arm_in_unknown_state(void) __arm_in("unknownstate");
 
 void valid_state_attrs_in_in1(void) __arm_in("za");
 void valid_state_attrs_in_in2(void) __arm_in("za", "za");
+void valid_state_attrs_in_in3(void) __arm_in("zt0");
+void valid_state_attrs_in_in4(void) __arm_in("zt0", "zt0");
+void valid_state_attrs_in_in5(void) __arm_in("za", "zt0");
+__arm_new("za") void valid_state_attrs_in_in6(void) __arm_in("zt0");
+__arm_new("zt0") void valid_state_attrs_in_in7(void) __arm_in("za");
 
 // expected-cpp-error@+2 {{missing state for '__arm_in'}}
 // expected-error@+1 {{missing state for '__arm_in'}}
@@ -400,3 +405,43 @@ void conflicting_state_attrs_preserves_out(void) __arm_preserves("za") __arm_out
 // expected-cpp-error@+2 {{conflicting attributes for state 'za'}}
 // expected-error@+1 {{conflicting attributes for state 'za'}}
 void conflicting_state_attrs_preserves_inout(void) __arm_preserves("za") __arm_inout("za");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_out(void) __arm_in("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_inout(void) __arm_in("zt0") __arm_inout("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_preserves(void) __arm_in("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_in(void) __arm_out("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_inout(void) __arm_out("zt0") __arm_inout("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_preserves(void) __arm_out("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_in(void) __arm_inout("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_out(void) __arm_inout("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_preserves(void) __arm_inout("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_in(void) __arm_preserves("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_out(void) __arm_preserves("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_inout(void) __arm_preserves("zt0") __arm_inout("zt0");

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-clang

Author: Sander de Smalen (sdesmalen-arm)

Changes

This patch builds on top of #76971 and implements support for:

  • __arm_new("zt0")
  • __arm_in("zt0")
  • __arm_out("zt0")
  • __arm_inout("zt0")
  • __arm_preserves("zt0")

I'll rebase this patch after I land #76971, as this is currently a
stacked pull-request on top of #76971.

The patch is ready for review but won't be able to land until LLVM
implements support for handling ZT0 state.


Full diff: https://github.com/llvm/llvm-project/pull/77941.diff

13 Files Affected:

  • (modified) clang/include/clang/AST/Type.h (+11-4)
  • (modified) clang/include/clang/Basic/Attr.td (+3)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4)
  • (modified) clang/lib/AST/TypePrinter.cpp (+8)
  • (modified) clang/lib/CodeGen/CGCall.cpp (+10)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+22)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+11)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+8)
  • (modified) clang/lib/Sema/SemaType.cpp (+3)
  • (added) clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp (+57)
  • (modified) clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp (+2)
  • (modified) clang/test/Sema/aarch64-sme-func-attrs.c (+45)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index a7efe78591635e0..a56401f9da0450b 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -4040,9 +4040,12 @@ class FunctionType : public Type {
     // Describes the value of the state using ArmStateValue.
     SME_ZAShift = 2,
     SME_ZAMask = 0b111 << SME_ZAShift,
+    SME_ZT0Shift = 5,
+    SME_ZT0Mask = 0b111 << SME_ZT0Shift,
 
-    SME_AttributeMask = 0b111'111 // We only support maximum 6 bits because of
-                                  // the bitmask in FunctionTypeExtraBitfields.
+    SME_AttributeMask =
+        0b111'111'11 // We can't support more than 8 bits because of
+                     // the bitmask in FunctionTypeExtraBitfields.
   };
 
   enum ArmStateValue : unsigned {
@@ -4057,6 +4060,10 @@ class FunctionType : public Type {
     return (ArmStateValue)((AttrBits & SME_ZAMask) >> SME_ZAShift);
   }
 
+  static ArmStateValue getArmZT0State(unsigned AttrBits) {
+    return (ArmStateValue)((AttrBits & SME_ZT0Mask) >> SME_ZT0Shift);
+  }
+
   /// A simple holder for various uncommon bits which do not fit in
   /// FunctionTypeBitfields. Aligned to alignof(void *) to maintain the
   /// alignment of subsequent objects in TrailingObjects.
@@ -4068,7 +4075,7 @@ class FunctionType : public Type {
 
     /// Any AArch64 SME ACLE type attributes that need to be propagated
     /// on declarations and function pointers.
-    unsigned AArch64SMEAttributes : 6;
+    unsigned AArch64SMEAttributes : 8;
     FunctionTypeExtraBitfields()
         : NumExceptionType(0), AArch64SMEAttributes(SME_NormalFunction) {}
   };
@@ -4253,7 +4260,7 @@ class FunctionProtoType final
     FunctionType::ExtInfo ExtInfo;
     unsigned Variadic : 1;
     unsigned HasTrailingReturn : 1;
-    unsigned AArch64SMEAttributes : 6;
+    unsigned AArch64SMEAttributes : 8;
     Qualifiers TypeQuals;
     RefQualifierKind RefQualifier = RQ_None;
     ExceptionSpecInfo ExceptionSpec;
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b9ec720dd9e199c..174d4e14bfa0ecb 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2549,6 +2549,9 @@ def ArmNew : InheritableAttr, TargetSpecificAttr<TargetAArch64> {
     bool isNewZA() const {
       return llvm::is_contained(newArgs(), "za");
     }
+    bool isNewZT0() const {
+      return llvm::is_contained(newArgs(), "zt0");
+    }
   }];
 }
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c50b188a1039acb..cca4a4419d0bf94 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3696,10 +3696,14 @@ def err_sme_call_in_non_sme_target : Error<
   "call to a streaming function requires 'sme'">;
 def err_sme_za_call_no_za_state : Error<
   "call to a shared ZA function requires the caller to have ZA state">;
+def err_sme_zt0_call_no_zt0_state : Error<
+  "call to a shared ZT0 function requires the caller to have ZT0 state">;
 def err_sme_definition_using_sm_in_non_sme_target : Error<
   "function executed in streaming-SVE mode requires 'sme'">;
 def err_sme_definition_using_za_in_non_sme_target : Error<
   "function using ZA state requires 'sme'">;
+def err_sme_definition_using_zt0_in_non_sme2_target : Error<
+  "function using ZT0 state requires 'sme2'">;
 def err_conflicting_attributes_arm_state : Error<
   "conflicting attributes for state '%0'">;
 def err_unknown_arm_state : Error<
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 1baf895ebaec2cd..80b42c8f84a00ae 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -951,6 +951,14 @@ void TypePrinter::printFunctionProtoAfter(const FunctionProtoType *T,
     OS << " __arm_out(\"za\")";
   if (FunctionType::getArmZAState(SMEBits) == FunctionType::ARM_InOut)
     OS << " __arm_inout(\"za\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Preserves)
+    OS << " __arm_preserves(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_In)
+    OS << " __arm_in(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Out)
+    OS << " __arm_out(\"zt0\")";
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_InOut)
+    OS << " __arm_inout(\"zt0\")";
 
   printFunctionAfter(Info, OS);
 
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index acf6cbad1c74809..0f2c80974b1f353 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1782,6 +1782,16 @@ static void AddAttributesFromFunctionProtoType(ASTContext &Ctx,
     FuncAttrs.addAttribute("aarch64_pstate_za_shared");
     FuncAttrs.addAttribute("aarch64_pstate_za_preserved");
   }
+
+  // ZT0
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Preserves)
+    FuncAttrs.addAttribute("aarch64_zt0_preserved");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_In)
+    FuncAttrs.addAttribute("aarch64_zt0_in");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_Out)
+    FuncAttrs.addAttribute("aarch64_zt0_out");
+  if (FunctionType::getArmZT0State(SMEBits) == FunctionType::ARM_InOut)
+    FuncAttrs.addAttribute("aarch64_zt0_inout");
 }
 
 static void AddAttributesFromAssumes(llvm::AttrBuilder &FuncAttrs,
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 01b042ce5dd1343..7c7b825ae3a6b9a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2383,6 +2383,8 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   if (auto *Attr = D->getAttr<ArmNewAttr>()) {
     if (Attr->isNewZA())
       B.addAttribute("aarch64_pstate_za_new");
+    if (Attr->isNewZT0())
+      B.addAttribute("aarch64_zt0_new");
   }
 
   // Track whether we need to add the optnone LLVM attribute,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ace3e386988f005..02d6f1b4d2586b3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7544,6 +7544,28 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
       if (!CallerHasZAState)
         Diag(Loc, diag::err_sme_za_call_no_za_state);
     }
+
+    // If the callee uses AArch64 SME ZT0 state but the caller doesn't define
+    // any, then this is an error.
+    FunctionType::ArmStateValue ArmZT0State =
+        FunctionType::getArmZT0State(ExtInfo.AArch64SMEAttributes);
+    if (ArmZT0State != FunctionType::ARM_None) {
+      bool CallerHasZT0State = false;
+      if (const auto *CallerFD = dyn_cast<FunctionDecl>(CurContext)) {
+        auto *Attr = CallerFD->getAttr<ArmNewAttr>();
+        if (Attr && Attr->isNewZT0())
+          CallerHasZT0State = true;
+        else if (const auto *FPT =
+                     CallerFD->getType()->getAs<FunctionProtoType>())
+          CallerHasZT0State =
+              FunctionType::getArmZT0State(
+                  FPT->getExtProtoInfo().AArch64SMEAttributes) !=
+              FunctionType::ARM_None;
+      }
+
+      if (!CallerHasZT0State)
+        Diag(Loc, diag::err_sme_zt0_call_no_zt0_state);
+    }
   }
 
   if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4e7049571eeb7a3..e2377bb986195eb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12182,12 +12182,15 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     const auto *Attr = NewFD->getAttr<ArmNewAttr>();
     bool UsesSM = NewFD->hasAttr<ArmLocallyStreamingAttr>();
     bool UsesZA = Attr && Attr->isNewZA();
+    bool UsesZT0 = Attr && Attr->isNewZT0();
     if (const auto *FPT = NewFD->getType()->getAs<FunctionProtoType>()) {
       FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
       UsesSM |=
           EPI.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask;
       UsesZA |= FunctionType::getArmZAState(EPI.AArch64SMEAttributes) !=
                 FunctionType::ARM_None;
+      UsesZT0 |= FunctionType::getArmZT0State(EPI.AArch64SMEAttributes) !=
+                 FunctionType::ARM_None;
     }
 
     if (UsesSM || UsesZA) {
@@ -12202,6 +12205,14 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
                diag::err_sme_definition_using_za_in_non_sme_target);
       }
     }
+    if (UsesZT0) {
+      llvm::StringMap<bool> FeatureMap;
+      Context.getFunctionFeatureMap(FeatureMap, NewFD);
+      if (!FeatureMap.contains("sme2")) {
+        Diag(NewFD->getLocation(),
+             diag::err_sme_definition_using_zt0_in_non_sme2_target);
+      }
+    }
   }
 
   return Redeclaration;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 7e6881049d8d953..946c4503e20b898 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8988,6 +8988,7 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   bool HasZA = false;
+  bool HasZT0 = false;
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
     StringRef StateName;
     SourceLocation LiteralLoc;
@@ -8996,6 +8997,8 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
     if (StateName == "za")
       HasZA = true;
+    else if (StateName == "zt0")
+      HasZT0 = true;
     else {
       S.Diag(LiteralLoc, diag::err_unknown_arm_state) << StateName;
       AL.setInvalid();
@@ -9014,6 +9017,11 @@ static void handleArmNewAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     if (HasZA && ZAState != FunctionType::ARM_None &&
         checkArmNewAttrMutualExclusion(S, AL, FPT, ZAState, "za"))
       return;
+    FunctionType::ArmStateValue ZT0State =
+        FunctionType::getArmZT0State(FPT->getAArch64SMEAttributes());
+    if (HasZT0 && ZT0State != FunctionType::ARM_None &&
+        checkArmNewAttrMutualExclusion(S, AL, FPT, ZT0State, "zt0"))
+      return;
   }
 
   D->dropAttr<ArmNewAttr>();
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 78702b41ab82005..26b5d7da59b0a6e 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7899,6 +7899,9 @@ static bool handleArmStateAttribute(Sema &S,
     if (StateName == "za") {
       Shift = FunctionType::SME_ZAShift;
       ExistingState = FunctionType::getArmZAState(EPI.AArch64SMEAttributes);
+    } else if (StateName == "zt0") {
+      Shift = FunctionType::SME_ZT0Shift;
+      ExistingState = FunctionType::getArmZT0State(EPI.AArch64SMEAttributes);
     } else {
       S.Diag(LiteralLoc, diag::err_unknown_arm_state) << StateName;
       Attr.setInvalid();
diff --git a/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp b/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp
new file mode 100644
index 000000000000000..f0a5ea265920ced
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-sme2-intrinsics/aarch64-sme2-attrs.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme2 \
+// RUN:   -S -disable-O0-optnone -Werror -emit-llvm -o - %s \
+// RUN: | opt -S -passes=mem2reg \
+// RUN: | opt -S -passes=inline \
+// RUN: | FileCheck %s
+
+// Test the attributes for ZT0 and their mappings to LLVM IR.
+
+extern "C" {
+
+// CHECK-LABEL: @in_zt0()
+// CHECK-SAME: #[[ZT0_IN:[0-9]+]]
+void in_zt0() __arm_in("zt0") { }
+
+// CHECK-LABEL: @out_zt0()
+// CHECK-SAME: #[[ZT0_OUT:[0-9]+]]
+void out_zt0() __arm_out("zt0") { }
+
+// CHECK-LABEL: @inout_zt0()
+// CHECK-SAME: #[[ZT0_INOUT:[0-9]+]]
+void inout_zt0() __arm_inout("zt0") { }
+
+// CHECK-LABEL: @preserves_zt0()
+// CHECK-SAME: #[[ZT0_PRESERVED:[0-9]+]]
+void preserves_zt0() __arm_preserves("zt0") { }
+
+// CHECK-LABEL: @new_zt0()
+// CHECK-SAME: #[[ZT0_NEW:[0-9]+]]
+__arm_new("zt0") void new_zt0() { }
+
+// CHECK-LABEL: @in_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_IN:[0-9]+]]
+void in_za_zt0() __arm_in("za", "zt0") { }
+
+// CHECK-LABEL: @out_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_OUT:[0-9]+]]
+void out_za_zt0() __arm_out("za", "zt0") { }
+
+// CHECK-LABEL: @inout_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_INOUT:[0-9]+]]
+void inout_za_zt0() __arm_inout("za", "zt0") { }
+
+// CHECK-LABEL: @preserves_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_PRESERVED:[0-9]+]]
+void preserves_za_zt0() __arm_preserves("za", "zt0") { }
+
+// CHECK-LABEL: @new_za_zt0()
+// CHECK-SAME: #[[ZA_ZT0_NEW:[0-9]+]]
+__arm_new("za", "zt0") void new_za_zt0() { }
+
+}
+
+// CHECK: attributes #[[ZT0_IN]] = {{{.*}} "aarch64_zt0_in" {{.*}}}
+// CHECK: attributes #[[ZT0_OUT]] = {{{.*}} "aarch64_zt0_out" {{.*}}}
+// CHECK: attributes #[[ZT0_INOUT]] = {{{.*}} "aarch64_zt0_inout" {{.*}}}
+// CHECK: attributes #[[ZT0_PRESERVED]] = {{{.*}} "aarch64_zt0_preserved" {{.*}}}
+// CHECK: attributes #[[ZT0_NEW]] = {{{.*}} "aarch64_zt0_new" {{.*}}}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp b/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
index 0a54a94f408b795..ec6bb6f5035784f 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
+++ b/clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp
@@ -8,6 +8,8 @@ void shared_za_def() __arm_inout("za") { } // expected-error {{function using ZA
 __arm_new("za") void new_za_def() { } // expected-error {{function using ZA state requires 'sme'}}
 __arm_locally_streaming void locally_streaming_def() { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
 void streaming_shared_za_def() __arm_streaming __arm_inout("za") { } // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+void inout_za_def() __arm_inout("za") { } // expected-error {{function using ZA state requires 'sme'}}
+void inout_zt0_def() __arm_inout("zt0") { } // expected-error {{function using ZT0 state requires 'sme2'}}
 
 // It should work fine when we explicitly add the target("sme") attribute.
 __attribute__((target("sme"))) void streaming_compatible_def_sme_attr() __arm_streaming_compatible {} // OK
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index b986b0b3de2e14d..b8a1d91a4b7e093 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -353,6 +353,11 @@ void invalid_arm_in_unknown_state(void) __arm_in("unknownstate");
 
 void valid_state_attrs_in_in1(void) __arm_in("za");
 void valid_state_attrs_in_in2(void) __arm_in("za", "za");
+void valid_state_attrs_in_in3(void) __arm_in("zt0");
+void valid_state_attrs_in_in4(void) __arm_in("zt0", "zt0");
+void valid_state_attrs_in_in5(void) __arm_in("za", "zt0");
+__arm_new("za") void valid_state_attrs_in_in6(void) __arm_in("zt0");
+__arm_new("zt0") void valid_state_attrs_in_in7(void) __arm_in("za");
 
 // expected-cpp-error@+2 {{missing state for '__arm_in'}}
 // expected-error@+1 {{missing state for '__arm_in'}}
@@ -400,3 +405,43 @@ void conflicting_state_attrs_preserves_out(void) __arm_preserves("za") __arm_out
 // expected-cpp-error@+2 {{conflicting attributes for state 'za'}}
 // expected-error@+1 {{conflicting attributes for state 'za'}}
 void conflicting_state_attrs_preserves_inout(void) __arm_preserves("za") __arm_inout("za");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_out(void) __arm_in("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_inout(void) __arm_in("zt0") __arm_inout("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_in_preserves(void) __arm_in("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_in(void) __arm_out("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_inout(void) __arm_out("zt0") __arm_inout("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_out_preserves(void) __arm_out("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_in(void) __arm_inout("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_out(void) __arm_inout("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_inout_preserves(void) __arm_inout("zt0") __arm_preserves("zt0");
+
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_in(void) __arm_preserves("zt0") __arm_in("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_out(void) __arm_preserves("zt0") __arm_out("zt0");
+// expected-cpp-error@+2 {{conflicting attributes for state 'zt0'}}
+// expected-error@+1 {{conflicting attributes for state 'zt0'}}
+void conflicting_state_attrs_preserves_inout(void) __arm_preserves("zt0") __arm_inout("zt0");

@sdesmalen-arm
Copy link
Collaborator Author

I won't approve because of the growth in FunctionProtoType — someone more qualified than me should sign off on that.

@AaronBallman or @erichkeane, would you be happy to sign off on this patch?

I think previously you raised concerns about the size of ExtProtoInfo when adding more bits to it. (This was discussed on https://reviews.llvm.org/D127762, but unfortunately this link currently results in a 404)

It's worth pointing out that extending ExtProtoInfo with some more bits doesn't change the actual size. It is 80 bytes at the moment and increasing it with 2 more bits doesn't change the total size of the struct. The size of FunctionTypeExtraBitfields will increase though, but I'm not sure if this is a problem because the comment above it says that these bits are uncommon to start with. I could shave off another two bits off NumExceptionType to avoid that, as the comment suggests 8 bits should be sufficient.

This also make me wonder if we should be reserving some extra bits for future state that may need adding?

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

The additional size of ExtProtoInfo and FunctionType are both, IMO, problematic, we should be shrinking these.

This also make me wonder if we should be reserving some extra bits for future state that may need adding?

The point is to minimize the size of these, since adding additional bits like this is limiting the code we can compile. The 'extra bits' were an attempt to organize, and minimize if 'none' were used (it is in trailing storage).

Why is the spec that this is from being so aggressive about modifying the types of functions? This is causing some pretty annoying amounts of memory pressure in the compiler.

IMO, we need to find some way to no longer store this information on the function type in a way that punishes every function.

clang/include/clang/AST/Type.h Show resolved Hide resolved
@sdesmalen-arm
Copy link
Collaborator Author

[rebased patch on top of #78424]

With the contentious part of adding new bits addressed in #78424 and @rsandifo-arm having reviewed and informally accepted the Arm/ZT0 side of the changes in this pull-request, are you happy to formally accept this PR @erichkeane / @AaronBallman?

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

There should be a release note for the new functionality, but otherwise the changes LGTM. Please wait a bit in case @erichkeane has any other concerns.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

I'm ok with this now.

This patch builds on top of llvm#76971 and implements support for:
* __arm_new("zt0")
* __arm_in("zt0")
* __arm_out("zt0")
* __arm_inout("zt0")
* __arm_preserves("zt0")

The patch is ready for review but won't be able to land until LLVM
implements support for handling ZT0 state.
Due to duplication of names, this test was failing.
Updated to match the implemented LLVM attribute names recognised by
the SMEAttrs class.
@sdesmalen-arm sdesmalen-arm merged commit 1652d44 into llvm:main Jan 23, 2024
3 of 4 checks passed
@RKSimon
Copy link
Collaborator

RKSimon commented Jan 23, 2024

@sdesmalen-arm This appears to be failing on some buildbots: https://lab.llvm.org/buildbot/#/builders/176/builds/8232

llvm-lit: /home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/llvm/utils/lit/lit/TestingConfig.py:152: fatal: unable to parse config file '/home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/stage2/runtimes/runtimes-bins/compiler-rt/unittests/lit.common.unit.configured', traceback: Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/llvm/llvm/utils/lit/lit/TestingConfig.py", line 140, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vls-2stage/stage2/runtimes/runtimes-bins/compiler-rt/unittests/lit.common.unit.configured", line 23
    config.aarch64_sme =
                        ^
SyntaxError: invalid syntax

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
llvm#76971)

This patch replaces the `__arm_new_za`, `__arm_shared_za` and
`__arm_preserves_za` attributes in favour of:
* `__arm_new("za")`
* `__arm_in("za")`
* `__arm_out("za")`
* `__arm_inout("za")`
* `__arm_preserves("za")`

As described in ARM-software/acle#276.

One change is that `__arm_in/out/inout/preserves(S)` are all mutually
exclusive, whereas previously it was fine to write `__arm_shared_za
__arm_preserves_za`. This case is now represented with `__arm_in("za")`.

The current implementation uses the same LLVM attributes under the hood,
since `__arm_in/out/inout` are all variations of "shared ZA", so can use
the existing `aarch64_pstate_za_shared` attribute in LLVM.

llvm#77941 will add support for the new "zt0" state as introduced
with SME2.
@sdesmalen-arm sdesmalen-arm deleted the sme-new-attributes-zt0 branch February 23, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen 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.

None yet

6 participants