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_roel...@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@+1 {{add '__arm_preserves("za")' to the callee if it preserves ZA}} share_zt0_only(); } + +// expected-cpp-error@+2 {{streaming function cannot be multi-versioned}} +// expected-error@+1 {{streaming function cannot be multi-versioned}} +__attribute__((target_version("sme2"))) + // expected-cpp-note@+2 {{previous declaration is here}} + // expected-note@+1 {{previous declaration is here}} +void cannot_work_version(void) __arm_streaming {} + +// expected-cpp-error@+3 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}} +// expected-error@+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@+2 {{streaming function cannot be multi-versioned}} +// expected-error@+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_roel...@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@+2 {{streaming function cannot be multi-versioned}} // expected-error@+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_roel...@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@+2 {{streaming function cannot be multi-versioned}} // expected-error@+1 {{streaming function cannot be multi-versioned}} __attribute__((target_version("sme2"))) - // expected-cpp-note@+2 {{previous declaration is here}} - // expected-note@+1 {{previous declaration is here}} void cannot_work_version(void) __arm_streaming {} - -// expected-cpp-error@+3 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}} -// expected-error@+2 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}} +// expected-cpp-error@+5 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}} +// expected-cpp-note@-2 {{previous declaration is here}} +// expected-error@+3 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}} +// expected-note@-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_roel...@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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
