[clang] [clang][sema][FMV] Forbid multi-versioning arm_streaming functions. (PR #81268)
Jon Roelofs via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 12 07:45:21 PST 2024
https://github.com/jroelofs updated https://github.com/llvm/llvm-project/pull/81268
>From 82a631017a114a15a6ba3376c049b7a85707973a Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Thu, 8 Feb 2024 16:21:28 -0800
Subject: [PATCH 1/4] [clang][sema][FMV] Forbid multi-versioning arm_streaming
functions.
The streaming mode change is incompatible with the ifunc mechanism used to
implement FMV: we can't conditionally change it based on the particular callee
that is resolved at runtime.
Fixes: https://github.com/llvm/llvm-project/issues/80077
---
.../clang/Basic/DiagnosticSemaKinds.td | 2 ++
clang/include/clang/Sema/Sema.h | 13 ++++----
clang/lib/Sema/SemaDeclAttr.cpp | 24 +++++++++++---
clang/test/Sema/aarch64-sme-func-attrs.c | 31 +++++++++++++++++++
4 files changed, 58 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b4dc4feee8e63a..83b89d1449f420 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3728,6 +3728,8 @@ 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_sme_streaming_cannot_be_multiversioned : Error<
+ "streaming function cannot be multi-versioned">;
def err_unknown_arm_state : Error<
"unknown state '%0'">;
def err_missing_arm_state : Error<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3c26003b5bda7f..9930ca2eb370e5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4838,13 +4838,12 @@ class Sema final {
llvm::Error isValidSectionSpecifier(StringRef Str);
bool checkSectionName(SourceLocation LiteralLoc, StringRef Str);
bool checkTargetAttr(SourceLocation LiteralLoc, StringRef Str);
- bool checkTargetVersionAttr(SourceLocation LiteralLoc, StringRef &Str,
- bool &isDefault);
- bool
- checkTargetClonesAttrString(SourceLocation LiteralLoc, StringRef Str,
- const StringLiteral *Literal, bool &HasDefault,
- bool &HasCommas, bool &HasNotDefault,
- SmallVectorImpl<SmallString<64>> &StringsBuffer);
+ bool checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
+ StringRef &Str, bool &isDefault);
+ bool checkTargetClonesAttrString(
+ SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
+ Decl *D, bool &HasDefault, bool &HasCommas, bool &HasNotDefault,
+ SmallVectorImpl<SmallString<64>> &StringsBuffer);
bool checkMSInheritanceAttrOnDefinition(
CXXRecordDecl *RD, SourceRange Range, bool BestCase,
MSInheritanceModel SemanticSpelling);
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d785714c4d811e..2a7c11cd4351f4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3501,9 +3501,18 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
return false;
}
+static bool isArmStreaming(const FunctionDecl *FD) {
+ if (FD->hasAttr<ArmLocallyStreamingAttr>())
+ return true;
+ if (const auto *T = FD->getType()->getAs<FunctionProtoType>())
+ if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
+ return true;
+ return false;
+}
+
// Check Target Version attrs
-bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, StringRef &AttrStr,
- bool &isDefault) {
+bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
+ StringRef &AttrStr, bool &isDefault) {
enum FirstParam { Unsupported };
enum SecondParam { None };
enum ThirdParam { Target, TargetClones, TargetVersion };
@@ -3519,6 +3528,8 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, StringRef &AttrStr,
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << CurFeature << TargetVersion;
}
+ if (isArmStreaming(cast<FunctionDecl>(D)))
+ return Diag(LiteralLoc, diag::err_sme_streaming_cannot_be_multiversioned);
return false;
}
@@ -3527,7 +3538,7 @@ static void handleTargetVersionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
SourceLocation LiteralLoc;
bool isDefault = false;
if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc) ||
- S.checkTargetVersionAttr(LiteralLoc, Str, isDefault))
+ S.checkTargetVersionAttr(LiteralLoc, D, Str, isDefault))
return;
// Do not create default only target_version attribute
if (!isDefault) {
@@ -3550,7 +3561,7 @@ static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
bool Sema::checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
- bool &HasDefault, bool &HasCommas, bool &HasNotDefault,
+ Decl *D, bool &HasDefault, bool &HasCommas, bool &HasNotDefault,
SmallVectorImpl<SmallString<64>> &StringsBuffer) {
enum FirstParam { Unsupported, Duplicate, Unknown };
enum SecondParam { None, CPU, Tune };
@@ -3619,6 +3630,9 @@ bool Sema::checkTargetClonesAttrString(
HasNotDefault = true;
}
}
+ if (isArmStreaming(cast<FunctionDecl>(D)))
+ return Diag(LiteralLoc,
+ diag::err_sme_streaming_cannot_be_multiversioned);
} else {
// Other targets ( currently X86 )
if (Cur.starts_with("arch=")) {
@@ -3670,7 +3684,7 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (!S.checkStringLiteralArgumentAttr(AL, I, CurStr, &LiteralLoc) ||
S.checkTargetClonesAttrString(
LiteralLoc, CurStr,
- cast<StringLiteral>(AL.getArgAsExpr(I)->IgnoreParenCasts()),
+ cast<StringLiteral>(AL.getArgAsExpr(I)->IgnoreParenCasts()), D,
HasDefault, HasCommas, HasNotDefault, StringsBuffer))
return;
}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 2bf1886951f1f7..2f8c3eb6aed9d8 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -454,3 +454,34 @@ void unimplemented_spill_fill_za(void (*share_zt0_only)(void) __arm_inout("zt0")
// expected-note at +1 {{add '__arm_preserves("za")' to the callee if it preserves ZA}}
share_zt0_only();
}
+
+// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
+// expected-error at +1 {{streaming function cannot be multi-versioned}}
+__attribute__((target_version("sme2")))
+ // expected-cpp-note at +2 {{previous declaration is here}}
+ // expected-note at +1 {{previous declaration is here}}
+void cannot_work_version(void) __arm_streaming {}
+
+// expected-cpp-error at +3 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}}
+// expected-error at +2 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}}
+__attribute__((target_version("default")))
+void cannot_work_version(void) {}
+
+// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
+// expected-error at +1 {{streaming function cannot be multi-versioned}}
+__attribute__((target_clones("sme2")))
+void cannot_work_clones(void) __arm_streaming {}
+
+__attribute__((target("sme2")))
+void just_fine_sme_streaming(void) __arm_streaming {}
+__attribute__((target_version("sme2")))
+void just_fine(void) { just_fine_sme_streaming(); }
+__attribute__((target_version("default")))
+void just_fine(void) { }
+
+
+void fmv_caller() {
+ cannot_work_version();
+ cannot_work_clones();
+ just_fine();
+}
>From 4f7708d8f7325042c1bb91fe34f09b91cb4a2e63 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Fri, 9 Feb 2024 09:24:09 -0800
Subject: [PATCH 2/4] allow locally_streaming
---
clang/lib/Sema/SemaDeclAttr.cpp | 8 +++-----
clang/test/Sema/aarch64-sme-func-attrs.c | 16 +++++++++++++---
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 2a7c11cd4351f4..4a6f274b7a8f85 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3501,9 +3501,7 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
return false;
}
-static bool isArmStreaming(const FunctionDecl *FD) {
- if (FD->hasAttr<ArmLocallyStreamingAttr>())
- return true;
+static bool hasStreamingModeChangeInABI(const FunctionDecl *FD) {
if (const auto *T = FD->getType()->getAs<FunctionProtoType>())
if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
return true;
@@ -3528,7 +3526,7 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << CurFeature << TargetVersion;
}
- if (isArmStreaming(cast<FunctionDecl>(D)))
+ if (hasStreamingModeChangeInABI(cast<FunctionDecl>(D)))
return Diag(LiteralLoc, diag::err_sme_streaming_cannot_be_multiversioned);
return false;
}
@@ -3630,7 +3628,7 @@ bool Sema::checkTargetClonesAttrString(
HasNotDefault = true;
}
}
- if (isArmStreaming(cast<FunctionDecl>(D)))
+ if (hasStreamingModeChangeInABI(cast<FunctionDecl>(D)))
return Diag(LiteralLoc,
diag::err_sme_streaming_cannot_be_multiversioned);
} else {
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 2f8c3eb6aed9d8..4092018a523454 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -467,21 +467,31 @@ void cannot_work_version(void) __arm_streaming {}
__attribute__((target_version("default")))
void cannot_work_version(void) {}
+
// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
// expected-error at +1 {{streaming function cannot be multi-versioned}}
__attribute__((target_clones("sme2")))
void cannot_work_clones(void) __arm_streaming {}
+
__attribute__((target("sme2")))
-void just_fine_sme_streaming(void) __arm_streaming {}
+void just_fine_streaming(void) __arm_streaming {}
+__attribute__((target_version("sme2")))
+void just_fine(void) { just_fine_streaming(); }
+__attribute__((target_version("default")))
+void just_fine(void) {}
+
+
+__arm_locally_streaming
__attribute__((target_version("sme2")))
-void just_fine(void) { just_fine_sme_streaming(); }
+void just_fine_locally_streaming(void) {}
__attribute__((target_version("default")))
-void just_fine(void) { }
+void just_fine_locally_streaming(void) {}
void fmv_caller() {
cannot_work_version();
cannot_work_clones();
just_fine();
+ just_fine_locally_streaming();
}
>From 6b1614f0ebe481e67b928a483204ed7b321f1ae6 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Fri, 9 Feb 2024 09:24:32 -0800
Subject: [PATCH 3/4] fix check order nit
---
clang/test/Sema/aarch64-sme-func-attrs.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 4092018a523454..47dbeca206a94e 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -458,12 +458,11 @@ void unimplemented_spill_fill_za(void (*share_zt0_only)(void) __arm_inout("zt0")
// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
// expected-error at +1 {{streaming function cannot be multi-versioned}}
__attribute__((target_version("sme2")))
- // expected-cpp-note at +2 {{previous declaration is here}}
- // expected-note at +1 {{previous declaration is here}}
void cannot_work_version(void) __arm_streaming {}
-
-// expected-cpp-error at +3 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}}
-// expected-error at +2 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}}
+// expected-cpp-error at +5 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}}
+// expected-cpp-note at -2 {{previous declaration is here}}
+// expected-error at +3 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}}
+// expected-note at -4 {{previous declaration is here}}
__attribute__((target_version("default")))
void cannot_work_version(void) {}
>From 0b18d706b7fbbcded574767c1255b6d5fce7fd06 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Mon, 12 Feb 2024 07:41:24 -0800
Subject: [PATCH 4/4] nit:
s/hasStreamingModeChangeInABI/hasArmStreamingInterface/g
---
clang/lib/Sema/SemaDeclAttr.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 4a6f274b7a8f85..d5526957937bbb 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3501,7 +3501,7 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
return false;
}
-static bool hasStreamingModeChangeInABI(const FunctionDecl *FD) {
+static bool hasArmStreamingInterface(const FunctionDecl *FD) {
if (const auto *T = FD->getType()->getAs<FunctionProtoType>())
if (T->getAArch64SMEAttributes() & FunctionType::SME_PStateSMEnabledMask)
return true;
@@ -3526,7 +3526,7 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << CurFeature << TargetVersion;
}
- if (hasStreamingModeChangeInABI(cast<FunctionDecl>(D)))
+ if (hasArmStreamingInterface(cast<FunctionDecl>(D)))
return Diag(LiteralLoc, diag::err_sme_streaming_cannot_be_multiversioned);
return false;
}
@@ -3628,7 +3628,7 @@ bool Sema::checkTargetClonesAttrString(
HasNotDefault = true;
}
}
- if (hasStreamingModeChangeInABI(cast<FunctionDecl>(D)))
+ if (hasArmStreamingInterface(cast<FunctionDecl>(D)))
return Diag(LiteralLoc,
diag::err_sme_streaming_cannot_be_multiversioned);
} else {
More information about the cfe-commits
mailing list