Skip to content

[FMV][AArch64] Do not emit ifunc resolver on use. #97761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 18, 2024
Merged

[FMV][AArch64] Do not emit ifunc resolver on use. #97761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b338e4d
[FMV][AArch64] Do not emit ifunc resolver on use.
labrinea Jul 4, 2024
ecf08e0
Changes from last revision:
labrinea Jul 8, 2024
49f4727
Merge branch 'main' into fmv-do-not-emit-resolver-on-use
labrinea Jul 17, 2024
9cabdf3
Changes from last revision:
labrinea Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 50 additions & 47 deletions clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3796,8 +3796,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
// Forward declarations are emitted lazily on first use.
if (!FD->doesThisDeclarationHaveABody()) {
if (!FD->doesDeclarationForceExternallyVisibleDefinition() &&
(!FD->isMultiVersion() ||
!FD->getASTContext().getTargetInfo().getTriple().isAArch64()))
(!FD->isMultiVersion() || !getTarget().getTriple().isAArch64()))
return;

StringRef MangledName = getMangledName(GD);
Expand Down Expand Up @@ -4191,23 +4190,6 @@ llvm::GlobalValue::LinkageTypes getMultiversionLinkage(CodeGenModule &CGM,
return llvm::GlobalValue::WeakODRLinkage;
}

static FunctionDecl *createDefaultTargetVersionFrom(const FunctionDecl *FD) {
auto *DeclCtx = const_cast<DeclContext *>(FD->getDeclContext());
TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
StorageClass SC = FD->getStorageClass();
DeclarationName Name = FD->getNameInfo().getName();

FunctionDecl *NewDecl =
FunctionDecl::Create(FD->getASTContext(), DeclCtx, FD->getBeginLoc(),
FD->getEndLoc(), Name, TInfo->getType(), TInfo, SC);

NewDecl->setIsMultiVersion();
NewDecl->addAttr(TargetVersionAttr::CreateImplicit(
NewDecl->getASTContext(), "default", NewDecl->getSourceRange()));

return NewDecl;
}

void CodeGenModule::emitMultiVersionFunctions() {
std::vector<GlobalDecl> MVFuncsToEmit;
MultiVersionFuncs.swap(MVFuncsToEmit);
Expand All @@ -4234,29 +4216,30 @@ void CodeGenModule::emitMultiVersionFunctions() {
return cast<llvm::Function>(Func);
};

bool HasDefaultDecl = !FD->isTargetVersionMultiVersion();
bool ShouldEmitResolver =
!getContext().getTargetInfo().getTriple().isAArch64();
// For AArch64, a resolver is only emitted if a function marked with
// target_version("default")) or target_clones() is present and defined
// in this TU. For other architectures it is always emitted.
bool ShouldEmitResolver = !getTarget().getTriple().isAArch64();
SmallVector<CodeGenFunction::MultiVersionResolverOption, 10> Options;

getContext().forEachMultiversionedFunctionVersion(
FD, [&](const FunctionDecl *CurFD) {
llvm::SmallVector<StringRef, 8> Feats;
bool IsDefined = CurFD->doesThisDeclarationHaveABody();

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);
if (TVA->isDefaultVersion() && IsDefined)
ShouldEmitResolver = true;
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();
if (IsDefined)
ShouldEmitResolver = true;
for (unsigned I = 0; I < TC->featuresStrs_size(); ++I) {
if (!TC->isFirstOfVersion(I))
continue;
Expand All @@ -4282,13 +4265,6 @@ void CodeGenModule::emitMultiVersionFunctions() {
if (!ShouldEmitResolver)
continue;

if (!HasDefaultDecl) {
FunctionDecl *NewFD = createDefaultTargetVersionFrom(FD);
llvm::Function *Func = createFunction(NewFD);
llvm::SmallVector<StringRef, 1> Feats;
Options.emplace_back(Func, /*Architecture*/ "", Feats);
}

llvm::Constant *ResolverConstant = GetOrCreateMultiVersionResolver(GD);
if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(ResolverConstant)) {
ResolverConstant = IFunc->getResolver();
Expand Down Expand Up @@ -4339,6 +4315,14 @@ void CodeGenModule::emitMultiVersionFunctions() {
emitMultiVersionFunctions();
}

static void replaceDeclarationWith(llvm::GlobalValue *Old,
llvm::Constant *New) {
assert(cast<llvm::Function>(Old)->isDeclaration() && "Not a declaration");
New->takeName(Old);
Old->replaceAllUsesWith(New);
Old->eraseFromParent();
}

void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
const auto *FD = cast<FunctionDecl>(GD.getDecl());
assert(FD && "Not a FunctionDecl?");
Expand Down Expand Up @@ -4443,12 +4427,9 @@ void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
// Fix up function declarations that were created for cpu_specific before
// cpu_dispatch was known
if (!isa<llvm::GlobalIFunc>(IFunc)) {
assert(cast<llvm::Function>(IFunc)->isDeclaration());
auto *GI = llvm::GlobalIFunc::create(DeclTy, 0, Linkage, "", ResolverFunc,
&getModule());
GI->takeName(IFunc);
IFunc->replaceAllUsesWith(GI);
IFunc->eraseFromParent();
replaceDeclarationWith(IFunc, GI);
IFunc = GI;
}

Expand Down Expand Up @@ -4478,7 +4459,8 @@ void CodeGenModule::AddDeferredMultiVersionResolverToEmit(GlobalDecl GD) {
}

/// If a dispatcher for the specified mangled name is not in the module, create
/// and return an llvm Function with the specified type.
/// and return it. The dispatcher is either an llvm Function with the specified
/// type, or a global ifunc.
llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
const auto *FD = cast<FunctionDecl>(GD.getDecl());
assert(FD && "Not a FunctionDecl?");
Expand Down Expand Up @@ -4506,8 +4488,15 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
ResolverName += ".resolver";
}

// If the resolver has already been created, just return it.
if (llvm::GlobalValue *ResolverGV = GetGlobalValue(ResolverName))
// If the resolver has already been created, just return it. This lookup may
// yield a function declaration instead of a resolver on AArch64. That is
// because we didn't know whether a resolver will be generated when we first
// encountered a use of the symbol named after this resolver. Therefore,
// targets which support ifuncs should not return here unless we actually
// found an ifunc.
llvm::GlobalValue *ResolverGV = GetGlobalValue(ResolverName);
if (ResolverGV &&
(isa<llvm::GlobalIFunc>(ResolverGV) || !getTarget().supportsIFunc()))
return ResolverGV;

const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD);
Expand All @@ -4533,7 +4522,8 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
"", Resolver, &getModule());
GIF->setName(ResolverName);
SetCommonAttributes(FD, GIF);

if (ResolverGV)
replaceDeclarationWith(ResolverGV, GIF);
return GIF;
}

