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/2] [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/2] 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() {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
