[llvm] [Linker] Do not keep a private member of a non-prevailing comdat group (PR #69143)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lto

Author: Igor Kudrin (igorkudrin)

<details>
<summary>Changes</summary>

`IRMover` links in referenced private global values unconditionally, see `IRLinker::shouldLink()`. If they are part of a non-prevailing comdat, this leads to duplication of the values. Full and Thin LTO avoid duplication by changing the linkage of members of non-prevailing comdat groups to `available_externally`, which was implemented in https://reviews.llvm.org/D34803 and https://reviews.llvm.org/D135427. This patch does the same for `Linker`, but limits the effect only to private members without aliases to minimize interference.

Motivation example:
```
> cat foo.h
inline int foo(int a) { return a + 1; }
int bar(int a);
> cat bar.cpp
#include "foo.h"
int bar(int a) { return foo(a + 1); }
> cat main.cpp
#include "foo.h"
int main(int argc, const char* argv[]) { return bar(argc) + foo(argc); }
> clang -c -flto -fprofile-instr-generate main.cpp -o main.o
> clang -c -flto -fprofile-instr-generate bar.cpp -o bar.o
> clang -fuse-ld=lld -fprofile-instr-generate main.o bar.o -o test1
> ./test1
> llvm-profdata merge --text default.profraw -o -
_Z3fooi
# Counter Values:
2
> llvm-link main.o bar.o -o combined.o
> clang -fuse-ld=lld -fprofile-instr-generate combined.o -o test2
> ./test2
> llvm-profdata merge --text default.profraw -o -
_Z3fooi
# Counter Values:
4
```

---
Full diff: https://github.com/llvm/llvm-project/pull/69143.diff


2 Files Affected:

- (modified) llvm/lib/Linker/LinkModules.cpp (+21) 
- (modified) llvm/test/Linker/comdat-nonprevailing-decl.ll (+29) 


``````````diff
diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index 2f5fac4951f29e0..4fe1f1a0f51833f 100644
--- a/llvm/lib/Linker/LinkModules.cpp
+++ b/llvm/lib/Linker/LinkModules.cpp
@@ -462,6 +462,7 @@ void ModuleLinker::dropReplacedComdat(
 bool ModuleLinker::run() {
   Module &DstM = Mover.getModule();
   DenseSet<const Comdat *> ReplacedDstComdats;
+  DenseSet<const Comdat *> NonPrevailingComdats;
 
   for (const auto &SMEC : SrcM->getComdatSymbolTable()) {
     const Comdat &C = SMEC.getValue();
@@ -473,6 +474,9 @@ bool ModuleLinker::run() {
       return true;
     ComdatsChosen[&C] = std::make_pair(SK, From);
 
+    if (From == LinkFrom::Dst)
+      NonPrevailingComdats.insert(&C);
+
     if (From != LinkFrom::Src)
       continue;
 
@@ -497,6 +501,23 @@ bool ModuleLinker::run() {
   for (Function &GV : llvm::make_early_inc_range(DstM))
     dropReplacedComdat(GV, ReplacedDstComdats);
 
+  if (!NonPrevailingComdats.empty()) {
+    DenseSet<GlobalObject *> AliasedGlobals;
+    for (auto &GA : SrcM->aliases())
+      if (GlobalObject *GO = GA.getAliaseeObject(); GO && GO->getComdat())
+        AliasedGlobals.insert(GO);
+    for (const Comdat *C : NonPrevailingComdats) {
+      SmallVector<GlobalObject *> ToUpdate;
+      for (GlobalObject *GO : C->getUsers())
+        if (GO->hasPrivateLinkage() && !AliasedGlobals.contains(GO))
+          ToUpdate.push_back(GO);
+      for (GlobalObject *GO : ToUpdate) {
+        GO->setLinkage(GlobalValue::AvailableExternallyLinkage);
+        GO->setComdat(nullptr);
+      }
+    }
+  }
+
   for (GlobalVariable &GV : SrcM->globals())
     if (GV.hasLinkOnceLinkage())
       if (const Comdat *SC = GV.getComdat())
diff --git a/llvm/test/Linker/comdat-nonprevailing-decl.ll b/llvm/test/Linker/comdat-nonprevailing-decl.ll
index d3dfec8fb2bc728..30dd0e0abf8c56c 100644
--- a/llvm/test/Linker/comdat-nonprevailing-decl.ll
+++ b/llvm/test/Linker/comdat-nonprevailing-decl.ll
@@ -1,5 +1,6 @@
 ; RUN: rm -rf %t && split-file %s %t
 ; RUN: llvm-link -S %t/1.ll %t/1-aux.ll -o - | FileCheck %s
+; RUN: llvm-link -S %t/2.ll %t/2-aux.ll -o - | FileCheck %s --check-prefix=CHECK2
 
 ;--- 1.ll
 $c = comdat any
@@ -23,3 +24,31 @@ define ptr @f3() {
   ret ptr @v3
 }
 
+;--- 2.ll
+;; Check that a private global variable from a non-prevailing comdat group is
+;; converted into 'available_externally' and excluded from the comdat group.
+
+; CHECK2: $__profc_foo = comdat any
+; CHECK2: @llvm.compiler.used = appending global [2 x ptr] [ptr @__profd_foo.[[SUFFIX:[0-9]+]], ptr @__profd_foo]
+; CHECK2: @__profd_foo.[[SUFFIX]] = private global ptr @__profc_foo, comdat($__profc_foo)
+; CHECK2: @__profc_foo = linkonce_odr global i64 1, comdat
+; CHECK2: @__profd_foo = available_externally dso_local global ptr @__profc_foo{{$}}
+
+$__profc_foo = comdat any
+ at __profc_foo = linkonce_odr global i64 1, comdat
+ at __profd_foo = private global ptr @__profc_foo, comdat($__profc_foo)
+ at llvm.compiler.used = appending global [1 x ptr] [ ptr @__profd_foo ]
+
+define ptr @bar() {
+  ret ptr @__profc_foo
+}
+
+;--- 2-aux.ll
+$__profc_foo = comdat any
+ at __profc_foo = linkonce_odr global i64 1, comdat
+ at __profd_foo = private global ptr @__profc_foo, comdat($__profc_foo)
+ at llvm.compiler.used = appending global [1 x ptr] [ ptr @__profd_foo ]
+
+define ptr @baz() {
+  ret ptr @__profc_foo
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/69143