Expand All @@ -4542,6 +4532,8 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
assert(isa<llvm::GlobalValue>(Resolver) &&
"Resolver should be created for the first time");
SetCommonAttributes(FD, cast<llvm::GlobalValue>(Resolver));
if (ResolverGV)
replaceDeclarationWith(ResolverGV, Resolver);
return Resolver;
}

Expand Down Expand Up @@ -4571,6 +4563,7 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
ForDefinition_t IsForDefinition) {
const Decl *D = GD.getDecl();

std::string NameWithoutMultiVersionMangling;
// Any attempts to use a MultiVersion function should result in retrieving
// the iFunc instead. Name Mangling will handle the rest of the changes.
if (const FunctionDecl *FD = cast_or_null<FunctionDecl>(D)) {
Expand All @@ -4592,14 +4585,24 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(

if (FD->isMultiVersion()) {
UpdateMultiVersionNames(GD, FD, MangledName);
if (FD->getASTContext().getTargetInfo().getTriple().isAArch64() &&
!FD->isUsed())
AddDeferredMultiVersionResolverToEmit(GD);
else if (!IsForDefinition)
return GetOrCreateMultiVersionResolver(GD);
if (!IsForDefinition) {
// On AArch64 we do not immediatelly emit an ifunc resolver when a
// function is used. Instead we defer the emission until we see a
// default definition. In the meantime we just reference the symbol
// without FMV mangling (it may or may not be replaced later).
if (getTarget().getTriple().isAArch64()) {
AddDeferredMultiVersionResolverToEmit(GD);
NameWithoutMultiVersionMangling = getMangledNameImpl(
*this, GD, FD, /*OmitMultiVersionMangling=*/true);
} else
return GetOrCreateMultiVersionResolver(GD);
}
}
}

if (!NameWithoutMultiVersionMangling.empty())
MangledName = NameWithoutMultiVersionMangling;

// Lookup the entry, lazily creating it if necessary.
llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
if (Entry) {
Expand Down
111 changes: 111 additions & 0 deletions clang/test/CodeGen/aarch64-fmv-resolver-emission.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -emit-llvm -o - %s | FileCheck %s

// CHECK: @used_before_default_def = weak_odr ifunc void (), ptr @used_before_default_def.resolver
// CHECK: @used_after_default_def = weak_odr ifunc void (), ptr @used_after_default_def.resolver
// CHECK-NOT: @used_before_default_decl = weak_odr ifunc void (), ptr @used_before_default_decl.resolver
// CHECK-NOT: @used_after_default_decl = weak_odr ifunc void (), ptr @used_after_default_decl.resolver
// CHECK-NOT: @used_no_default = weak_odr ifunc void (), ptr @used_no_default.resolver
// CHECK-NOT: @not_used_no_default = weak_odr ifunc void (), ptr @not_used_no_default.resolver
// CHECK: @not_used_with_default = weak_odr ifunc void (), ptr @not_used_with_default.resolver


// Test that an ifunc is generated and used when the default
// version is defined after the first use of the function.
//
__attribute__((target_version("aes"))) void used_before_default_def(void) {}
// CHECK-LABEL: define dso_local void @used_before_default_def._Maes(
//
void call_before_def(void) { used_before_default_def(); }
// CHECK-LABEL: define dso_local void @call_before_def(
// CHECK: call void @used_before_default_def()
//
__attribute__((target_version("default"))) void used_before_default_def(void) {}
// CHECK-LABEL: define dso_local void @used_before_default_def.default(
//
// CHECK-NOT: declare void @used_before_default_def(


// Test that an ifunc is generated and used when the default
// version is defined before the first use of the function.
//
__attribute__((target_version("aes"))) void used_after_default_def(void) {}
// CHECK-LABEL: define dso_local void @used_after_default_def._Maes(
//
__attribute__((target_version("default"))) void used_after_default_def(void) {}
// CHECK-LABEL: define dso_local void @used_after_default_def.default(
//
void call_after_def(void) { used_after_default_def(); }
// CHECK-LABEL: define dso_local void @call_after_def(
// CHECK: call void @used_after_default_def()
//
// CHECK-NOT: declare void @used_after_default_def(


// Test that an unmagled declaration is generated and used when the
// default version is declared after the first use of the function.
//
__attribute__((target_version("aes"))) void used_before_default_decl(void) {}
// CHECK-LABEL: define dso_local void @used_before_default_decl._Maes(
//
void call_before_decl(void) { used_before_default_decl(); }
// CHECK-LABEL: define dso_local void @call_before_decl(
// CHECK: call void @used_before_default_decl()
//
__attribute__((target_version("default"))) void used_before_default_decl(void);
// CHECK: declare void @used_before_default_decl()


// Test that an unmagled declaration is generated and used when the
// default version is declared before the first use of the function.
//
__attribute__((target_version("aes"))) void used_after_default_decl(void) {}
// CHECK-LABEL: define dso_local void @used_after_default_decl._Maes(
//
__attribute__((target_version("default"))) void used_after_default_decl(void);
// CHECK: declare void @used_after_default_decl()
//
void call_after_decl(void) { used_after_default_decl(); }
// CHECK-LABEL: define dso_local void @call_after_decl(
// CHECK: call void @used_after_default_decl()


// Test that an unmagled declaration is generated and used when
// the default version is not present.
//
__attribute__((target_version("aes"))) void used_no_default(void) {}
// CHECK-LABEL: define dso_local void @used_no_default._Maes(
//
void call_no_default(void) { used_no_default(); }
// CHECK-LABEL: define dso_local void @call_no_default(
// CHECK: call void @used_no_default()
//
// CHECK: declare void @used_no_default()


// Test that neither an ifunc nor a declaration is generated if the default
// definition is missing since the versioned function is not used.
//
__attribute__((target_version("aes"))) void not_used_no_default(void) {}
// CHECK-LABEL: define dso_local void @not_used_no_default._Maes(
//
// CHECK-NOT: declare void @not_used_no_default(


// Test that an ifunc is generated if the default version is defined but not used.
//
__attribute__((target_version("aes"))) void not_used_with_default(void) {}
// CHECK-LABEL: define dso_local void @not_used_with_default._Maes(
//
__attribute__((target_version("default"))) void not_used_with_default(void) {}
// CHECK-LABEL: define dso_local void @not_used_with_default.default(
//
// CHECK-NOT: declare void @not_used_with_default(


// CHECK: define weak_odr ptr @used_before_default_def.resolver()
// CHECK: define weak_odr ptr @used_after_default_def.resolver()
// CHECK-NOT: define weak_odr ptr @used_before_default_decl.resolver(
// CHECK-NOT: define weak_odr ptr @used_after_default_decl.resolver(
// CHECK-NOT: define weak_odr ptr @used_no_default.resolver(
// CHECK-NOT: define weak_odr ptr @not_used_no_default.resolver(
// CHECK: define weak_odr ptr @not_used_with_default.resolver()
6 changes: 3 additions & 3 deletions clang/test/CodeGen/aarch64-mixed-target-attributes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,9 @@ __attribute__((target_version("jscvt"))) int default_def_with_version_decls(void
// CHECK: attributes #[[ATTR3]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+lse,-v9.5a" }
// CHECK: attributes #[[ATTR4]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+neon,+rdm,-v9.5a" }
// CHECK: attributes #[[ATTR5:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+dotprod,+fp-armv8,+neon,-v9.5a" }
// CHECK: attributes #[[ATTR6:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+jsconv,+neon,-v9.5a" }
// CHECK: attributes #[[ATTR7:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="-v9.5a" }
// CHECK: attributes #[[ATTR8:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+lse,-v9.5a" }
// CHECK: attributes #[[ATTR6:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="-v9.5a" }
// CHECK: attributes #[[ATTR7:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+lse,-v9.5a" }
// CHECK: attributes #[[ATTR8:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+jsconv,+neon,-v9.5a" }
Comment on lines +264 to +266
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attributes are not actually matched against anything, is there any point testing them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention is to check that we are correctly enabling the FMV implied features. If those are fully covered in clang/test/CodeGen/aarch64-fmv-dependencies.c, which was added later, then I guess we can remove them from every other codegen test file. But this should be done in a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way do you know how to disable the attribute checks?

//.
// CHECK-NOFMV: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="-fmv" }
//.
Expand Down
Loading
Loading