[AArch64][SME] Add diagnostics for SME attributes on lambda functions #121777

kmclaughlin-arm · 2025-01-06T15:32:17Z

CheckFunctionDeclaration emits diagnostics if any SME attributes are used
by a function definition without the required +sme or +sme2 target features.
This patch adds moves these diagnostics to a new function in SemaARM and
also adds a call to this from ActOnStartOfLambdaDefinition.

CheckFunctionDeclaration emits diagnostics if any SME attributes are used by a function definition without the required +sme or +sme2 target features. This patch adds similar diagnostics to CheckConstexprFunctionDefinition to ensure this emits the same errors when attributes such as __arm_new("za") are found without +sme/+sme2.

llvmbot · 2025-01-06T16:07:26Z

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-clang

Author: Kerry McLaughlin (kmclaughlin-arm)

Changes

CheckFunctionDeclaration emits diagnostics if any SME attributes are used
by a function definition without the required +sme or +sme2 target features.
This patch adds moves these diagnostics to a new function in SemaARM and
also adds a call to this from CheckConstexprFunctionDefinition.

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

5 Files Affected:

(modified) clang/include/clang/Sema/SemaARM.h (+1)
(modified) clang/lib/Sema/SemaARM.cpp (+53)
(modified) clang/lib/Sema/SemaDecl.cpp (+3-52)
(modified) clang/lib/Sema/SemaDeclCXX.cpp (+4)
(modified) clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp (+7-1)

diff --git a/clang/include/clang/Sema/SemaARM.h b/clang/include/clang/Sema/SemaARM.h
index 8c4c56e2221301..08b03af7fd4bc6 100644
--- a/clang/include/clang/Sema/SemaARM.h
+++ b/clang/include/clang/Sema/SemaARM.h
@@ -79,6 +79,7 @@ class SemaARM : public SemaBase {
   void handleNewAttr(Decl *D, const ParsedAttr &AL);
   void handleCmseNSEntryAttr(Decl *D, const ParsedAttr &AL);
   void handleInterruptAttr(Decl *D, const ParsedAttr &AL);
+  void CheckSMEFunctionDefAttributes(const FunctionDecl *FD);
 };
 
 SemaARM::ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD);
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index 411baa066f7097..eafd43eb979ba0 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -1328,4 +1328,57 @@ void SemaARM::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
                  ARMInterruptAttr(getASTContext(), AL, Kind));
 }
 
+// Check if the function definition uses any AArch64 SME features without
+// having the '+sme' feature enabled and warn user if sme locally streaming
+// function returns or uses arguments with VL-based types.
+void SemaARM::CheckSMEFunctionDefAttributes(const FunctionDecl *FD) {
+  const auto *Attr = FD->getAttr<ArmNewAttr>();
+  bool UsesSM = FD->hasAttr<ArmLocallyStreamingAttr>();
+  bool UsesZA = Attr && Attr->isNewZA();
+  bool UsesZT0 = Attr && Attr->isNewZT0();
+
+  if (FD->hasAttr<ArmLocallyStreamingAttr>()) {
+    if (FD->getReturnType()->isSizelessVectorType())
+      Diag(FD->getLocation(),
+           diag::warn_sme_locally_streaming_has_vl_args_returns)
+          << /*IsArg=*/false;
+    if (llvm::any_of(FD->parameters(), [](ParmVarDecl *P) {
+          return P->getOriginalType()->isSizelessVectorType();
+        }))
+      Diag(FD->getLocation(),
+           diag::warn_sme_locally_streaming_has_vl_args_returns)
+          << /*IsArg=*/true;
+  }
+  if (const auto *FPT = FD->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;
+  }
+
+  ASTContext &Context = getASTContext();
+  if (UsesSM || UsesZA) {
+    llvm::StringMap<bool> FeatureMap;
+    Context.getFunctionFeatureMap(FeatureMap, FD);
+    if (!FeatureMap.contains("sme")) {
+      if (UsesSM)
+        Diag(FD->getLocation(),
+             diag::err_sme_definition_using_sm_in_non_sme_target);
+      else
+        Diag(FD->getLocation(),
+             diag::err_sme_definition_using_za_in_non_sme_target);
+    }
+  }
+  if (UsesZT0) {
+    llvm::StringMap<bool> FeatureMap;
+    Context.getFunctionFeatureMap(FeatureMap, FD);
+    if (!FeatureMap.contains("sme2")) {
+      Diag(FD->getLocation(),
+           diag::err_sme_definition_using_zt0_in_non_sme2_target);
+    }
+  }
+}
+
 } // namespace clang
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4001c4d263f1d2..1603b622a4faf0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -45,6 +45,7 @@
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/SemaARM.h"
 #include "clang/Sema/SemaCUDA.h"
 #include "clang/Sema/SemaHLSL.h"
 #include "clang/Sema/SemaInternal.h"
