[CodeGen] Function multi-versioning: don't set comdat for internal linkage resolvers

[MaskRay]

Summary

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

[erichkeane]

Needs a release note. I don't have a problem with the patch, but don't really understand how COMDAT works/linking works, but since you're basically the person I'd tell someone else to ask, I guess I'll trust you know what you're doing :)

Please add the release note while committing.

[MaskRay]

add release note

[ilinpv]

clang/docs/ReleaseNotes.rst

208

If I read the code right it applies for function multi-versioning using target_version and target_clones attributes ( AArch64 ) as well, doesn't it?

[MaskRay]

clang/docs/ReleaseNotes.rst

208

Yes, this patch fixed target_version as well.

Perhaps update_cc_test_checks.py should be updated to test comdat as well.

I'll adjust the release note to:

For function multi-versioning using the `target, target_clones, target_version` ...

clang/docs/ReleaseNotes.rst

 - Fixed an assertion if a function has cleanups and fatal erors.
   (`#48974 <https://github.com/llvm/llvm-project/issues/48974>`_)
 - Clang now emits an error if it is not possible to deduce array size for a
   variable with incomplete array type.
   (`#37257 <https://github.com/llvm/llvm-project/issues/37257>`_)
 - Clang's ``-Wunused-private-field`` no longer warns on fields whose type is
   declared with ``[[maybe_unused]]``.
   (`#61334 <https://github.com/llvm/llvm-project/issues/61334>`_)
+- For function multi-versioning using the ``target`` or ``target_clones``
+  attributes, remove comdat for internal linkage functions.
+  (`#65114 <https://github.com/llvm/llvm-project/issues/65114>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/lib/CodeGen/CodeGenModule.cpp

for (GlobalDecl GD : MVFuncsToEmit) {
 
     llvm::Constant *ResolverConstant = GetOrCreateMultiVersionResolver(GD);
     if (auto *IFunc = dyn_cast<llvm::GlobalIFunc>(ResolverConstant))
       ResolverConstant = IFunc->getResolver();
     llvm::Function *ResolverFunc = cast<llvm::Function>(ResolverConstant);
 
     ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
 
-    if (supportsCOMDAT())
+    if (!ResolverFunc->hasLocalLinkage() && supportsCOMDAT())
       ResolverFunc->setComdat(
           getModule().getOrInsertComdat(ResolverFunc->getName()));
 
     const TargetInfo &TI = getTarget();
     llvm::stable_sort(
         Options, [&TI](const CodeGenFunction::MultiVersionResolverOption &LHS,
                        const CodeGenFunction::MultiVersionResolverOption &RHS) {
           return TargetMVPriority(TI, LHS) > TargetMVPriority(TI, RHS);

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

 // WINDOWS: $foo_dupes = comdat any
 // WINDOWS: $unused = comdat any
 // WINDOWS: $foo_inline = comdat any
 // WINDOWS: $foo_inline2 = comdat any
 
 // 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
 
+static int __attribute__((target_clones("sse4.2, default"))) internal(void) { return 0; }
+int use(void) { return internal(); }
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @internal.resolver() {
+// WINDOWS: define internal i32 @internal() {
+
 int __attribute__((target_clones("sse4.2, default"))) foo(void) { return 0; }
 // LINUX: define {{.*}}i32 @foo.sse4.2.0()
 // LINUX: define {{.*}}i32 @foo.default.1()
 // LINUX: define weak_odr ptr @foo.resolver() comdat
 // LINUX: ret ptr @foo.sse4.2.0
 // LINUX: ret ptr @foo.default.1
 
 // WINDOWS: define dso_local i32 @foo.sse4.2.0()

 // LINUX: declare i32 @foo_used_no_defn.sse4.2.0()
 
 // WINDOWS: declare dso_local i32 @foo_used_no_defn.default.1()
 // WINDOWS: declare dso_local i32 @foo_used_no_defn.sse4.2.0()
 
 
 // CHECK: attributes #[[SSE42]] =
 // CHECK-SAME: "target-features"="+crc32,+cx8,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
-// CHECK: attributes #[[DEF]] =
-// Don't bother checking features, we verified it is the same as a normal function.
 // CHECK: attributes #[[SB]] =
 // 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

 int __attribute__((target("arch=lunarlake"))) foo(void) {return 23;}
 int __attribute__((target("arch=gracemont"))) foo(void) {return 24;}
 int __attribute__((target("default"))) foo(void) { return 2; }
 
 int bar(void) {
   return foo();
 }
 
+static int __attribute__((target("arch=meteorlake"))) foo_internal(void) {return 15;}
+static int __attribute__((target("default"))) foo_internal(void) { return 2; }
+
+int bar1(void) {
+  return foo_internal();
+}
+
 inline int __attribute__((target("sse4.2"))) foo_inline(void) { return 0; }
 inline int __attribute__((target("arch=sandybridge"))) foo_inline(void);
 inline int __attribute__((target("arch=ivybridge"))) foo_inline(void) {return 1;}
 inline int __attribute__((target("default"))) foo_inline(void) { return 2; }
 
 int bar2(void) {
   return foo_inline();
 }

 // WINDOWS: $pr50025b = comdat any
 
 
 // LINUX: @llvm.compiler.used = appending global [2 x ptr] [ptr @foo_used, ptr @foo_used2.avx_sse4.2], section "llvm.metadata"
 // WINDOWS: @llvm.used = appending global [2 x ptr] [ptr @foo_used, ptr @foo_used2.avx_sse4.2], section "llvm.metadata"
 
 
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
+// LINUX: @foo_internal.ifunc = internal ifunc i32 (), ptr @foo_internal.resolver
 // LINUX: @foo_inline.ifunc = weak_odr ifunc i32 (), ptr @foo_inline.resolver
 // LINUX: @foo_decls.ifunc = weak_odr ifunc void (), ptr @foo_decls.resolver
 // LINUX: @foo_multi.ifunc = weak_odr ifunc void (i32, double), ptr @foo_multi.resolver
 // LINUX: @fwd_decl_default.ifunc = weak_odr ifunc i32 (), ptr @fwd_decl_default.resolver
 // LINUX: @fwd_decl_avx.ifunc = weak_odr ifunc i32 (), ptr @fwd_decl_avx.resolver
 
 // LINUX: define{{.*}} i32 @foo.sse4.2()
 // LINUX: ret i32 0

 
 // WINDOWS: define weak_odr dso_local i32 @foo.resolver() comdat
 // WINDOWS: call void @__cpu_indicator_init()
 // WINDOWS: call i32 @foo.arch_sandybridge
 // WINDOWS: call i32 @foo.arch_ivybridge
 // WINDOWS: call i32 @foo.sse4.2
 // WINDOWS: call i32 @foo
 
+/// Internal linkage resolvers do not use comdat.
+// LINUX: define internal ptr @foo_internal.resolver() {
+
+// WINDOWS: define internal i32 @foo_internal.resolver() {
+
 // LINUX: define{{.*}} i32 @bar2()
 // LINUX: call i32 @foo_inline.ifunc()
 
 // WINDOWS: define dso_local i32 @bar2()
 // WINDOWS: call i32 @foo_inline.resolver()
 
 // LINUX: define weak_odr ptr @foo_inline.resolver() comdat
 // LINUX: call void @__cpu_indicator_init()