[clang][SME] Account for C++ lambdas in SME builtin diagnostics #124750
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A C++ lambda does not inherit attributes from the parent function. So the SME builtin diagnostics should look at the lambda's attributes, not the parent function's. The fix is very simple and just adds the missing "AllowLambda" flag to the function decl lookups.
MacDue
requested review from
sdesmalen-arm,
efriedma-quic and
kmclaughlin-arm
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-arm Author: Benjamin Maxwell (MacDue) ChangesA C++ lambda does not inherit attributes from the parent function. So the SME builtin diagnostics should look at the lambda's attributes, not the parent function's. The fix is very simple and just adds the missing "AllowLambda" flag to the function decl lookups. Full diff: https://github.com/llvm/llvm-project/pull/124750.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index 2620bbc97ba02a..df865d1b7df8c1 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -650,7 +650,8 @@ static ArmSMEState getSMEState(unsigned BuiltinID) {
bool SemaARM::CheckSMEBuiltinFunctionCall(unsigned BuiltinID,
CallExpr *TheCall) {
- if (const FunctionDecl *FD = SemaRef.getCurFunctionDecl()) {
+ if (const FunctionDecl *FD =
+ SemaRef.getCurFunctionDecl(/*AllowLambda=*/true)) {
std::optional<ArmStreamingType> BuiltinType;
switch (BuiltinID) {
@@ -690,7 +691,8 @@ bool SemaARM::CheckSMEBuiltinFunctionCall(unsigned BuiltinID,
bool SemaARM::CheckSVEBuiltinFunctionCall(unsigned BuiltinID,
CallExpr *TheCall) {
- if (const FunctionDecl *FD = SemaRef.getCurFunctionDecl()) {
+ if (const FunctionDecl *FD =
+ SemaRef.getCurFunctionDecl(/*AllowLambda=*/true)) {
std::optional<ArmStreamingType> BuiltinType;
switch (BuiltinID) {
@@ -719,7 +721,8 @@ bool SemaARM::CheckSVEBuiltinFunctionCall(unsigned BuiltinID,
bool SemaARM::CheckNeonBuiltinFunctionCall(const TargetInfo &TI,
unsigned BuiltinID,
CallExpr *TheCall) {
- if (const FunctionDecl *FD = SemaRef.getCurFunctionDecl()) {
+ if (const FunctionDecl *FD =
+ SemaRef.getCurFunctionDecl(/*AllowLambda=*/true)) {
switch (BuiltinID) {
default:
diff --git a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.cpp b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.cpp
new file mode 100644
index 00000000000000..12ef7ad06b68b1
--- /dev/null
+++ b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -target-feature +sme -target-feature +neon -x c++ -std=c++20 -Waarch64-sme-attributes -fsyntax-only -verify %s
+
+// REQUIRES: aarch64-registered-target
+
+#include <arm_sme.h>
+#include <arm_neon.h>
+
+void use_streaming_builtin_in_lambda(uint32_t slice_base, svbool_t pg, const void *ptr) __arm_streaming __arm_out("za")
+{
+ [&]{
+ /// The lambda is its own function and does not inherit the SME attributes (so this should error).
+ // expected-error@+1 {{builtin can only be called from a streaming function}}
+ svld1_hor_za64(0, slice_base, pg, ptr);
+ }();
+}
+
+void use_streaming_builtin(uint32_t slice_base, svbool_t pg, const void *ptr) __arm_streaming __arm_out("za")
+{
+ /// Without the lambda the same builtin is okay (as the SME attributes apply).
+ svld1_hor_za64(0, slice_base, pg, ptr);
+}
+
+int16x8_t use_neon_builtin_sm(int16x8_t splat) __arm_streaming_compatible {
+ // expected-error@+1 {{builtin can only be called from a non-streaming function}}
+ return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
+}
+
+int16x8_t use_neon_builtin_sm_in_lambda(int16x8_t splat) __arm_streaming_compatible {
+ return [&]{
+ /// This should not error (as we switch out of streaming mode to execute the lambda).
+ /// Note: The result int16x8_t is spilled and reloaded as a q-register.
+ return (int16x8_t)__builtin_neon_vqaddq_v((int8x16_t)splat, (int8x16_t)splat, 33);
+ }();
+}
+
+float use_incomp_sve_builtin_sm() __arm_streaming {
+ // expected-error@+1 {{builtin can only be called from a non-streaming function}}
+ return svadda(svptrue_b32(), 0, svdup_f32(1));
+}
+
+float incomp_sve_sm_fadda_sm_in_lambda(void) __arm_streaming {
+ return [&]{
+ /// This should work like the Neon builtin.
+ return svadda(svptrue_b32(), 0, svdup_f32(1));
+ }();
+}
|
sdesmalen-arm
approved these changes
Jan 28, 2025
kmclaughlin-arm
approved these changes
Jan 28, 2025
|
Please add some tests for lambdas with streaming attributes, for example: |
Done 👍 |
efriedma-quic
approved these changes
Jan 29, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9620 Here is the relevant piece of the build log for the reference |
|
/cherry-pick 2b7509e |
|
/cherry-pick 2b7509e |
|
/pull-request #125049 |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 1, 2025
…#124750) A C++ lambda does not inherit attributes from the parent function. So the SME builtin diagnostics should look at the lambda's attributes, not the parent function's. The fix is very simple and just adds the missing "AllowLambda" flag to the function decl lookups. (cherry picked from commit 2b7509e)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A C++ lambda does not inherit attributes from the parent function. So the SME builtin diagnostics should look at the lambda's attributes, not the parent function's.
The fix is very simple and just adds the missing "AllowLambda" flag to the function decl lookups.