https://github.com/labrinea updated https://github.com/llvm/llvm-project/pull/149067
>From 9a56f94c9851721c92dc4b026a33e44d3ebf3b20 Mon Sep 17 00:00:00 2001 From: Alexandros Lamprineas <alexandros.lamprin...@arm.com> Date: Tue, 15 Jul 2025 14:57:57 +0100 Subject: [PATCH 1/3] [NFC][FMV] Refactor sema checking of target_version/clones attributes. Sema currently has checkTargetVersionAttr and checkTargetClonesAttrString to diagnose the said attributes. However the code tries to handle all of AArch64, RISC-V and X86 targets at once which is hard to maintain, therefore I am splitting these functions. Unfortunately I could not use polymorphism because all of Sema, SemaARM, SemaRISCV and SemaX86 inherit from SemaBase. The Sema instance itself contains instances of every other target specific Sema. --- clang/include/clang/Sema/Sema.h | 7 - clang/include/clang/Sema/SemaARM.h | 5 + clang/include/clang/Sema/SemaRISCV.h | 5 + clang/include/clang/Sema/SemaX86.h | 4 + clang/lib/Sema/SemaARM.cpp | 87 ++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 285 ++++----------------------- clang/lib/Sema/SemaRISCV.cpp | 112 +++++++++++ clang/lib/Sema/SemaX86.cpp | 58 ++++++ 8 files changed, 307 insertions(+), 256 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b331acbe606b7..352dc42edf464 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4922,13 +4922,6 @@ class Sema final : public SemaBase { // handled later in the process, once we know how many exist. bool checkTargetAttr(SourceLocation LiteralLoc, StringRef Str); - /// Check Target Version attrs - bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str); - bool checkTargetClonesAttrString( - SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal, - Decl *D, bool &HasDefault, bool &HasCommas, bool &HasNotDefault, - SmallVectorImpl<SmallString<64>> &StringsBuffer); - ErrorAttr *mergeErrorAttr(Decl *D, const AttributeCommonInfo &CI, StringRef NewUserDiagnostic); FormatAttr *mergeFormatAttr(Decl *D, const AttributeCommonInfo &CI, diff --git a/clang/include/clang/Sema/SemaARM.h b/clang/include/clang/Sema/SemaARM.h index 788a7abf5f9c1..4c2ca59dc929a 100644 --- a/clang/include/clang/Sema/SemaARM.h +++ b/clang/include/clang/Sema/SemaARM.h @@ -91,6 +91,11 @@ class SemaARM : public SemaBase { /// Return true if the given vector types are lax-compatible SVE vector types, /// false otherwise. bool areLaxCompatibleSveTypes(QualType FirstType, QualType SecondType); + + bool checkTargetVersionAttr(StringRef Str, SourceLocation Loc); + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer); }; SemaARM::ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD); diff --git a/clang/include/clang/Sema/SemaRISCV.h b/clang/include/clang/Sema/SemaRISCV.h index 8d2e1c6b7512f..77e1f4a5894c3 100644 --- a/clang/include/clang/Sema/SemaRISCV.h +++ b/clang/include/clang/Sema/SemaRISCV.h @@ -55,6 +55,11 @@ class SemaRISCV : public SemaBase { bool DeclareAndesVectorBuiltins = false; std::unique_ptr<sema::RISCVIntrinsicManager> IntrinsicManager; + + bool checkTargetVersionAttr(StringRef Str, SourceLocation Loc); + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer); }; std::unique_ptr<sema::RISCVIntrinsicManager> diff --git a/clang/include/clang/Sema/SemaX86.h b/clang/include/clang/Sema/SemaX86.h index b5a23f1bede04..bfe0b54ad1be9 100644 --- a/clang/include/clang/Sema/SemaX86.h +++ b/clang/include/clang/Sema/SemaX86.h @@ -37,6 +37,10 @@ class SemaX86 : public SemaBase { void handleAnyInterruptAttr(Decl *D, const ParsedAttr &AL); void handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL); + + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer); }; } // namespace clang diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp index bd603a925d15e..6c1917511b59a 100644 --- a/clang/lib/Sema/SemaARM.cpp +++ b/clang/lib/Sema/SemaARM.cpp @@ -1535,4 +1535,91 @@ bool SemaARM::areLaxCompatibleSveTypes(QualType FirstType, IsLaxCompatible(SecondType, FirstType); } +enum FirstParam { Unsupported, Duplicate, Unknown }; +enum SecondParam { None, CPU, Tune }; +enum ThirdParam { Target, TargetClones, TargetVersion }; + +bool SemaARM::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { + llvm::SmallVector<StringRef, 8> Features; + Str.split(Features, '+'); + for (StringRef Feat : Features) { + Feat = Feat.trim(); + if (Feat == "default") + continue; + if (!getASTContext().getTargetInfo().validateCpuSupports(Feat)) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Feat << TargetVersion; + } + return false; +} + +bool SemaARM::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer) { + if (!getASTContext().getTargetInfo().hasFeature("fmv")) + return true; + + assert(Strs.size() == Locs.size() && + "Mismatch between number of strings and locations"); + + bool HasDefault = false; + bool HasNonDefault = false; + for (unsigned I = 0; I < Strs.size(); ++I) { + StringRef Str = Strs[I].trim(); + SourceLocation Loc = Locs[I]; + + if (Str.empty()) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << "" << TargetClones; + + if (Str == "default") { + if (HasDefault) + Diag(Loc, diag::warn_target_clone_duplicate_options); + else { + Buffer.push_back(Str); + HasDefault = true; + } + continue; + } + + bool HasCodeGenImpact = false; + llvm::SmallVector<StringRef, 8> Features; + llvm::SmallVector<StringRef, 8> ValidFeatures; + Str.split(Features, '+'); + for (StringRef Feat : Features) { + Feat = Feat.trim(); + if (!getASTContext().getTargetInfo().validateCpuSupports(Feat)) { + Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Feat << TargetClones; + continue; + } + if (getASTContext().getTargetInfo().doesFeatureAffectCodeGen(Feat)) + HasCodeGenImpact = true; + ValidFeatures.push_back(Feat); + } + // Canonize TargetClones Attributes + SmallString<64> NewStr; + llvm::sort(ValidFeatures); + for (StringRef Feat : ValidFeatures) { + if (!NewStr.empty()) + NewStr.append("+"); + NewStr.append(Feat); + } + if (llvm::is_contained(Buffer, NewStr)) + Diag(Loc, diag::warn_target_clone_duplicate_options); + else if (!HasCodeGenImpact) + // Ignore features in target_clone attribute that don't impact + // code generation + Diag(Loc, diag::warn_target_clone_no_impact_options); + else if (!NewStr.empty()) { + Buffer.push_back(NewStr); + HasNonDefault = true; + } + } + if (!HasNonDefault) + return true; + + return false; +} + } // namespace clang diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 5f481ed1f7139..3b0b549b70035 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3331,78 +3331,20 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) { return false; } -bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D, - StringRef AttrStr) { - enum FirstParam { Unsupported }; - enum SecondParam { None }; - enum ThirdParam { Target, TargetClones, TargetVersion }; - llvm::SmallVector<StringRef, 8> Features; - if (Context.getTargetInfo().getTriple().isRISCV()) { - llvm::SmallVector<StringRef, 8> AttrStrs; - AttrStr.split(AttrStrs, ';'); - - bool HasArch = false; - bool HasPriority = false; - bool HasDefault = false; - bool DuplicateAttr = false; - for (auto &AttrStr : AttrStrs) { - // Only support arch=+ext,... syntax. - if (AttrStr.starts_with("arch=+")) { - if (HasArch) - DuplicateAttr = true; - HasArch = true; - ParsedTargetAttr TargetAttr = - Context.getTargetInfo().parseTargetAttr(AttrStr); - - if (TargetAttr.Features.empty() || - llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) { - return !RISCV().isValidFMVExtension(Ext); - })) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << AttrStr << TargetVersion; - } else if (AttrStr.starts_with("default")) { - if (HasDefault) - DuplicateAttr = true; - HasDefault = true; - } else if (AttrStr.consume_front("priority=")) { - if (HasPriority) - DuplicateAttr = true; - HasPriority = true; - unsigned Digit; - if (AttrStr.getAsInteger(0, Digit)) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << AttrStr << TargetVersion; - } else { - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << AttrStr << TargetVersion; - } - } - - if (((HasPriority || HasArch) && HasDefault) || DuplicateAttr || - (HasPriority && !HasArch)) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << AttrStr << TargetVersion; - - return false; - } - AttrStr.split(Features, "+"); - for (auto &CurFeature : Features) { - CurFeature = CurFeature.trim(); - if (CurFeature == "default") - continue; - if (!Context.getTargetInfo().validateCpuSupports(CurFeature)) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << CurFeature << TargetVersion; - } - return false; -} - static void handleTargetVersionAttr(Sema &S, Decl *D, const ParsedAttr &AL) { StringRef Str; - SourceLocation LiteralLoc; - if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc) || - S.checkTargetVersionAttr(LiteralLoc, D, Str)) + SourceLocation Loc; + if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &Loc)) return; + + if (S.Context.getTargetInfo().getTriple().isAArch64()) { + if (S.ARM().checkTargetVersionAttr(Str, Loc)) + return; + } else if (S.Context.getTargetInfo().getTriple().isRISCV()) { + if (S.RISCV().checkTargetVersionAttr(Str, Loc)) + return; + } + TargetVersionAttr *NewAttr = ::new (S.Context) TargetVersionAttr(S.Context, AL, Str); D->addAttr(NewAttr); @@ -3419,158 +3361,7 @@ static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(NewAttr); } -bool Sema::checkTargetClonesAttrString( - SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal, - Decl *D, bool &HasDefault, bool &HasCommas, bool &HasNotDefault, - SmallVectorImpl<SmallString<64>> &StringsBuffer) { - enum FirstParam { Unsupported, Duplicate, Unknown }; - enum SecondParam { None, CPU, Tune }; - enum ThirdParam { Target, TargetClones }; - HasCommas = HasCommas || Str.contains(','); - const TargetInfo &TInfo = Context.getTargetInfo(); - // Warn on empty at the beginning of a string. - if (Str.size() == 0) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << "" << TargetClones; - - std::pair<StringRef, StringRef> Parts = {{}, Str}; - while (!Parts.second.empty()) { - Parts = Parts.second.split(','); - StringRef Cur = Parts.first.trim(); - SourceLocation CurLoc = - Literal->getLocationOfByte(Cur.data() - Literal->getString().data(), - getSourceManager(), getLangOpts(), TInfo); - - bool DefaultIsDupe = false; - bool HasCodeGenImpact = false; - if (Cur.empty()) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << "" << TargetClones; - - if (TInfo.getTriple().isAArch64()) { - // AArch64 target clones specific - if (Cur == "default") { - DefaultIsDupe = HasDefault; - HasDefault = true; - if (llvm::is_contained(StringsBuffer, Cur) || DefaultIsDupe) - Diag(CurLoc, diag::warn_target_clone_duplicate_options); - else - StringsBuffer.push_back(Cur); - } else { - std::pair<StringRef, StringRef> CurParts = {{}, Cur}; - llvm::SmallVector<StringRef, 8> CurFeatures; - while (!CurParts.second.empty()) { - CurParts = CurParts.second.split('+'); - StringRef CurFeature = CurParts.first.trim(); - if (!TInfo.validateCpuSupports(CurFeature)) { - Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << CurFeature << TargetClones; - continue; - } - if (TInfo.doesFeatureAffectCodeGen(CurFeature)) - HasCodeGenImpact = true; - CurFeatures.push_back(CurFeature); - } - // Canonize TargetClones Attributes - llvm::sort(CurFeatures); - SmallString<64> Res; - for (auto &CurFeat : CurFeatures) { - if (!Res.empty()) - Res.append("+"); - Res.append(CurFeat); - } - if (llvm::is_contained(StringsBuffer, Res) || DefaultIsDupe) - Diag(CurLoc, diag::warn_target_clone_duplicate_options); - else if (!HasCodeGenImpact) - // Ignore features in target_clone attribute that don't impact - // code generation - Diag(CurLoc, diag::warn_target_clone_no_impact_options); - else if (!Res.empty()) { - StringsBuffer.push_back(Res); - HasNotDefault = true; - } - } - } else if (TInfo.getTriple().isRISCV()) { - // Suppress warn_target_clone_mixed_values - HasCommas = false; - - // Cur is split's parts of Str. RISC-V uses Str directly, - // so skip when encountered more than once. - if (!Str.starts_with(Cur)) - continue; - - llvm::SmallVector<StringRef, 8> AttrStrs; - Str.split(AttrStrs, ";"); - - bool IsPriority = false; - bool IsDefault = false; - for (auto &AttrStr : AttrStrs) { - // Only support arch=+ext,... syntax. - if (AttrStr.starts_with("arch=+")) { - ParsedTargetAttr TargetAttr = - Context.getTargetInfo().parseTargetAttr(AttrStr); - - if (TargetAttr.Features.empty() || - llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) { - return !RISCV().isValidFMVExtension(Ext); - })) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; - } else if (AttrStr.starts_with("default")) { - IsDefault = true; - DefaultIsDupe = HasDefault; - HasDefault = true; - } else if (AttrStr.consume_front("priority=")) { - IsPriority = true; - unsigned Digit; - if (AttrStr.getAsInteger(0, Digit)) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; - } else { - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; - } - } - - if (IsPriority && IsDefault) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; - - if (llvm::is_contained(StringsBuffer, Str) || DefaultIsDupe) - Diag(CurLoc, diag::warn_target_clone_duplicate_options); - StringsBuffer.push_back(Str); - } else { - // Other targets ( currently X86 ) - if (Cur.starts_with("arch=")) { - if (!Context.getTargetInfo().isValidCPUName( - Cur.drop_front(sizeof("arch=") - 1))) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << CPU << Cur.drop_front(sizeof("arch=") - 1) - << TargetClones; - } else if (Cur == "default") { - DefaultIsDupe = HasDefault; - HasDefault = true; - } else if (!Context.getTargetInfo().isValidFeatureName(Cur) || - Context.getTargetInfo().getFMVPriority(Cur) == 0) - return Diag(CurLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Cur << TargetClones; - if (llvm::is_contained(StringsBuffer, Cur) || DefaultIsDupe) - Diag(CurLoc, diag::warn_target_clone_duplicate_options); - // Note: Add even if there are duplicates, since it changes name mangling. - StringsBuffer.push_back(Cur); - } - } - if (Str.rtrim().ends_with(",")) - return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) - << Unsupported << None << "" << TargetClones; - return false; -} - static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (S.Context.getTargetInfo().getTriple().isAArch64() && - !S.Context.getTargetInfo().hasFeature("fmv")) - return; - // Ensure we don't combine these with themselves, since that causes some // confusing behavior. if (const auto *Other = D->getAttr<TargetClonesAttr>()) { @@ -3581,31 +3372,6 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (checkAttrMutualExclusion<TargetClonesAttr>(S, D, AL)) return; - SmallVector<StringRef, 2> Strings; - SmallVector<SmallString<64>, 2> StringsBuffer; - bool HasCommas = false, HasDefault = false, HasNotDefault = false; - - for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) { - StringRef CurStr; - SourceLocation LiteralLoc; - if (!S.checkStringLiteralArgumentAttr(AL, I, CurStr, &LiteralLoc) || - S.checkTargetClonesAttrString( - LiteralLoc, CurStr, - cast<StringLiteral>(AL.getArgAsExpr(I)->IgnoreParenCasts()), D, - HasDefault, HasCommas, HasNotDefault, StringsBuffer)) - return; - } - for (auto &SmallStr : StringsBuffer) - Strings.push_back(SmallStr.str()); - - if (HasCommas && AL.getNumArgs() > 1) - S.Diag(AL.getLoc(), diag::warn_target_clone_mixed_values); - - if (!HasDefault && !S.Context.getTargetInfo().getTriple().isAArch64()) { - S.Diag(AL.getLoc(), diag::err_target_clone_must_have_default); - return; - } - // FIXME: We could probably figure out how to get this to work for lambdas // someday. if (const auto *MD = dyn_cast<CXXMethodDecl>(D)) { @@ -3617,11 +3383,32 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } } - // No multiversion if we have default version only. - if (S.Context.getTargetInfo().getTriple().isAArch64() && !HasNotDefault) - return; + SmallVector<StringRef, 2> Strings; + SmallVector<SourceLocation, 2> Locations; + for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) { + StringRef Str; + SourceLocation Loc; + if (!S.checkStringLiteralArgumentAttr(AL, I, Str, &Loc)) + return; + Strings.push_back(Str); + Locations.push_back(Loc); + } + + SmallVector<SmallString<64>, 2> Buffer; + if (S.Context.getTargetInfo().getTriple().isAArch64()) { + if (S.ARM().checkTargetClonesAttr(Strings, Locations, Buffer)) + return; + } else if (S.Context.getTargetInfo().getTriple().isRISCV()) { + if (S.RISCV().checkTargetClonesAttr(Strings, Locations, Buffer)) + return; + } else if (S.Context.getTargetInfo().getTriple().isX86()) { + if (S.X86().checkTargetClonesAttr(Strings, Locations, Buffer)) + return; + } + Strings.clear(); + for (auto &SmallStr : Buffer) + Strings.push_back(SmallStr.str()); - cast<FunctionDecl>(D)->setIsMultiVersion(); TargetClonesAttr *NewAttr = ::new (S.Context) TargetClonesAttr(S.Context, AL, Strings.data(), Strings.size()); D->addAttr(NewAttr); diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp index cc110e1115ed5..bc9d8330edf6c 100644 --- a/clang/lib/Sema/SemaRISCV.cpp +++ b/clang/lib/Sema/SemaRISCV.cpp @@ -1635,6 +1635,118 @@ bool SemaRISCV::isValidFMVExtension(StringRef Ext) { return -1 != RISCVISAInfo::getRISCVFeaturesBitsInfo(Ext).second; } +enum FirstParam { Unsupported, Duplicate, Unknown }; +enum SecondParam { None, CPU, Tune }; +enum ThirdParam { Target, TargetClones, TargetVersion }; + +bool SemaRISCV::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { + llvm::SmallVector<StringRef, 8> AttrStrs; + Str.split(AttrStrs, ';'); + + bool HasArch = false; + bool HasPriority = false; + bool HasDefault = false; + bool DuplicateAttr = false; + for (StringRef AttrStr : AttrStrs) { + // Only support arch=+ext,... syntax. + if (AttrStr.starts_with("arch=+")) { + if (HasArch) + DuplicateAttr = true; + HasArch = true; + ParsedTargetAttr TargetAttr = + getASTContext().getTargetInfo().parseTargetAttr(AttrStr); + + if (TargetAttr.Features.empty() || + llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) { + return !isValidFMVExtension(Ext); + })) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << AttrStr << TargetVersion; + } else if (AttrStr.starts_with("default")) { + if (HasDefault) + DuplicateAttr = true; + HasDefault = true; + } else if (AttrStr.consume_front("priority=")) { + if (HasPriority) + DuplicateAttr = true; + HasPriority = true; + unsigned Digit; + if (AttrStr.getAsInteger(0, Digit)) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << AttrStr << TargetVersion; + } else { + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << AttrStr << TargetVersion; + } + } + + if (((HasPriority || HasArch) && HasDefault) || DuplicateAttr || + (HasPriority && !HasArch)) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Str << TargetVersion; + + return false; +} + +bool SemaRISCV::checkTargetClonesAttr( + SmallVectorImpl<StringRef> &Strs, SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer) { + assert(Strs.size() == Locs.size() && + "Mismatch between number of strings and locations"); + + bool HasDefault = false; + for (unsigned I = 0; I < Strs.size(); ++I) { + StringRef Str = Strs[I].trim(); + SourceLocation Loc = Locs[I]; + + llvm::SmallVector<StringRef, 8> AttrStrs; + Str.split(AttrStrs, ";"); + + bool IsPriority = false; + bool IsDefault = false; + for (StringRef AttrStr : AttrStrs) { + // Only support arch=+ext,... syntax. + if (AttrStr.starts_with("arch=+")) { + ParsedTargetAttr TargetAttr = + getASTContext().getTargetInfo().parseTargetAttr(AttrStr); + + if (TargetAttr.Features.empty() || + llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) { + return !isValidFMVExtension(Ext); + })) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Str << TargetClones; + } else if (AttrStr.starts_with("default")) { + if (HasDefault) + Diag(Loc, diag::warn_target_clone_duplicate_options); + IsDefault = true; + HasDefault = true; + } else if (AttrStr.consume_front("priority=")) { + IsPriority = true; + unsigned Digit; + if (AttrStr.getAsInteger(0, Digit)) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Str << TargetClones; + } else { + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Str << TargetClones; + } + } + + if (IsPriority && IsDefault) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << Str << TargetClones; + + if (llvm::is_contained(Buffer, Str)) + Diag(Loc, diag::warn_target_clone_duplicate_options); + Buffer.push_back(Str); + } + if (!HasDefault) + return Diag(Locs[0], diag::err_target_clone_must_have_default); + + return false; +} + SemaRISCV::SemaRISCV(Sema &S) : SemaBase(S) {} } // namespace clang diff --git a/clang/lib/Sema/SemaX86.cpp b/clang/lib/Sema/SemaX86.cpp index 5c149bdec7073..ca76019eefc54 100644 --- a/clang/lib/Sema/SemaX86.cpp +++ b/clang/lib/Sema/SemaX86.cpp @@ -1056,4 +1056,62 @@ void SemaX86::handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL) { X86ForceAlignArgPointerAttr(getASTContext(), AL)); } +enum FirstParam { Unsupported, Duplicate, Unknown }; +enum SecondParam { None, CPU, Tune }; +enum ThirdParam { Target, TargetClones, TargetVersion }; + +bool SemaX86::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &Buffer) { + assert(Strs.size() == Locs.size() && + "Mismatch between number of strings and locations"); + + bool HasDefault = false; + bool HasComma = false; + for (unsigned I = 0; I < Strs.size(); ++I) { + StringRef Str = Strs[I].trim(); + SourceLocation Loc = Locs[I]; + + if (Str.empty() || Str.ends_with(',')) + return Diag(Loc, diag::warn_unsupported_target_attribute) + << Unsupported << None << "" << TargetClones; + + if (Str.contains(',')) + HasComma = true; + + StringRef LHS; + StringRef RHS = Str; + do { + std::tie(LHS, RHS) = RHS.split(','); + LHS = LHS.trim(); + SourceLocation CurLoc = Loc.getLocWithOffset(LHS.data() - Str.data()); + + if (LHS.starts_with("arch=")) { + if (!getASTContext().getTargetInfo().isValidCPUName( + LHS.drop_front(sizeof("arch=") - 1))) + return Diag(CurLoc, diag::warn_unsupported_target_attribute) + << Unsupported << CPU << LHS.drop_front(sizeof("arch=") - 1) + << TargetClones; + } else if (LHS == "default") + HasDefault = true; + else if (!getASTContext().getTargetInfo().isValidFeatureName(LHS) || + getASTContext().getTargetInfo().getFMVPriority(LHS) == 0) + return Diag(CurLoc, diag::warn_unsupported_target_attribute) + << Unsupported << None << LHS << TargetClones; + + if (llvm::is_contained(Buffer, LHS)) + Diag(CurLoc, diag::warn_target_clone_duplicate_options); + // Note: Add even if there are duplicates, since it changes name mangling. + Buffer.push_back(LHS); + } while (!RHS.empty()); + } + if (HasComma && Strs.size() > 1) + Diag(Locs[0], diag::warn_target_clone_mixed_values); + + if (!HasDefault) + return Diag(Locs[0], diag::err_target_clone_must_have_default); + + return false; +} + } // namespace clang >From 889e7b8ccb58c1a649011b23e488f538ed716dcc Mon Sep 17 00:00:00 2001 From: Alexandros Lamprineas <alexandros.lamprin...@arm.com> Date: Wed, 16 Jul 2025 16:21:42 +0100 Subject: [PATCH 2/3] changes from last revision: For the SemaRISCV * trim substrings before comparison * refactor some conditional code * check string equality for default version instead of starts_with * remove redundant diagnostic and add a corresponding test --- clang/lib/Sema/SemaRISCV.cpp | 17 +++++++---------- clang/test/SemaCXX/attr-target-clones-riscv.cpp | 3 +++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp index bc9d8330edf6c..28bc519197b77 100644 --- a/clang/lib/Sema/SemaRISCV.cpp +++ b/clang/lib/Sema/SemaRISCV.cpp @@ -1648,10 +1648,10 @@ bool SemaRISCV::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { bool HasDefault = false; bool DuplicateAttr = false; for (StringRef AttrStr : AttrStrs) { + AttrStr = AttrStr.trim(); // Only support arch=+ext,... syntax. if (AttrStr.starts_with("arch=+")) { - if (HasArch) - DuplicateAttr = true; + DuplicateAttr = HasArch; HasArch = true; ParsedTargetAttr TargetAttr = getASTContext().getTargetInfo().parseTargetAttr(AttrStr); @@ -1662,13 +1662,11 @@ bool SemaRISCV::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { })) return Diag(Loc, diag::warn_unsupported_target_attribute) << Unsupported << None << AttrStr << TargetVersion; - } else if (AttrStr.starts_with("default")) { - if (HasDefault) - DuplicateAttr = true; + } else if (AttrStr == "default") { + DuplicateAttr = HasDefault; HasDefault = true; } else if (AttrStr.consume_front("priority=")) { - if (HasPriority) - DuplicateAttr = true; + DuplicateAttr = HasPriority; HasPriority = true; unsigned Digit; if (AttrStr.getAsInteger(0, Digit)) @@ -1705,6 +1703,7 @@ bool SemaRISCV::checkTargetClonesAttr( bool IsPriority = false; bool IsDefault = false; for (StringRef AttrStr : AttrStrs) { + AttrStr = AttrStr.trim(); // Only support arch=+ext,... syntax. if (AttrStr.starts_with("arch=+")) { ParsedTargetAttr TargetAttr = @@ -1716,9 +1715,7 @@ bool SemaRISCV::checkTargetClonesAttr( })) return Diag(Loc, diag::warn_unsupported_target_attribute) << Unsupported << None << Str << TargetClones; - } else if (AttrStr.starts_with("default")) { - if (HasDefault) - Diag(Loc, diag::warn_target_clone_duplicate_options); + } else if (AttrStr == "default") { IsDefault = true; HasDefault = true; } else if (AttrStr.consume_front("priority=")) { diff --git a/clang/test/SemaCXX/attr-target-clones-riscv.cpp b/clang/test/SemaCXX/attr-target-clones-riscv.cpp index 102bb4b9b3d2b..7648284f80c48 100644 --- a/clang/test/SemaCXX/attr-target-clones-riscv.cpp +++ b/clang/test/SemaCXX/attr-target-clones-riscv.cpp @@ -9,6 +9,9 @@ void __attribute__((target_clones("default", "mtune=sifive-u74"))) mtune() {} // expected-warning@+1 {{version list contains duplicate entries}} void __attribute__((target_clones("default", "arch=+c", "arch=+c"))) dupVersion() {} +// expected-warning@+1 {{version list contains duplicate entries}} +void __attribute__((target_clones(" default", "default "))) dupDefault() {} + // expected-warning@+1 {{unsupported '' in the 'target_clones' attribute string; 'target_clones' attribute ignored}} void __attribute__((target_clones("default", ""))) emptyVersion() {} >From adde9cb54432d5e8d6cc8dc35d874fc63e4c87eb Mon Sep 17 00:00:00 2001 From: Alexandros Lamprineas <alexandros.lamprin...@arm.com> Date: Fri, 18 Jul 2025 14:44:48 +0100 Subject: [PATCH 3/3] changes from last revision Renamed variables, added constness to types and reorder SemaARM code --- clang/include/clang/Sema/Sema.h | 7 +++ clang/include/clang/Sema/SemaARM.h | 6 +-- clang/include/clang/Sema/SemaRISCV.h | 6 +-- clang/include/clang/Sema/SemaX86.h | 4 +- clang/lib/Sema/SemaARM.cpp | 68 +++++++++++++++------------- clang/lib/Sema/SemaDeclAttr.cpp | 39 ++++++++-------- clang/lib/Sema/SemaRISCV.cpp | 41 +++++++++-------- clang/lib/Sema/SemaX86.cpp | 35 +++++++------- 8 files changed, 108 insertions(+), 98 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 352dc42edf464..c4096f5d3162a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -834,6 +834,13 @@ enum class CCEKind { ///< message. }; +/// Enums for the diagnostics of target, target_version and target_clones. +namespace DiagAttrParams { + enum DiagType { Unsupported, Duplicate, Unknown }; + enum Specifier { None, CPU, Tune }; + enum AttrName { Target, TargetClones, TargetVersion }; +} // end namespace DiagAttrParams + void inferNoReturnAttr(Sema &S, const Decl *D); /// Sema - This implements semantic analysis and AST building for C. diff --git a/clang/include/clang/Sema/SemaARM.h b/clang/include/clang/Sema/SemaARM.h index 4c2ca59dc929a..e77d65f9362d8 100644 --- a/clang/include/clang/Sema/SemaARM.h +++ b/clang/include/clang/Sema/SemaARM.h @@ -92,10 +92,10 @@ class SemaARM : public SemaBase { /// false otherwise. bool areLaxCompatibleSveTypes(QualType FirstType, QualType SecondType); - bool checkTargetVersionAttr(StringRef Str, SourceLocation Loc); - bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + bool checkTargetVersionAttr(const StringRef Str, const SourceLocation Loc); + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer); + SmallVectorImpl<SmallString<64>> &NewParams); }; SemaARM::ArmStreamingType getArmStreamingFnType(const FunctionDecl *FD); diff --git a/clang/include/clang/Sema/SemaRISCV.h b/clang/include/clang/Sema/SemaRISCV.h index 77e1f4a5894c3..844cc3ce4a440 100644 --- a/clang/include/clang/Sema/SemaRISCV.h +++ b/clang/include/clang/Sema/SemaRISCV.h @@ -56,10 +56,10 @@ class SemaRISCV : public SemaBase { std::unique_ptr<sema::RISCVIntrinsicManager> IntrinsicManager; - bool checkTargetVersionAttr(StringRef Str, SourceLocation Loc); - bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + bool checkTargetVersionAttr(const StringRef Param, const SourceLocation Loc); + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer); + SmallVectorImpl<SmallString<64>> &NewParams); }; std::unique_ptr<sema::RISCVIntrinsicManager> diff --git a/clang/include/clang/Sema/SemaX86.h b/clang/include/clang/Sema/SemaX86.h index bfe0b54ad1be9..20783e344c02f 100644 --- a/clang/include/clang/Sema/SemaX86.h +++ b/clang/include/clang/Sema/SemaX86.h @@ -38,9 +38,9 @@ class SemaX86 : public SemaBase { void handleAnyInterruptAttr(Decl *D, const ParsedAttr &AL); void handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL); - bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, + bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer); + SmallVectorImpl<SmallString<64>> &NewParams); }; } // namespace clang diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp index 6c1917511b59a..8e27fabccd583 100644 --- a/clang/lib/Sema/SemaARM.cpp +++ b/clang/lib/Sema/SemaARM.cpp @@ -1535,13 +1535,12 @@ bool SemaARM::areLaxCompatibleSveTypes(QualType FirstType, IsLaxCompatible(SecondType, FirstType); } -enum FirstParam { Unsupported, Duplicate, Unknown }; -enum SecondParam { None, CPU, Tune }; -enum ThirdParam { Target, TargetClones, TargetVersion }; +bool SemaARM::checkTargetVersionAttr(const StringRef Param, + const SourceLocation Loc) { + using namespace DiagAttrParams; -bool SemaARM::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { llvm::SmallVector<StringRef, 8> Features; - Str.split(Features, '+'); + Param.split(Features, '+'); for (StringRef Feat : Features) { Feat = Feat.trim(); if (Feat == "default") @@ -1553,30 +1552,32 @@ bool SemaARM::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { return false; } -bool SemaARM::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, - SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer) { +bool SemaARM::checkTargetClonesAttr( + SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams) { + using namespace DiagAttrParams; + if (!getASTContext().getTargetInfo().hasFeature("fmv")) return true; - assert(Strs.size() == Locs.size() && - "Mismatch between number of strings and locations"); + assert(Params.size() == Locs.size() && + "Mismatch between number of string parameters and locations"); bool HasDefault = false; bool HasNonDefault = false; - for (unsigned I = 0; I < Strs.size(); ++I) { - StringRef Str = Strs[I].trim(); - SourceLocation Loc = Locs[I]; + for (unsigned I = 0, E = Params.size(); I < E; ++I) { + const StringRef Param = Params[I].trim(); + const SourceLocation &Loc = Locs[I]; - if (Str.empty()) + if (Param.empty()) return Diag(Loc, diag::warn_unsupported_target_attribute) << Unsupported << None << "" << TargetClones; - if (Str == "default") { + if (Param == "default") { if (HasDefault) Diag(Loc, diag::warn_target_clone_duplicate_options); else { - Buffer.push_back(Str); + NewParams.push_back(Param); HasDefault = true; } continue; @@ -1585,7 +1586,7 @@ bool SemaARM::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, bool HasCodeGenImpact = false; llvm::SmallVector<StringRef, 8> Features; llvm::SmallVector<StringRef, 8> ValidFeatures; - Str.split(Features, '+'); + Param.split(Features, '+'); for (StringRef Feat : Features) { Feat = Feat.trim(); if (!getASTContext().getTargetInfo().validateCpuSupports(Feat)) { @@ -1597,24 +1598,27 @@ bool SemaARM::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, HasCodeGenImpact = true; ValidFeatures.push_back(Feat); } - // Canonize TargetClones Attributes - SmallString<64> NewStr; - llvm::sort(ValidFeatures); - for (StringRef Feat : ValidFeatures) { - if (!NewStr.empty()) - NewStr.append("+"); - NewStr.append(Feat); + + // Ignore features that don't impact code generation. + if (!HasCodeGenImpact) { + Diag(Loc, diag::warn_target_clone_no_impact_options); + continue; } - if (llvm::is_contained(Buffer, NewStr)) + + if (ValidFeatures.empty()) + continue; + + // Canonicalize attribute parameter. + llvm::sort(ValidFeatures); + SmallString<64> NewParam(llvm::join(ValidFeatures, "+")); + if (llvm::is_contained(NewParams, NewParam)) { Diag(Loc, diag::warn_target_clone_duplicate_options); - else if (!HasCodeGenImpact) - // Ignore features in target_clone attribute that don't impact - // code generation - Diag(Loc, diag::warn_target_clone_no_impact_options); - else if (!NewStr.empty()) { - Buffer.push_back(NewStr); - HasNonDefault = true; + continue; } + + // Valid non-default argument. + NewParams.push_back(NewParam); + HasNonDefault = true; } if (!HasNonDefault) return true; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 3b0b549b70035..78c6c49321371 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3254,9 +3254,8 @@ static void handleCodeSegAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) { - enum FirstParam { Unsupported, Duplicate, Unknown }; - enum SecondParam { None, CPU, Tune }; - enum ThirdParam { Target, TargetClones }; + using namespace DiagAttrParams; + if (AttrStr.contains("fpmath=")) return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << Unsupported << None << "fpmath=" << Target; @@ -3332,21 +3331,21 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) { } static void handleTargetVersionAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - StringRef Str; + StringRef Param; SourceLocation Loc; - if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &Loc)) + if (!S.checkStringLiteralArgumentAttr(AL, 0, Param, &Loc)) return; if (S.Context.getTargetInfo().getTriple().isAArch64()) { - if (S.ARM().checkTargetVersionAttr(Str, Loc)) + if (S.ARM().checkTargetVersionAttr(Param, Loc)) return; } else if (S.Context.getTargetInfo().getTriple().isRISCV()) { - if (S.RISCV().checkTargetVersionAttr(Str, Loc)) + if (S.RISCV().checkTargetVersionAttr(Param, Loc)) return; } TargetVersionAttr *NewAttr = - ::new (S.Context) TargetVersionAttr(S.Context, AL, Str); + ::new (S.Context) TargetVersionAttr(S.Context, AL, Param); D->addAttr(NewAttr); } @@ -3383,34 +3382,34 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } } - SmallVector<StringRef, 2> Strings; + SmallVector<StringRef, 2> Params; SmallVector<SourceLocation, 2> Locations; for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) { - StringRef Str; + StringRef Param; SourceLocation Loc; - if (!S.checkStringLiteralArgumentAttr(AL, I, Str, &Loc)) + if (!S.checkStringLiteralArgumentAttr(AL, I, Param, &Loc)) return; - Strings.push_back(Str); + Params.push_back(Param); Locations.push_back(Loc); } - SmallVector<SmallString<64>, 2> Buffer; + SmallVector<SmallString<64>, 2> NewParams; if (S.Context.getTargetInfo().getTriple().isAArch64()) { - if (S.ARM().checkTargetClonesAttr(Strings, Locations, Buffer)) + if (S.ARM().checkTargetClonesAttr(Params, Locations, NewParams)) return; } else if (S.Context.getTargetInfo().getTriple().isRISCV()) { - if (S.RISCV().checkTargetClonesAttr(Strings, Locations, Buffer)) + if (S.RISCV().checkTargetClonesAttr(Params, Locations, NewParams)) return; } else if (S.Context.getTargetInfo().getTriple().isX86()) { - if (S.X86().checkTargetClonesAttr(Strings, Locations, Buffer)) + if (S.X86().checkTargetClonesAttr(Params, Locations, NewParams)) return; } - Strings.clear(); - for (auto &SmallStr : Buffer) - Strings.push_back(SmallStr.str()); + Params.clear(); + for (auto &SmallStr : NewParams) + Params.push_back(SmallStr.str()); TargetClonesAttr *NewAttr = ::new (S.Context) - TargetClonesAttr(S.Context, AL, Strings.data(), Strings.size()); + TargetClonesAttr(S.Context, AL, Params.data(), Params.size()); D->addAttr(NewAttr); } diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp index 28bc519197b77..a50429ba6528d 100644 --- a/clang/lib/Sema/SemaRISCV.cpp +++ b/clang/lib/Sema/SemaRISCV.cpp @@ -1635,13 +1635,12 @@ bool SemaRISCV::isValidFMVExtension(StringRef Ext) { return -1 != RISCVISAInfo::getRISCVFeaturesBitsInfo(Ext).second; } -enum FirstParam { Unsupported, Duplicate, Unknown }; -enum SecondParam { None, CPU, Tune }; -enum ThirdParam { Target, TargetClones, TargetVersion }; +bool SemaRISCV::checkTargetVersionAttr(const StringRef Param, + const SourceLocation Loc) { + using namespace DiagAttrParams; -bool SemaRISCV::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { llvm::SmallVector<StringRef, 8> AttrStrs; - Str.split(AttrStrs, ';'); + Param.split(AttrStrs, ';'); bool HasArch = false; bool HasPriority = false; @@ -1681,24 +1680,26 @@ bool SemaRISCV::checkTargetVersionAttr(StringRef Str, SourceLocation Loc) { if (((HasPriority || HasArch) && HasDefault) || DuplicateAttr || (HasPriority && !HasArch)) return Diag(Loc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetVersion; + << Unsupported << None << Param << TargetVersion; return false; } bool SemaRISCV::checkTargetClonesAttr( - SmallVectorImpl<StringRef> &Strs, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer) { - assert(Strs.size() == Locs.size() && - "Mismatch between number of strings and locations"); + SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams) { + using namespace DiagAttrParams; + + assert(Params.size() == Locs.size() && + "Mismatch between number of string parameters and locations"); bool HasDefault = false; - for (unsigned I = 0; I < Strs.size(); ++I) { - StringRef Str = Strs[I].trim(); - SourceLocation Loc = Locs[I]; + for (unsigned I = 0, E = Params.size(); I < E; ++I) { + const StringRef Param = Params[I].trim(); + const SourceLocation &Loc = Locs[I]; llvm::SmallVector<StringRef, 8> AttrStrs; - Str.split(AttrStrs, ";"); + Param.split(AttrStrs, ';'); bool IsPriority = false; bool IsDefault = false; @@ -1714,7 +1715,7 @@ bool SemaRISCV::checkTargetClonesAttr( return !isValidFMVExtension(Ext); })) return Diag(Loc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; + << Unsupported << None << Param << TargetClones; } else if (AttrStr == "default") { IsDefault = true; HasDefault = true; @@ -1723,20 +1724,20 @@ bool SemaRISCV::checkTargetClonesAttr( unsigned Digit; if (AttrStr.getAsInteger(0, Digit)) return Diag(Loc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; + << Unsupported << None << Param << TargetClones; } else { return Diag(Loc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; + << Unsupported << None << Param << TargetClones; } } if (IsPriority && IsDefault) return Diag(Loc, diag::warn_unsupported_target_attribute) - << Unsupported << None << Str << TargetClones; + << Unsupported << None << Param << TargetClones; - if (llvm::is_contained(Buffer, Str)) + if (llvm::is_contained(NewParams, Param)) Diag(Loc, diag::warn_target_clone_duplicate_options); - Buffer.push_back(Str); + NewParams.push_back(Param); } if (!HasDefault) return Diag(Locs[0], diag::err_target_clone_must_have_default); diff --git a/clang/lib/Sema/SemaX86.cpp b/clang/lib/Sema/SemaX86.cpp index ca76019eefc54..8dc3cd74d7d8f 100644 --- a/clang/lib/Sema/SemaX86.cpp +++ b/clang/lib/Sema/SemaX86.cpp @@ -1056,35 +1056,34 @@ void SemaX86::handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL) { X86ForceAlignArgPointerAttr(getASTContext(), AL)); } -enum FirstParam { Unsupported, Duplicate, Unknown }; -enum SecondParam { None, CPU, Tune }; -enum ThirdParam { Target, TargetClones, TargetVersion }; +bool SemaX86::checkTargetClonesAttr( + SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams) { + using namespace DiagAttrParams; -bool SemaX86::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, - SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &Buffer) { - assert(Strs.size() == Locs.size() && - "Mismatch between number of strings and locations"); + assert(Params.size() == Locs.size() && + "Mismatch between number of string parameters and locations"); bool HasDefault = false; bool HasComma = false; - for (unsigned I = 0; I < Strs.size(); ++I) { - StringRef Str = Strs[I].trim(); - SourceLocation Loc = Locs[I]; + for (unsigned I = 0, E = Params.size(); I < E; ++I) { + const StringRef Param = Params[I].trim(); + const SourceLocation &Loc = Locs[I]; - if (Str.empty() || Str.ends_with(',')) + if (Param.empty() || Param.ends_with(',')) return Diag(Loc, diag::warn_unsupported_target_attribute) << Unsupported << None << "" << TargetClones; - if (Str.contains(',')) + if (Param.contains(',')) HasComma = true; StringRef LHS; - StringRef RHS = Str; + StringRef RHS = Param; do { std::tie(LHS, RHS) = RHS.split(','); LHS = LHS.trim(); - SourceLocation CurLoc = Loc.getLocWithOffset(LHS.data() - Str.data()); + const SourceLocation &CurLoc = + Loc.getLocWithOffset(LHS.data() - Param.data()); if (LHS.starts_with("arch=")) { if (!getASTContext().getTargetInfo().isValidCPUName( @@ -1099,13 +1098,13 @@ bool SemaX86::checkTargetClonesAttr(SmallVectorImpl<StringRef> &Strs, return Diag(CurLoc, diag::warn_unsupported_target_attribute) << Unsupported << None << LHS << TargetClones; - if (llvm::is_contained(Buffer, LHS)) + if (llvm::is_contained(NewParams, LHS)) Diag(CurLoc, diag::warn_target_clone_duplicate_options); // Note: Add even if there are duplicates, since it changes name mangling. - Buffer.push_back(LHS); + NewParams.push_back(LHS); } while (!RHS.empty()); } - if (HasComma && Strs.size() > 1) + if (HasComma && Params.size() > 1) Diag(Locs[0], diag::warn_target_clone_mixed_values); if (!HasDefault) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits