[FMV] Allow mixing target_version with target_clones. #86493
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Alexandros Lamprineas (labrinea) ChangesThe latest ACLE allows it and further clarifies the following "If the ("default" refers to the default clone here) Patch is 47.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86493.diff 8 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 3e03e55612645b..318d4e5ac5ba44 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3088,6 +3088,20 @@ def TargetClones : InheritableAttr {
StringRef getFeatureStr(unsigned Index) const {
return *(featuresStrs_begin() + Index);
}
+ bool isDefaultVersion(unsigned Index) const {
+ return getFeatureStr(Index) == "default";
+ }
+ void getFeatures(llvm::SmallVectorImpl<StringRef> &Out,
+ unsigned Index) const {
+ if (isDefaultVersion(Index)) return;
+ StringRef Features = getFeatureStr(Index);
+ SmallVector<StringRef, 8> AttrFeatures;
+ Features.split(AttrFeatures, "+");
+ for (auto &Feature : AttrFeatures) {
+ Feature = Feature.trim();
+ Out.push_back(Feature);
+ }
+ }
// Given an index into the 'featuresStrs' sequence, compute a unique
// ID to be used with function name mangling for the associated variant.
// This mapping is necessary due to a requirement that the mangling ID
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index fcf801adeaf5ef..0b5f20a572a742 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13676,22 +13676,19 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
} else if (const auto *TC = FD->getAttr<TargetClonesAttr>()) {
std::vector<std::string> Features;
- StringRef VersionStr = TC->getFeatureStr(GD.getMultiVersionIndex());
if (Target->getTriple().isAArch64()) {
// TargetClones for AArch64
- if (VersionStr != "default") {
- SmallVector<StringRef, 1> VersionFeatures;
- VersionStr.split(VersionFeatures, "+");
- for (auto &VFeature : VersionFeatures) {
- VFeature = VFeature.trim();
+ llvm::SmallVector<StringRef, 8> Feats;
+ TC->getFeatures(Feats, GD.getMultiVersionIndex());
+ for (StringRef Feat : Feats)
+ if (Target->validateCpuSupports(Feat.str()))
// Use '?' to mark features that came from AArch64 TargetClones.
- Features.push_back((StringRef{"?"} + VFeature).str());
- }
- }
+ Features.push_back("?" + Feat.str());
Features.insert(Features.begin(),
Target->getTargetOpts().FeaturesAsWritten.begin(),
Target->getTargetOpts().FeaturesAsWritten.end());
} else {
+ StringRef VersionStr = TC->getFeatureStr(GD.getMultiVersionIndex());
if (VersionStr.starts_with("arch="))
TargetCPU = VersionStr.drop_front(sizeof("arch=") - 1);
else if (VersionStr != "default")
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index ac81df8cf7adfe..bc7d7ac561113b 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3712,7 +3712,8 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
// Forward declarations are emitted lazily on first use.
if (!FD->doesThisDeclarationHaveABody()) {
if (!FD->doesDeclarationForceExternallyVisibleDefinition() &&
- !FD->isTargetVersionMultiVersion())
+ (!FD->isMultiVersion() ||
+ !FD->getASTContext().getTargetInfo().getTriple().isAArch64()))
return;
StringRef MangledName = getMangledName(GD);
@@ -3994,10 +3995,11 @@ void CodeGenModule::EmitMultiVersionFunctionDefinition(GlobalDecl GD,
auto *Spec = FD->getAttr<CPUSpecificAttr>();
for (unsigned I = 0; I < Spec->cpus_size(); ++I)
EmitGlobalFunctionDefinition(GD.getWithMultiVersionIndex(I), nullptr);
- } else if (FD->isTargetClonesMultiVersion()) {
- auto *Clone = FD->getAttr<TargetClonesAttr>();
- for (unsigned I = 0; I < Clone->featuresStrs_size(); ++I)
- if (Clone->isFirstOfVersion(I))
+ } else if (auto *TC = FD->getAttr<TargetClonesAttr>()) {
+ for (unsigned I = 0; I < TC->featuresStrs_size(); ++I)
+ // AArch64 favors the default target version over the clone if any.
+ if ((!TC->isDefaultVersion(I) || !getTarget().getTriple().isAArch64()) &&
+ TC->isFirstOfVersion(I))
EmitGlobalFunctionDefinition(GD.getWithMultiVersionIndex(I), nullptr);
// Ensure that the resolver function is also emitted.
GetOrCreateMultiVersionResolver(GD);
@@ -4137,57 +4139,49 @@ void CodeGenModule::emitMultiVersionFunctions() {
};
bool HasDefaultDecl = !FD->isTargetVersionMultiVersion();
- bool ShouldEmitResolver = !FD->isTargetVersionMultiVersion();
+ bool ShouldEmitResolver =
+ !getContext().getTargetInfo().getTriple().isAArch64();
SmallVector<CodeGenFunction::MultiVersionResolverOption, 10> Options;
- if (FD->isTargetMultiVersion()) {
- getContext().forEachMultiversionedFunctionVersion(
- FD, [&](const FunctionDecl *CurFD) {
- llvm::SmallVector<StringRef, 8> Feats;
- llvm::Function *Func = createFunction(CurFD);
- if (const auto *TA = CurFD->getAttr<TargetAttr>()) {
- TA->getAddedFeatures(Feats);
- Options.emplace_back(Func, TA->getArchitecture(), Feats);
- } else if (const auto *TVA = CurFD->getAttr<TargetVersionAttr>()) {
- bool HasDefaultDef = TVA->isDefaultVersion() &&
- CurFD->doesThisDeclarationHaveABody();
- HasDefaultDecl |= TVA->isDefaultVersion();
- ShouldEmitResolver |= (CurFD->isUsed() || HasDefaultDef);
- TVA->getFeatures(Feats);
- Options.emplace_back(Func, /*Architecture*/ "", Feats);
- } else
- llvm_unreachable("unexpected MultiVersionKind");
- });
- } else if (const auto *TC = FD->getAttr<TargetClonesAttr>()) {
- for (unsigned I = 0; I < TC->featuresStrs_size(); ++I) {
- if (!TC->isFirstOfVersion(I))
- continue;
+ getContext().forEachMultiversionedFunctionVersion(
+ FD, [&](const FunctionDecl *CurFD) {
+ llvm::SmallVector<StringRef, 8> Feats;
- llvm::Function *Func = createFunction(FD, I);
- StringRef Version = TC->getFeatureStr(I);
- StringRef Architecture;
- llvm::SmallVector<StringRef, 1> Feature;
-
- if (getTarget().getTriple().isAArch64()) {
- if (Version != "default") {
- llvm::SmallVector<StringRef, 8> VerFeats;
- Version.split(VerFeats, "+");
- for (auto &CurFeat : VerFeats)
- Feature.push_back(CurFeat.trim());
- }
- } else {
- if (Version.starts_with("arch="))
- Architecture = Version.drop_front(sizeof("arch=") - 1);
- else if (Version != "default")
- Feature.push_back(Version);
- }
-
- Options.emplace_back(Func, Architecture, Feature);
- }
- } else {
- assert(0 && "Expected a target or target_clones multiversion function");
- continue;
- }
+ if (const auto *TA = CurFD->getAttr<TargetAttr>()) {
+ TA->getAddedFeatures(Feats);
+ llvm::Function *Func = createFunction(CurFD);
+ Options.emplace_back(Func, TA->getArchitecture(), Feats);
+ } else if (const auto *TVA = CurFD->getAttr<TargetVersionAttr>()) {
+ bool HasDefaultDef = TVA->isDefaultVersion() &&
+ CurFD->doesThisDeclarationHaveABody();
+ HasDefaultDecl |= TVA->isDefaultVersion();
+ ShouldEmitResolver |= (CurFD->isUsed() || HasDefaultDef);
+ TVA->getFeatures(Feats);
+ llvm::Function *Func = createFunction(CurFD);
+ Options.emplace_back(Func, /*Architecture*/ "", Feats);
+ } else if (const auto *TC = CurFD->getAttr<TargetClonesAttr>()) {
+ ShouldEmitResolver |= CurFD->doesThisDeclarationHaveABody();
+ for (unsigned I = 0; I < TC->featuresStrs_size(); ++I) {
+ if (!TC->isFirstOfVersion(I))
+ continue;
+
+ llvm::Function *Func = createFunction(CurFD, I);
+ StringRef Architecture;
+ Feats.clear();
+ if (getTarget().getTriple().isAArch64())
+ TC->getFeatures(Feats, I);
+ else {
+ StringRef Version = TC->getFeatureStr(I);
+ if (Version.starts_with("arch="))
+ Architecture = Version.drop_front(sizeof("arch=") - 1);
+ else if (Version != "default")
+ Feats.push_back(Version);
+ }
+ Options.emplace_back(Func, Architecture, Feats);
+ }
+ } else
+ llvm_unreachable("unexpected MultiVersionKind");
+ });
if (!ShouldEmitResolver)
continue;
@@ -4378,7 +4372,7 @@ void CodeGenModule::AddDeferredMultiVersionResolverToEmit(GlobalDecl GD) {
const auto *FD = cast<FunctionDecl>(GD.getDecl());
assert(FD && "Not a FunctionDecl?");
- if (FD->isTargetVersionMultiVersion()) {
+ if (FD->isTargetVersionMultiVersion() || FD->isTargetClonesMultiVersion()) {
std::string MangledName =
getMangledNameImpl(*this, GD, FD, /*OmitMultiVersionMangling=*/true);
if (!DeferredResolversToEmit.insert(MangledName).second)
@@ -4489,7 +4483,8 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
if (FD->isMultiVersion()) {
UpdateMultiVersionNames(GD, FD, MangledName);
- if (FD->isTargetVersionMultiVersion() && !FD->isUsed())
+ if (FD->getASTContext().getTargetInfo().getTriple().isAArch64() &&
+ !FD->isUsed())
AddDeferredMultiVersionResolverToEmit(GD);
else if (!IsForDefinition)
return GetOrCreateMultiVersionResolver(GD);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 73ea155053d737..b1f2749c408bd6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11260,11 +11260,13 @@ static bool checkNonMultiVersionCompatAttributes(Sema &S,
return Diagnose(S, A);
break;
case attr::TargetVersion:
- if (MVKind != MultiVersionKind::TargetVersion)
+ if (MVKind != MultiVersionKind::TargetVersion &&
+ MVKind != MultiVersionKind::TargetClones)
return Diagnose(S, A);
break;
case attr::TargetClones:
- if (MVKind != MultiVersionKind::TargetClones)
+ if (MVKind != MultiVersionKind::TargetClones &&
+ MVKind != MultiVersionKind::TargetVersion)
return Diagnose(S, A);
break;
default:
@@ -11469,6 +11471,23 @@ static bool PreviousDeclsHaveMultiVersionAttribute(const FunctionDecl *FD) {
return false;
}
+static
+void patchDefaultTargetVersion(FunctionDecl *From, FunctionDecl *To) {
+ if (!From->getASTContext().getTargetInfo().getTriple().isAArch64())
+ return;
+
+ MultiVersionKind MVKindFrom = From->getMultiVersionKind();
+ MultiVersionKind MVKindTo = To->getMultiVersionKind();
+
+ if (MVKindTo == MultiVersionKind::None &&
+ (MVKindFrom == MultiVersionKind::TargetVersion ||
+ MVKindFrom == MultiVersionKind::TargetClones)) {
+ To->setIsMultiVersion();
+ To->addAttr(TargetVersionAttr::CreateImplicit(
+ To->getASTContext(), "default", To->getSourceRange()));
+ }
+}
+
static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
FunctionDecl *NewFD,
bool &Redeclaration,
@@ -11479,10 +11498,7 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
// The definitions should be allowed in any order. If we have discovered
// a new target version and the preceeding was the default, then add the
// corresponding attribute to it.
- if (OldFD->getMultiVersionKind() == MultiVersionKind::None &&
- NewFD->getMultiVersionKind() == MultiVersionKind::TargetVersion)
- OldFD->addAttr(TargetVersionAttr::CreateImplicit(S.Context, "default",
- OldFD->getSourceRange()));
+ patchDefaultTargetVersion(NewFD, OldFD);
const auto *NewTA = NewFD->getAttr<TargetAttr>();
const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
@@ -11583,36 +11599,61 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
return false;
}
-static bool MultiVersionTypesCompatible(MultiVersionKind Old,
- MultiVersionKind New) {
- if (Old == New || Old == MultiVersionKind::None ||
- New == MultiVersionKind::None)
+static bool MultiVersionTypesCompatible(FunctionDecl *Old,
+ FunctionDecl *New) {
+ MultiVersionKind OldKind = Old->getMultiVersionKind();
+ MultiVersionKind NewKind = New->getMultiVersionKind();
+
+ if (OldKind == NewKind || OldKind == MultiVersionKind::None ||
+ NewKind == MultiVersionKind::None)
return true;
- return (Old == MultiVersionKind::CPUDispatch &&
- New == MultiVersionKind::CPUSpecific) ||
- (Old == MultiVersionKind::CPUSpecific &&
- New == MultiVersionKind::CPUDispatch);
+ if (Old->getASTContext().getTargetInfo().getTriple().isAArch64()) {
+ switch (OldKind) {
+ case MultiVersionKind::TargetVersion:
+ return NewKind == MultiVersionKind::TargetClones;
+ case MultiVersionKind::TargetClones:
+ return NewKind == MultiVersionKind::TargetVersion;
+ default:
+ return false;
+ }
+ } else {
+ switch (OldKind) {
+ case MultiVersionKind::CPUDispatch:
+ return NewKind == MultiVersionKind::CPUSpecific;
+ case MultiVersionKind::CPUSpecific:
+ return NewKind == MultiVersionKind::CPUDispatch;
+ default:
+ return false;
+ }
+ }
}
/// Check the validity of a new function declaration being added to an existing
/// multiversioned declaration collection.
static bool CheckMultiVersionAdditionalDecl(
Sema &S, FunctionDecl *OldFD, FunctionDecl *NewFD,
- MultiVersionKind NewMVKind, const CPUDispatchAttr *NewCPUDisp,
- const CPUSpecificAttr *NewCPUSpec, const TargetClonesAttr *NewClones,
- bool &Redeclaration, NamedDecl *&OldDecl, LookupResult &Previous) {
- const auto *NewTA = NewFD->getAttr<TargetAttr>();
- const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
- MultiVersionKind OldMVKind = OldFD->getMultiVersionKind();
+ const CPUDispatchAttr *NewCPUDisp, const CPUSpecificAttr *NewCPUSpec,
+ const TargetClonesAttr *NewClones, bool &Redeclaration, NamedDecl *&OldDecl,
+ LookupResult &Previous) {
+
// Disallow mixing of multiversioning types.
- if (!MultiVersionTypesCompatible(OldMVKind, NewMVKind)) {
+ if (!MultiVersionTypesCompatible(OldFD, NewFD)) {
S.Diag(NewFD->getLocation(), diag::err_multiversion_types_mixed);
S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
NewFD->setInvalidDecl();
return true;
}
+ // Add the default target_version attribute if it's missing.
+ patchDefaultTargetVersion(OldFD, NewFD);
+ patchDefaultTargetVersion(NewFD, OldFD);
+
+ const auto *NewTA = NewFD->getAttr<TargetAttr>();
+ const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
+ MultiVersionKind NewMVKind = NewFD->getMultiVersionKind();
+ MultiVersionKind OldMVKind = OldFD->getMultiVersionKind();
+
ParsedTargetAttr NewParsed;
if (NewTA) {
NewParsed = S.getASTContext().getTargetInfo().parseTargetAttr(
@@ -11641,19 +11682,6 @@ static bool CheckMultiVersionAdditionalDecl(
S.IsOverload(NewFD, CurFD, UseMemberUsingDeclRules))
continue;
- if (NewMVKind == MultiVersionKind::None &&
- OldMVKind == MultiVersionKind::TargetVersion) {
- NewFD->addAttr(TargetVersionAttr::CreateImplicit(
- S.Context, "default", NewFD->getSourceRange()));
- NewFD->setIsMultiVersion();
- NewMVKind = MultiVersionKind::TargetVersion;
- if (!NewTVA) {
- NewTVA = NewFD->getAttr<TargetVersionAttr>();
- NewTVA->getFeatures(NewFeats);
- llvm::sort(NewFeats);
- }
- }
-
switch (NewMVKind) {
case MultiVersionKind::None:
assert(OldMVKind == MultiVersionKind::TargetClones &&
@@ -11681,43 +11709,81 @@ static bool CheckMultiVersionAdditionalDecl(
break;
}
case MultiVersionKind::TargetVersion: {
- const auto *CurTVA = CurFD->getAttr<TargetVersionAttr>();
- if (CurTVA->getName() == NewTVA->getName()) {
- NewFD->setIsMultiVersion();
- Redeclaration = true;
- OldDecl = ND;
- return false;
- }
- llvm::SmallVector<StringRef, 8> CurFeats;
- if (CurTVA) {
+ if (const auto *CurTVA = CurFD->getAttr<TargetVersionAttr>()) {
+ if (CurTVA->getName() == NewTVA->getName()) {
+ NewFD->setIsMultiVersion();
+ Redeclaration = true;
+ OldDecl = ND;
+ return false;
+ }
+ llvm::SmallVector<StringRef, 8> CurFeats;
CurTVA->getFeatures(CurFeats);
llvm::sort(CurFeats);
- }
- if (CurFeats == NewFeats) {
- S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
- S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
- NewFD->setInvalidDecl();
- return true;
+
+ if (CurFeats == NewFeats) {
+ S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
+ S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
+ NewFD->setInvalidDecl();
+ return true;
+ }
+ } else if (const auto *CurClones = CurFD->getAttr<TargetClonesAttr>()) {
+ // Default
+ if (NewFeats.empty())
+ break;
+
+ for (unsigned I = 0; I < CurClones->featuresStrs_size(); ++I) {
+ llvm::SmallVector<StringRef, 8> CurFeats;
+ CurClones->getFeatures(CurFeats, I);
+ llvm::sort(CurFeats);
+
+ if (CurFeats == NewFeats) {
+ S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
+ S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
+ NewFD->setInvalidDecl();
+ return true;
+ }
+ }
}
break;
}
case MultiVersionKind::TargetClones: {
- const auto *CurClones = CurFD->getAttr<TargetClonesAttr>();
+ assert(NewClones && "MultiVersionKind does not match attribute type");
+ if (const auto *CurClones = CurFD->getAttr<TargetClonesAttr>()) {
+ if (CurClones->featuresStrs_size() != NewClones->featuresStrs_size() ||
+ !std::equal(CurClones->featuresStrs_begin(),
+ CurClones->featuresStrs_end(),
+ NewClones->featuresStrs_begin())) {
+ S.Diag(NewFD->getLocation(), diag::err_target_clone_doesnt_match);
+ S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
+ NewFD->setInvalidDecl();
+ return true;
+ }
+ } else if (const auto *CurTVA = CurFD->getAttr<TargetVersionAttr>()) {
+ llvm::SmallVector<StringRef, 8> CurFeats;
+ CurTVA->getFeatures(CurFeats);
+ llvm::sort(CurFeats);
+
+ // Default
+ if (CurFeats.empty())
+ break;
+
+ for (unsigned I = 0; I < NewClones->featuresStrs_size(); ++I) {
+ NewFeats.clear();
+ NewClones->getFeatures(NewFeats, I);
+ llvm::sort(NewFeats);
+
+ if (CurFeats == NewFeats) {
+ S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
+ S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
+ NewFD->setInvalidDecl();
+ return true;
+ }
+ }
+ break;
+ }
Redeclaration = true;
OldDecl = CurFD;
NewFD->setIsMultiVersion();
-
- if (CurClones && NewClones &&
- (CurClones->featuresStrs_size() != NewClones->featuresStrs_size() ||
- !std::equal(CurClones->featuresStrs_begin(),
- CurClones->featuresStrs_end(),
- NewClones->featuresStrs_begin()))) {
- S.Diag(NewFD->getLocation(), diag::err_target_clone_doesnt_match);
- S.Diag(CurFD->getLocation(), diag::note_previous_declaration);
- NewFD->setInvalidDecl();
- return true;
- }
-
return false;
}
case MultiVersionKind::CPUSp...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
The latest ACLE allows it and further clarifies the following
in regards to the combination of the two attributes:
"If the `default` matches with another explicitly provided
version in the same translation unit, then the compiler can
emit only one function instead of the two. The explicitly
provided version shall be preferred."
("default" refers to the default clone here)
ARM-software/acle#310
labrinea
force-pushed
the
allow-mixing-fmv-attributes
branch
from
65bfc47
to
dfef9d0
Compare
jroelofs
reviewed
Mar 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The latest ACLE allows it and further clarifies the following
in regards to the combination of the two attributes:
"If the
defaultmatches with another explicitly providedversion in the same translation unit, then the compiler can
emit only one function instead of the two. The explicitly
provided version shall be preferred."
("default" refers to the default clone here)
ARM-software/acle#310