@@ -12286,58 +12287,8 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     }
   }
 
-  // Check if the function definition uses any AArch64 SME features without
-  // having the '+sme' feature enabled and warn user if sme locally streaming
-  // function returns or uses arguments with VL-based types.
-  if (DeclIsDefn) {
-    const auto *Attr = NewFD->getAttr<ArmNewAttr>();
-    bool UsesSM = NewFD->hasAttr<ArmLocallyStreamingAttr>();
-    bool UsesZA = Attr && Attr->isNewZA();
-    bool UsesZT0 = Attr && Attr->isNewZT0();
-
-    if (NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
-      if (NewFD->getReturnType()->isSizelessVectorType())
-        Diag(NewFD->getLocation(),
-             diag::warn_sme_locally_streaming_has_vl_args_returns)
-            << /*IsArg=*/false;
-      if (llvm::any_of(NewFD->parameters(), [](ParmVarDecl *P) {
-            return P->getOriginalType()->isSizelessVectorType();
-          }))
-        Diag(NewFD->getLocation(),
-             diag::warn_sme_locally_streaming_has_vl_args_returns)
-            << /*IsArg=*/true;
-    }
-    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) {
-      llvm::StringMap<bool> FeatureMap;
-      Context.getFunctionFeatureMap(FeatureMap, NewFD);
-      if (!FeatureMap.contains("sme")) {
-        if (UsesSM)
-          Diag(NewFD->getLocation(),
-               diag::err_sme_definition_using_sm_in_non_sme_target);
-        else
-          Diag(NewFD->getLocation(),
-               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);
-      }
-    }
-  }
+  if (DeclIsDefn && Context.getTargetInfo().getTriple().isAArch64())
+    ARM().CheckSMEFunctionDefAttributes(NewFD);
 
   return Redeclaration;
 }
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c5a72cf812ebc9..e599ff3731a314 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -41,6 +41,7 @@
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/SemaARM.h"
 #include "clang/Sema/SemaCUDA.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/SemaObjC.h"
@@ -1854,6 +1855,9 @@ bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
     }
   }
 
+  if (Context.getTargetInfo().getTriple().isAArch64())
+    ARM().CheckSMEFunctionDefAttributes(NewFD);
+
   // - each of its parameter types shall be a literal type; (removed in C++23)
   if (!getLangOpts().CPlusPlus23 &&
       !CheckConstexprParameterTypes(*this, NewFD, Kind))
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 ec6bb6f5035784..02ebfc8060f37e 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
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -std=c++23 -fsyntax-only -verify %s
 
 // This test is testing the diagnostics that Clang emits when compiling without '+sme'.
 
