[flang-commits] [lldb] [libc] [compiler-rt] [llvm] [flang] [libcxx] [lld] [clang] [clang-tools-extra] [Clang] Fix linker error for function multiversioning (PR #71706)
via flang-commits
flang-commits at lists.llvm.org
Tue Dec 5 08:39:56 PST 2023
https://github.com/elizabethandrews updated https://github.com/llvm/llvm-project/pull/71706
>From 534fad70af45a6a22ba2d03f474089e896f4fcd6 Mon Sep 17 00:00:00 2001
From: Elizabeth Andrews <elizabeth.andrews at intel.com>
Date: Thu, 26 Oct 2023 08:53:54 -0700
Subject: [PATCH 1/4] [Clang] Fix linker error for function multiversioning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Currently target_clones attribute results in a linker error when
there are no multi-versioned function declarations in the calling TU.
In the calling TU, the call is generated with the ‘normal’ assembly name.
This does not match any of the versions or the ifunc, since version mangling
includes a .versionstring, and the ifunc includes .ifunc suffix. The linker
error is not seen with GCC since the mangling for the ifunc symbol in GCC
is the ‘normal’ assembly name for function i.e. no ifunc suffix.
This PR removes the .ifunc suffix to match GCC. It also adds alias with
the .ifunc suffix so as to ensure backward compatibility.
The changes exclude aarch64 target because the mangling for default
versions on aarch64 does not include a .default suffix and is the
'normal' assembly name, unlike other targets. It is not clear to me
what the correct behavior for this target is.
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/CodeGen/CodeGenModule.cpp | 35 +++++++++++++++++---
clang/test/CodeGen/attr-target-clones.c | 33 +++++++++++-------
clang/test/CodeGenCXX/attr-target-clones.cpp | 27 +++++++++------
4 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8bac599f88503..9e73fa03a0355 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -550,6 +550,7 @@ Bug Fixes in This Version
Fixes (`#67687 <https://github.com/llvm/llvm-project/issues/67687>`_)
- Fix crash from constexpr evaluator evaluating uninitialized arrays as rvalue.
Fixes (`#67317 <https://github.com/llvm/llvm-project/issues/67317>`_)
+- Fix linker error when using multiversioned function defined in a different TU.
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 2e96fff808113..b54c4296a0f9d 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4098,8 +4098,26 @@ void CodeGenModule::emitMultiVersionFunctions() {
}
llvm::Constant *ResolverConstant = GetOrCreateMultiVersionResolver(GD);
- if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(ResolverConstant))
+ if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(ResolverConstant)) {
ResolverConstant = IFunc->getResolver();
+ // In Aarch64, default versions of multiversioned functions are mangled to
+ // their 'normal' assembly name. This deviates from other targets which
+ // append a '.default' string. As a result we need to continue appending
+ // .ifunc in Aarch64.
+ // FIXME: Should Aarch64 mangling for 'default' multiversion function and
+ // in turn ifunc function match that of other targets?
+ if (FD->isTargetClonesMultiVersion() &&
+ !getTarget().getTriple().isAArch64()) {
+ const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD);
+ llvm::FunctionType *DeclTy = getTypes().GetFunctionType(FI);
+ std::string MangledName = getMangledNameImpl(
+ *this, GD, FD, /*OmitMultiVersionMangling=*/true);
+ auto *Alias = llvm::GlobalAlias::create(
+ DeclTy, 0, llvm::Function::WeakODRLinkage, MangledName + ".ifunc",
+ IFunc, &getModule());
+ SetCommonAttributes(FD, Alias);
+ }
+ }
llvm::Function *ResolverFunc = cast<llvm::Function>(ResolverConstant);
ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
@@ -4266,10 +4284,19 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
// Holds the name of the resolver, in ifunc mode this is the ifunc (which has
// a separate resolver).
std::string ResolverName = MangledName;
- if (getTarget().supportsIFunc())
- ResolverName += ".ifunc";
- else if (FD->isTargetMultiVersion())
+ if (getTarget().supportsIFunc()) {
+ // In Aarch64, default versions of multiversioned functions are mangled to
+ // their 'normal' assembly name. This deviates from other targets which
+ // append a '.default' string. As a result we need to continue appending
+ // .ifunc in Aarch64.
+ // FIXME: Should Aarch64 mangling for 'default' multiversion function and
+ // in turn ifunc function match that of other targets?
+ if (!FD->isTargetClonesMultiVersion() ||
+ getTarget().getTriple().isAArch64())
+ ResolverName += ".ifunc";
+ } else if (FD->isTargetMultiVersion()) {
ResolverName += ".resolver";
+ }
// If the resolver has already been created, just return it.
if (llvm::GlobalValue *ResolverGV = GetGlobalValue(ResolverName))
diff --git a/clang/test/CodeGen/attr-target-clones.c b/clang/test/CodeGen/attr-target-clones.c
index 98ffea40f56d8..0b7c44f26acc6 100644
--- a/clang/test/CodeGen/attr-target-clones.c
+++ b/clang/test/CodeGen/attr-target-clones.c
@@ -16,13 +16,22 @@
// LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
// LINUX: @__cpu_features2 = external dso_local global [3 x i32]
-// LINUX: @internal.ifunc = internal ifunc i32 (), ptr @internal.resolver
-// LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
-// LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
-// LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
-// LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
-// LINUX: @foo_inline2.ifunc = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
-// LINUX: @foo_used_no_defn.ifunc = weak_odr ifunc i32 (), ptr @foo_used_no_defn.resolver
+// LINUX: @internal.ifunc = weak_odr alias i32 (), ptr @internal
+// LINUX: @foo.ifunc = weak_odr alias i32 (), ptr @foo
+// LINUX: @foo_dupes.ifunc = weak_odr alias void (), ptr @foo_dupes
+// LINUX: @unused.ifunc = weak_odr alias void (), ptr @unused
+// LINUX: @foo_inline.ifunc = weak_odr alias i32 (), ptr @foo_inline
+// LINUX: @foo_inline2.ifunc = weak_odr alias i32 (), ptr @foo_inline2
+// LINUX: @foo_used_no_defn.ifunc = weak_odr alias i32 (), ptr @foo_used_no_defn
+// LINUX: @isa_level.ifunc = weak_odr alias i32 (i32), ptr @isa_level
+
+// LINUX: @internal = internal ifunc i32 (), ptr @internal.resolver
+// LINUX: @foo = weak_odr ifunc i32 (), ptr @foo.resolver
+// LINUX: @foo_dupes = weak_odr ifunc void (), ptr @foo_dupes.resolver
+// LINUX: @unused = weak_odr ifunc void (), ptr @unused.resolver
+// LINUX: @foo_inline = weak_odr ifunc i32 (), ptr @foo_inline.resolver
+// LINUX: @foo_inline2 = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
+// LINUX: @foo_used_no_defn = weak_odr ifunc i32 (), ptr @foo_used_no_defn.resolver
static int __attribute__((target_clones("sse4.2, default"))) internal(void) { return 0; }
int use(void) { return internal(); }
@@ -60,7 +69,7 @@ void bar2(void) {
// LINUX: define {{.*}}void @bar2()
// WINDOWS: define dso_local void @bar2()
foo_dupes();
- // LINUX: call void @foo_dupes.ifunc()
+ // LINUX: call void @foo_dupes()
// WINDOWS: call void @foo_dupes()
}
@@ -68,7 +77,7 @@ int bar(void) {
// LINUX: define {{.*}}i32 @bar() #[[DEF:[0-9]+]]
// WINDOWS: define dso_local i32 @bar() #[[DEF:[0-9]+]]
return foo();
- // LINUX: call i32 @foo.ifunc()
+ // LINUX: call i32 @foo()
// WINDOWS: call i32 @foo()
}
@@ -95,8 +104,8 @@ int bar3(void) {
// LINUX: define {{.*}}i32 @bar3()
// WINDOWS: define dso_local i32 @bar3()
return foo_inline() + foo_inline2();
- // LINUX: call i32 @foo_inline.ifunc()
- // LINUX: call i32 @foo_inline2.ifunc()
+ // LINUX: call i32 @foo_inline()
+ // LINUX: call i32 @foo_inline2()
// WINDOWS: call i32 @foo_inline()
// WINDOWS: call i32 @foo_inline2()
}
@@ -134,7 +143,7 @@ int test_foo_used_no_defn(void) {
// LINUX: define {{.*}}i32 @test_foo_used_no_defn()
// WINDOWS: define dso_local i32 @test_foo_used_no_defn()
return foo_used_no_defn();
- // LINUX: call i32 @foo_used_no_defn.ifunc()
+ // LINUX: call i32 @foo_used_no_defn()
// WINDOWS: call i32 @foo_used_no_defn()
}
diff --git a/clang/test/CodeGenCXX/attr-target-clones.cpp b/clang/test/CodeGenCXX/attr-target-clones.cpp
index 86293b98dbbd3..fd2d38062a71e 100644
--- a/clang/test/CodeGenCXX/attr-target-clones.cpp
+++ b/clang/test/CodeGenCXX/attr-target-clones.cpp
@@ -1,13 +1,20 @@
// RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix=LINUX
// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-pc -emit-llvm %s -o - | FileCheck %s --check-prefix=WINDOWS
+// Aliases for ifuncs
+// LINUX: @_Z10overloadedi.ifunc = weak_odr alias i32 (i32), ptr @_Z10overloadedi
+// LINUX: @_Z10overloadedPKc.ifunc = weak_odr alias i32 (ptr), ptr @_Z10overloadedPKc
+// LINUX: @_ZN1CIssE3fooEv.ifunc = weak_odr alias i32 (ptr), ptr @_ZN1CIssE3fooEv
+// LINUX: @_ZN1CIisE3fooEv.ifunc = weak_odr alias i32 (ptr), ptr @_ZN1CIisE3fooEv
+// LINUX: @_ZN1CIdfE3fooEv.ifunc = weak_odr alias i32 (ptr), ptr @_ZN1CIdfE3fooEv
+
// Overloaded ifuncs
-// LINUX: @_Z10overloadedi.ifunc = weak_odr ifunc i32 (i32), ptr @_Z10overloadedi.resolver
-// LINUX: @_Z10overloadedPKc.ifunc = weak_odr ifunc i32 (ptr), ptr @_Z10overloadedPKc.resolver
+// LINUX: @_Z10overloadedi = weak_odr ifunc i32 (i32), ptr @_Z10overloadedi.resolver
+// LINUX: @_Z10overloadedPKc = weak_odr ifunc i32 (ptr), ptr @_Z10overloadedPKc.resolver
// struct 'C' ifuncs, note the 'float, U' one doesn't get one.
-// LINUX: @_ZN1CIssE3fooEv.ifunc = weak_odr ifunc i32 (ptr), ptr @_ZN1CIssE3fooEv.resolver
-// LINUX: @_ZN1CIisE3fooEv.ifunc = weak_odr ifunc i32 (ptr), ptr @_ZN1CIisE3fooEv.resolver
-// LINUX: @_ZN1CIdfE3fooEv.ifunc = weak_odr ifunc i32 (ptr), ptr @_ZN1CIdfE3fooEv.resolver
+// LINUX: @_ZN1CIssE3fooEv = weak_odr ifunc i32 (ptr), ptr @_ZN1CIssE3fooEv.resolver
+// LINUX: @_ZN1CIisE3fooEv = weak_odr ifunc i32 (ptr), ptr @_ZN1CIisE3fooEv.resolver
+// LINUX: @_ZN1CIdfE3fooEv = weak_odr ifunc i32 (ptr), ptr @_ZN1CIdfE3fooEv.resolver
int __attribute__((target_clones("sse4.2", "default"))) overloaded(int) { return 1; }
// LINUX: define {{.*}}i32 @_Z10overloadedi.sse4.2.0(i32{{.+}})
@@ -37,10 +44,10 @@ int __attribute__((target_clones("arch=ivybridge", "default"))) overloaded(const
void use_overloaded() {
overloaded(1);
- // LINUX: call noundef i32 @_Z10overloadedi.ifunc
+ // LINUX: call noundef i32 @_Z10overloadedi
// WINDOWS: call noundef i32 @"?overloaded@@YAHH at Z"
overloaded(nullptr);
- // LINUX: call noundef i32 @_Z10overloadedPKc.ifunc
+ // LINUX: call noundef i32 @_Z10overloadedPKc
// WINDOWS: call noundef i32 @"?overloaded@@YAHPEBD at Z"
}
@@ -64,11 +71,11 @@ int __attribute__((target_clones("sse4.2", "default"))) foo(){ return 3;}
void uses_specialized() {
C<short, short> c;
c.foo();
- // LINUX: call noundef i32 @_ZN1CIssE3fooEv.ifunc(ptr
+ // LINUX: call noundef i32 @_ZN1CIssE3fooEv(ptr
// WINDOWS: call noundef i32 @"?foo@?$C at FF@@QEAAHXZ"(ptr
C<int, short> c2;
c2.foo();
- // LINUX: call noundef i32 @_ZN1CIisE3fooEv.ifunc(ptr
+ // LINUX: call noundef i32 @_ZN1CIisE3fooEv(ptr
// WINDOWS: call noundef i32 @"?foo@?$C at HF@@QEAAHXZ"(ptr
C<float, short> c3;
c3.foo();
@@ -77,7 +84,7 @@ void uses_specialized() {
// WINDOWS: call noundef i32 @"?foo@?$C at MF@@QEAAHXZ"(ptr
C<double, float> c4;
c4.foo();
- // LINUX: call noundef i32 @_ZN1CIdfE3fooEv.ifunc(ptr
+ // LINUX: call noundef i32 @_ZN1CIdfE3fooEv(ptr
// WINDOWS: call noundef i32 @"?foo@?$C at NM@@QEAAHXZ"(ptr
}
>From 362f0cbb435c5a37969c2d23bda18063af75d500 Mon Sep 17 00:00:00 2001
From: Elizabeth Andrews <elizabeth.andrews at intel.com>
Date: Tue, 14 Nov 2023 12:00:13 -0800
Subject: [PATCH 2/4] Use getMultiversionLinkage() for Linkage when generating
alias
---
clang/lib/CodeGen/CodeGenModule.cpp | 4 ++--
clang/test/CodeGen/attr-target-clones.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index b54c4296a0f9d..b6f26278a25a7 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4113,8 +4113,8 @@ void CodeGenModule::emitMultiVersionFunctions() {
std::string MangledName = getMangledNameImpl(
*this, GD, FD, /*OmitMultiVersionMangling=*/true);
auto *Alias = llvm::GlobalAlias::create(
- DeclTy, 0, llvm::Function::WeakODRLinkage, MangledName + ".ifunc",
- IFunc, &getModule());
+ DeclTy, 0, getMultiversionLinkage(*this, GD),
+ MangledName + ".ifunc", IFunc, &getModule());
SetCommonAttributes(FD, Alias);
}
}
diff --git a/clang/test/CodeGen/attr-target-clones.c b/clang/test/CodeGen/attr-target-clones.c
index 0b7c44f26acc6..2bcec08d52bf9 100644
--- a/clang/test/CodeGen/attr-target-clones.c
+++ b/clang/test/CodeGen/attr-target-clones.c
@@ -16,7 +16,7 @@
// LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
// LINUX: @__cpu_features2 = external dso_local global [3 x i32]
-// LINUX: @internal.ifunc = weak_odr alias i32 (), ptr @internal
+// LINUX: @internal.ifunc = internal alias i32 (), ptr @internal
// LINUX: @foo.ifunc = weak_odr alias i32 (), ptr @foo
// LINUX: @foo_dupes.ifunc = weak_odr alias void (), ptr @foo_dupes
// LINUX: @unused.ifunc = weak_odr alias void (), ptr @unused
>From 763221c1637b5004f78e39916028d51fed769531 Mon Sep 17 00:00:00 2001
From: Elizabeth Andrews <elizabeth.andrews at intel.com>
Date: Fri, 1 Dec 2023 08:26:31 -0800
Subject: [PATCH 3/4] Apply review comments
---
clang/docs/ReleaseNotes.rst | 4 +++-
clang/include/clang/Basic/AttrDocs.td | 6 ++++++
clang/lib/CodeGen/CodeGenModule.cpp | 3 +++
clang/test/CodeGen/attr-target-clones.c | 1 +
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index af90e6dcc7fa0..2dc7114a3587d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -555,7 +555,9 @@ Bug Fixes in This Version
Fixes (`#67687 <https://github.com/llvm/llvm-project/issues/67687>`_)
- Fix crash from constexpr evaluator evaluating uninitialized arrays as rvalue.
Fixes (`#67317 <https://github.com/llvm/llvm-project/issues/67317>`_)
-- Fix linker error when using multiversioned function defined in a different TU.
+- Fix the name of the ifunc symbol emitted for multiversion functions declared with the
+ ``target_clones`` attribute. This addresses a linker error that would otherwise occur
+ when these functions are referenced from other TUs
- Fixed an issue that a benign assertion might hit when instantiating a pack expansion
inside a lambda. (`#61460 <https://github.com/llvm/llvm-project/issues/61460>`_)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index fa6f6acd0c30e..c54c9e690bd90 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2515,6 +2515,12 @@ example, the following will emit 4 versions of the function:
__attribute__((target_clones("arch=atom,avx2","arch=ivybridge","default")))
void foo() {}
+Dispatch is done via ``ifunc`` mechanism. The assembler name of the indirect
+function is the original assembler name of the multiversioned function. For
+backward compatibility, an alias to this function is currently generated
+with an ``.ifunc`` suffix. This symbol is deprecated and will be removed
+in the future.
+
}];
}
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 2b43fe4fd77bf..9e6fef33efde2 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4128,6 +4128,9 @@ void CodeGenModule::emitMultiVersionFunctions() {
llvm::FunctionType *DeclTy = getTypes().GetFunctionType(FI);
std::string MangledName = getMangledNameImpl(
*this, GD, FD, /*OmitMultiVersionMangling=*/true);
+ // In prior versions of Clang, the mangling for ifuncs incorrectly
+ // included an .ifunc suffix. This alias is generated for backward
+ // compatibility and should be deprecated in the future.
auto *Alias = llvm::GlobalAlias::create(
DeclTy, 0, getMultiversionLinkage(*this, GD),
MangledName + ".ifunc", IFunc, &getModule());
diff --git a/clang/test/CodeGen/attr-target-clones.c b/clang/test/CodeGen/attr-target-clones.c
index 2bcec08d52bf9..4b99914031b10 100644
--- a/clang/test/CodeGen/attr-target-clones.c
+++ b/clang/test/CodeGen/attr-target-clones.c
@@ -32,6 +32,7 @@
// LINUX: @foo_inline = weak_odr ifunc i32 (), ptr @foo_inline.resolver
// LINUX: @foo_inline2 = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
// LINUX: @foo_used_no_defn = weak_odr ifunc i32 (), ptr @foo_used_no_defn.resolver
+// LINUX: @isa_level = weak_odr ifunc i32 (i32), ptr @isa_level.resolver
static int __attribute__((target_clones("sse4.2, default"))) internal(void) { return 0; }
int use(void) { return internal(); }
>From fcc6db85a76d104ca4188ee7902314ad4f2c6738 Mon Sep 17 00:00:00 2001
From: Elizabeth Andrews <elizabeth.andrews at intel.com>
Date: Tue, 5 Dec 2023 08:39:02 -0800
Subject: [PATCH 4/4] Apply review comments
---
clang/include/clang/Basic/AttrDocs.td | 2 +-
clang/lib/CodeGen/CodeGenModule.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 4148b63213363..a2e850d8f8e1a 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -2518,7 +2518,7 @@ example, the following will emit 4 versions of the function:
Dispatch is done via ``ifunc`` mechanism. The assembler name of the indirect
function is the original assembler name of the multiversioned function. For
backward compatibility, an alias to this function is currently generated
-with an ``.ifunc`` suffix. This symbol is deprecated and will be removed
+with an ``.ifunc`` suffix. This symbol is deprecated and may be removed
in the future.
}];
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 32b760704dfcc..6a20723bf2bca 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4194,7 +4194,7 @@ void CodeGenModule::emitMultiVersionFunctions() {
*this, GD, FD, /*OmitMultiVersionMangling=*/true);
// In prior versions of Clang, the mangling for ifuncs incorrectly
// included an .ifunc suffix. This alias is generated for backward
- // compatibility and should be deprecated in the future.
+ // compatibility. It is deprecated, and may be removed in the future.
auto *Alias = llvm::GlobalAlias::create(
DeclTy, 0, getMultiversionLinkage(*this, GD),
MangledName + ".ifunc", IFunc, &getModule());
More information about the flang-commits
mailing list