llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-x86 Author: Alexandros Lamprineas (labrinea) <details> <summary>Changes</summary> 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. --- Patch is 25.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149067.diff 8 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (-7) - (modified) clang/include/clang/Sema/SemaARM.h (+5) - (modified) clang/include/clang/Sema/SemaRISCV.h (+5) - (modified) clang/include/clang/Sema/SemaX86.h (+4) - (modified) clang/lib/Sema/SemaARM.cpp (+87) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (+36-249) - (modified) clang/lib/Sema/SemaRISCV.cpp (+112) - (modified) clang/lib/Sema/SemaX86.cpp (+58) ``````````diff 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 Ex... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/149067 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
