Skip to content

Commit 651b2fb

Browse files
committed
[CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers
For function multi-versioning using the target or target_clones function attributes, currently we incorrectly set comdat for internal linkage resolvers. This is problematic for ELF linkers as GRP_COMDAT deduplication will kick in even with STB_LOCAL signature (https://groups.google.com/g/generic-abi/c/2X6mR-s2zoc "GRP_COMDAT group with STB_LOCAL signature"). In short, two `__attribute((target_clones(...))) static void foo()` in two translation units will be deduplicated. Fix this. Fix #65114 Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158963
1 parent 3c9988f commit 651b2fb

File tree

4 files changed

+24
-3
lines changed

4 files changed

+24
-3
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ Bug Fixes in This Version
205205
- Clang's ``-Wunused-private-field`` no longer warns on fields whose type is
206206
declared with ``[[maybe_unused]]``.
207207
(`#61334 <https://github.com/llvm/llvm-project/issues/61334>`_)
208+
- For function multi-versioning using the ``target`` or ``target_clones``
209+
attributes, remove comdat for internal linkage functions.
210+
(`#65114 <https://github.com/llvm/llvm-project/issues/65114>`_)
208211

209212
Bug Fixes to Compiler Builtins
210213
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4052,7 +4052,7 @@ void CodeGenModule::emitMultiVersionFunctions() {
40524052

40534053
ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
40544054

4055-
if (supportsCOMDAT())
4055+
if (!ResolverFunc->hasLocalLinkage() && supportsCOMDAT())
40564056
ResolverFunc->setComdat(
40574057
getModule().getOrInsertComdat(ResolverFunc->getName()));
40584058

clang/test/CodeGen/attr-target-clones.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,20 @@
1616
// LINUX: @__cpu_model = external dso_local global { i32, i32, i32, [1 x i32] }
1717
// LINUX: @__cpu_features2 = external dso_local global [3 x i32]
1818

19+
// LINUX: @internal.ifunc = internal ifunc i32 (), ptr @internal.resolver
1920
// LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
2021
// LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
2122
// LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
2223
// LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
2324
// LINUX: @foo_inline2.ifunc = weak_odr ifunc i32 (), ptr @foo_inline2.resolver
2425
// LINUX: @foo_used_no_defn.ifunc = weak_odr ifunc i32 (), ptr @foo_used_no_defn.resolver
2526

27+
static int __attribute__((target_clones("sse4.2, default"))) internal(void) { return 0; }
28+
int use(void) { return internal(); }
29+
/// Internal linkage resolvers do not use comdat.
30+
// LINUX: define internal ptr @internal.resolver() {
31+
// WINDOWS: define internal i32 @internal() {
32+
2633
int __attribute__((target_clones("sse4.2, default"))) foo(void) { return 0; }
2734
// LINUX: define {{.*}}i32 @foo.sse4.2.0()
2835
// LINUX: define {{.*}}i32 @foo.default.1()
@@ -192,7 +199,5 @@ int isa_level(int) { return 0; }
192199

193200
// CHECK: attributes #[[SSE42]] =
194201
// CHECK-SAME: "target-features"="+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
195-
// CHECK: attributes #[[DEF]] =
196-
// Don't bother checking features, we verified it is the same as a normal function.
197202
// CHECK: attributes #[[SB]] =
198203
// CHECK-SAME: "target-features"="+avx,+cmov,+crc32,+cx16,+cx8,+fxsr,+mmx,+pclmul,+popcnt,+sahf,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"

clang/test/CodeGen/attr-target-mv.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ int bar(void) {
3232
return foo();
3333
}
3434

35+
static int __attribute__((target("arch=meteorlake"))) foo_internal(void) {return 15;}
36+
static int __attribute__((target("default"))) foo_internal(void) { return 2; }
37+
38+
int bar1(void) {
39+
return foo_internal();
40+
}
41+
3542
inline int __attribute__((target("sse4.2"))) foo_inline(void) { return 0; }
3643
inline int __attribute__((target("arch=sandybridge"))) foo_inline(void);
3744
inline int __attribute__((target("arch=ivybridge"))) foo_inline(void) {return 1;}
@@ -128,6 +135,7 @@ void calls_pr50025c(void) { pr50025c(); }
128135

129136

130137
// LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
138+
// LINUX: @foo_internal.ifunc = internal ifunc i32 (), ptr @foo_internal.resolver
131139
// LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
132140
// LINUX: @foo_decls.ifunc = weak_odr ifunc void (), ptr @foo_decls.resolver
133141
// LINUX: @foo_multi.ifunc = weak_odr ifunc void (i32, double), ptr @foo_multi.resolver
@@ -254,6 +262,11 @@ void calls_pr50025c(void) { pr50025c(); }
254262
// WINDOWS: call i32 @foo.sse4.2
255263
// WINDOWS: call i32 @foo
256264

265+
/// Internal linkage resolvers do not use comdat.
266+
// LINUX: define internal ptr @foo_internal.resolver() {
267+
268+
// WINDOWS: define internal i32 @foo_internal.resolver() {
269+
257270
// LINUX: define{{.*}} i32 @bar2()
258271
// LINUX: call i32 @foo_inline.ifunc()
259272

0 commit comments

Comments
 (0)