@@ -48,3 +48,9 @@ void streaming_compatible_def2(void (*streaming_fn_ptr)(void) __arm_streaming,
 // Also test when call-site is not a function.
 int streaming_decl_ret_int() __arm_streaming;
 int x = streaming_decl_ret_int(); // expected-error {{call to a streaming function requires 'sme'}}
+
+void sme_attrs_lambdas() {
+  [&] __arm_locally_streaming () { return; }();  // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
+  [&] __arm_new("za") () { return; }();  // expected-error {{function using ZA state requires 'sme'}}
+  [&] __arm_new("zt0") () { return; }();  // expected-error {{function using ZT0 state requires 'sme2'}}
+}

sdesmalen-arm · 2025-01-06T16:13:38Z

clang/lib/Sema/SemaARM.cpp

+// Check if the function definition uses any AArch64 SME features without
+// having the '+sme' feature enabled and warn user if sme locally streaming
+// function returns or uses arguments with VL-based types.
+void SemaARM::CheckSMEFunctionDefAttributes(const FunctionDecl *FD) {


Should this be a static member function instead? (would have to take the ASTContext as parameter)

I've changed this to static and added Sema as a parameter, so that the function can still use Diag() for emitting the diagnostics.

It's a bit inconsistent with everything else, the intent is to use ARM().CheckSMEFunctionDefAttributes(FD)
(The reason we did that was to split Sema into multiple classes/files)

@Endilll

It seems like an ARM-specific function, so you correctly placed it in SemaARM, and it's referenced in SemaDecl and SemaDeclCXX, so it can't be static. Corentin is correct that ARM().CheckSMEFunctionDefAttributes(FD) is the way to use it from outside of SemaARM.

The current state of the changes seems correct to me.

Thanks for your feedback, I didn't realise that there was already precedent for using ARM().Check...

sdesmalen-arm · 2025-01-06T16:14:29Z

clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -std=c++23 -fsyntax-only -verify %s


What about this change requires c++23?

I think adding attributes to lambdas before the parameter list was only allowed in C++23. If I don't pass this flag, the error emitted is instead "'attribute' in this position is a C++23 extension".

sdesmalen-arm · 2025-01-06T16:15:09Z

clang/test/Sema/aarch64-sme-func-attrs-without-target-feature.cpp

 int x = streaming_decl_ret_int(); // expected-error {{call to a streaming function requires 'sme'}}
+
+void sme_attrs_lambdas() {
+  [&] __arm_locally_streaming () { return; }();  // expected-error {{function executed in streaming-SVE mode requires 'sme'}}


nit: the function doesn't use anything from the parent's scope.

Suggested change

[&] __arm_locally_streaming () { return; }(); // expected-error {{function executed in streaming-SVE mode requires 'sme'}}

[] __arm_locally_streaming () { return; }(); // expected-error {{function executed in streaming-SVE mode requires 'sme'}}

clang/lib/Sema/SemaARM.cpp

cor3ntin · 2025-01-06T18:35:15Z

clang/lib/Sema/SemaDeclCXX.cpp

+  if (Context.getTargetInfo().getTriple().isAArch64())
+    ARM().CheckSMEFunctionDefAttributes(NewFD);
+


This seems like a weird place to do that (afaict this has nothing to do with constexpr). I suspect ActOnStartOfLambdaDefinition - after the ProcessDeclAttributes call might be a better place

Thanks, ActOnStartOfLambdaDefinition does seem like a better place to check the attributes.

- Move CheckSMEFunctionDefAttributes call to ActOnStartOfLambdaDefinition

sdesmalen-arm · 2025-01-09T14:10:51Z

clang/lib/Sema/SemaARM.cpp

+// Check if the function definition uses any AArch64 SME features without
+// having the '+sme' feature enabled and warn user if sme locally streaming
+// function returns or uses arguments with VL-based types.
+void SemaARM::CheckSMEFunctionDefAttributes(const FunctionDecl *FD) {


Thanks for your feedback, I didn't realise that there was already precedent for using ARM().Check...

llvm-ci · 2025-01-10T10:06:04Z

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-linux running on avx512-intel64 while building clang at step 12 "setup lit".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/9473

Here is the relevant piece of the build log for the reference

Step 12 (setup lit) failure: '/localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/bin/python /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/lnt/setup.py ...' (failure)
...

        See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
        ********************************************************************************

!!
  self.initialize_options()
zip_safe flag not set; analyzing archive contents...
Adding python-gnupg 0.3.7 to easy-install.pth file
detected new path './pytz-2016.10-py3.9.egg'

Installed /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/lib64/python3.9/site-packages/python_gnupg-0.3.7-py3.9.egg
Searching for itsdangerous==0.24
Reading https://pypi.org/simple/itsdangerous/
Downloading https://files.pythonhosted.org/packages/dc/b4/a60bcdba945c00f6d608d8975131ab3f25b22f2bcfe1dab221165194b2d4/itsdangerous-0.24.tar.gz#sha256=cbb3fcf8d3e33df861709ecaf89d9e6629cff0a217bc2848f1b41cd30d360519
Best match: itsdangerous 0.24
Processing itsdangerous-0.24.tar.gz
Writing /tmp/easy_install-ga32lss_/itsdangerous-0.24/setup.cfg
Running itsdangerous-0.24/setup.py -q bdist_egg --dist-dir /tmp/easy_install-ga32lss_/itsdangerous-0.24/egg-dist-tmp-6f5fns26
warning: no previously-included files matching '*' found under directory 'docs/_build'
/localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/lib/python3.9/site-packages/setuptools/_distutils/cmd.py:66: SetuptoolsDeprecationWarning: setup.py install is deprecated.
!!

        ********************************************************************************
        Please avoid running ``setup.py`` directly.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html for details.
        ********************************************************************************

!!
  self.initialize_options()
Adding itsdangerous 0.24 to easy-install.pth file
detected new path './python_gnupg-0.3.7-py3.9.egg'

Installed /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/lib64/python3.9/site-packages/itsdangerous-0.24-py3.9.egg
Searching for Werkzeug==0.15.6
Reading https://pypi.org/simple/Werkzeug/
Downloading https://files.pythonhosted.org/packages/b7/61/c0a1adf9ad80db012ed7191af98fa05faa95fa09eceb71bb6fa8b66e6a43/Werkzeug-0.15.6-py2.py3-none-any.whl#sha256=00d32beac38fcd48d329566f80d39f10ec2ed994efbecfb8dd4b320062d05902
Best match: Werkzeug 0.15.6
Processing Werkzeug-0.15.6-py2.py3-none-any.whl
Installing Werkzeug-0.15.6-py2.py3-none-any.whl to /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/lib64/python3.9/site-packages
Adding Werkzeug 0.15.6 to easy-install.pth file
detected new path './itsdangerous-0.24-py3.9.egg'

Installed /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/test/sandbox/lib64/python3.9/site-packages/Werkzeug-0.15.6-py3.9.egg
Searching for SQLAlchemy==1.2.19
Reading https://pypi.org/simple/SQLAlchemy/
Downloading https://files.pythonhosted.org/packages/f9/67/d07cf7ac7e6dd0bc55ba62816753f86d7c558107104ca915e730c9ec2512/SQLAlchemy-1.2.19.tar.gz#sha256=5bb2c4fc2bcc3447ad45716c66581eab982c007dcf925482498d8733f86f17c7
error: [Errno 104] Connection reset by peer

…llvm#121777) CheckFunctionDeclaration emits diagnostics if any SME attributes are used by a function definition without the required +sme or +sme2 target features. This patch moves these diagnostics to a new function in SemaARM and also adds a call to this from ActOnStartOfLambdaDefinition.

kmclaughlin-arm added 2 commits January 6, 2025 15:22

- Move diagnostics to SemaARM.cpp

bbc80c2

kmclaughlin-arm marked this pull request as ready for review January 6, 2025 16:06

kmclaughlin-arm requested a review from MacDue January 6, 2025 16:07

llvmbot added clang Clang issues not falling into any other category backend:ARM backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 6, 2025

kmclaughlin-arm requested a review from sdesmalen-arm January 6, 2025 16:07

sdesmalen-arm reviewed Jan 6, 2025

View reviewed changes

MacDue reviewed Jan 6, 2025

View reviewed changes

clang/lib/Sema/SemaARM.cpp Show resolved Hide resolved

cor3ntin reviewed Jan 6, 2025

View reviewed changes

- Mark CheckSMEFunctionDefAttributes as static

02658f7

- Move CheckSMEFunctionDefAttributes call to ActOnStartOfLambdaDefinition

kmclaughlin-arm changed the title ~~[AArch64][SME] Add diagnostics to CheckConstexprFunctionDefinition~~ [AArch64][SME] Add diagnostics for SME attributes on lambda functions Jan 7, 2025

- Removed static from CheckSMEFunctionDefAttributes

d47933c

sdesmalen-arm approved these changes Jan 9, 2025

View reviewed changes

kmclaughlin-arm merged commit 4e32271 into llvm:main Jan 10, 2025
8 checks passed

kmclaughlin-arm deleted the sme-decl-attrs branch February 6, 2025 13:48

		@@ -1,4 +1,4 @@
		// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -fsyntax-only -verify %s
		// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -std=c++23 -fsyntax-only -verify %s

	[&] __arm_locally_streaming () { return; }(); // expected-error {{function executed in streaming-SVE mode requires 'sme'}}
	[] __arm_locally_streaming () { return; }(); // expected-error {{function executed in streaming-SVE mode requires 'sme'}}

		if (Context.getTargetInfo().getTriple().isAArch64())
		ARM().CheckSMEFunctionDefAttributes(NewFD);

[AArch64][SME] Add diagnostics for SME attributes on lambda functions #121777

[AArch64][SME] Add diagnostics for SME attributes on lambda functions #121777

Uh oh!

Conversation

kmclaughlin-arm commented Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cor3ntin Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jan 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kmclaughlin-arm commented Jan 6, 2025 •

edited

Loading

llvmbot commented Jan 6, 2025 •

edited

Loading

cor3ntin Jan 6, 2025 •

edited

Loading