[Linker] Replace comdat based bool LinkFromSrc with enum class LinkFrom and improve...

This is different from symbol resolution based LinkFromSrc.  Rename to be
clearer.

In the future we may support a new enum member 'Both' for nodeduplicate. This is
feasible (by renaming to a private linkage GlobalValue), but we need to be
careful not to break InstrProfiling.cpp's expectation of parallel profd/profc.

The challenge is that current LTO symbol resolution only allows to mark one
profc as prevailing: the other profc in another comdat nodeduplicate may be
discarded while its associated profd isn't.

diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index 971d3f0..555aeea 100644 (file)
--- a/llvm/lib/Linker/LinkModules.cpp
+++ b/llvm/lib/Linker/LinkModules.cpp
@@ -24,6 +24,8 @@ using namespace llvm;
 
 namespace {
 
+enum class LinkFrom { Dst, Src };
+
 /// This is an implementation class for the LinkModules function, which is the
 /// entrypoint for this file.
 class ModuleLinker {
@@ -67,11 +69,11 @@ class ModuleLinker {
                                      Comdat::SelectionKind Src,
                                      Comdat::SelectionKind Dst,
                                      Comdat::SelectionKind &Result,
-                                     bool &LinkFromSrc);
-  std::map<const Comdat *, std::pair<Comdat::SelectionKind, bool>>
+                                     LinkFrom &From);
+  DenseMap<const Comdat *, std::pair<Comdat::SelectionKind, LinkFrom>>
       ComdatsChosen;
   bool getComdatResult(const Comdat *SrcC, Comdat::SelectionKind &SK,
-                       bool &LinkFromSrc);
+                       LinkFrom &From);
   // Keep track of the lazy linked global members of each comdat in source.
   DenseMap<const Comdat *, std::vector<GlobalValue *>> LazyComdatMembers;
 
@@ -114,7 +116,7 @@ public:
 
   bool run();
 };
-}
+} // namespace
 
 static GlobalValue::VisibilityTypes
 getMinVisibility(GlobalValue::VisibilityTypes A,
@@ -151,7 +153,7 @@ bool ModuleLinker::computeResultingSelectionKind(StringRef ComdatName,
                                                  Comdat::SelectionKind Src,
                                                  Comdat::SelectionKind Dst,
                                                  Comdat::SelectionKind &Result,
-                                                 bool &LinkFromSrc) {
+                                                 LinkFrom &From) {
   Module &DstM = Mover.getModule();
   // The ability to mix Comdat::SelectionKind::Any with
   // Comdat::SelectionKind::Largest is a behavior that comes from COFF.
@@ -175,7 +177,7 @@ bool ModuleLinker::computeResultingSelectionKind(StringRef ComdatName,
   switch (Result) {
   case Comdat::SelectionKind::Any:
     // Go with Dst.
-    LinkFromSrc = false;
+    From = LinkFrom::Dst;
     break;
   case Comdat::SelectionKind::NoDeduplicate:
     return emitError("Linking COMDATs named '" + ComdatName +
@@ -197,14 +199,14 @@ bool ModuleLinker::computeResultingSelectionKind(StringRef ComdatName,
       if (SrcGV->getInitializer() != DstGV->getInitializer())
         return emitError("Linking COMDATs named '" + ComdatName +
                          "': ExactMatch violated!");
-      LinkFromSrc = false;
+      From = LinkFrom::Dst;
     } else if (Result == Comdat::SelectionKind::Largest) {
-      LinkFromSrc = SrcSize > DstSize;
+      From = SrcSize > DstSize ? LinkFrom::Src : LinkFrom::Dst;
     } else if (Result == Comdat::SelectionKind::SameSize) {
       if (SrcSize != DstSize)
         return emitError("Linking COMDATs named '" + ComdatName +
                          "': SameSize violated!");
-      LinkFromSrc = false;
+      From = LinkFrom::Dst;
     } else {
       llvm_unreachable("unknown selection kind");
     }
@@ -217,7 +219,7 @@ bool ModuleLinker::computeResultingSelectionKind(StringRef ComdatName,
 
 bool ModuleLinker::getComdatResult(const Comdat *SrcC,
                                    Comdat::SelectionKind &Result,
-                                   bool &LinkFromSrc) {
+                                   LinkFrom &From) {
   Module &DstM = Mover.getModule();
   Comdat::SelectionKind SSK = SrcC->getSelectionKind();
   StringRef ComdatName = SrcC->getName();
@@ -226,15 +228,14 @@ bool ModuleLinker::getComdatResult(const Comdat *SrcC,
 
   if (DstCI == ComdatSymTab.end()) {
     // Use the comdat if it is only available in one of the modules.
-    LinkFromSrc = true;
+    From = LinkFrom::Src;
     Result = SSK;
     return false;
   }
 
   const Comdat *DstC = &DstCI->second;
   Comdat::SelectionKind DSK = DstC->getSelectionKind();
-  return computeResultingSelectionKind(ComdatName, SSK, DSK, Result,
-                                       LinkFromSrc);
+  return computeResultingSelectionKind(ComdatName, SSK, DSK, Result, From);
 }
 
 bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc,
@@ -377,11 +378,10 @@ bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
   if (GV.isDeclaration())
     return false;
 
+  LinkFrom ComdatFrom = LinkFrom::Dst;
   if (const Comdat *SC = GV.getComdat()) {
-    bool LinkFromSrc;
-    Comdat::SelectionKind SK;
-    std::tie(SK, LinkFromSrc) = ComdatsChosen[SC];
-    if (!LinkFromSrc)
+    std::tie(std::ignore, ComdatFrom) = ComdatsChosen[SC];
+    if (ComdatFrom == LinkFrom::Dst)
       return false;
   }
 
@@ -462,12 +462,12 @@ bool ModuleLinker::run() {
     if (ComdatsChosen.count(&C))
       continue;
     Comdat::SelectionKind SK;
-    bool LinkFromSrc;
-    if (getComdatResult(&C, SK, LinkFromSrc))
+    LinkFrom From;
+    if (getComdatResult(&C, SK, From))
       return true;
-    ComdatsChosen[&C] = std::make_pair(SK, LinkFromSrc);
+    ComdatsChosen[&C] = std::make_pair(SK, From);
 
-    if (!LinkFromSrc)
+    if (From != LinkFrom::Src)
       continue;
 
     Module::ComdatSymTabType &ComdatSymTab = DstM.getComdatSymbolTable();

diff --git a/llvm/test/Linker/comdat-nodeduplicate.ll b/llvm/test/Linker/comdat-nodeduplicate.ll
index ae59909..2724311 100644 (file)
--- a/llvm/test/Linker/comdat-nodeduplicate.ll
+++ b/llvm/test/Linker/comdat-nodeduplicate.ll
@@ -1,11 +1,39 @@
 ; RUN: rm -rf %t && split-file %s %t
-; RUN: not llvm-link %t/1.ll %t/1-aux.ll -S -o - 2>&1 | FileCheck %s
+; RUN: not llvm-link -S %t/1.ll %t/1-aux.ll 2>&1 | FileCheck %s
+
+; CHECK: error: Linking COMDATs named 'foo': nodeduplicate has been violated!
+
+; RUN: not llvm-link -S %t/2.ll %t/2-aux.ll 2>&1 | FileCheck %s --check-prefix=CHECK2
+; RUN: not llvm-link -S %t/2-aux.ll %t/2.ll 2>&1 | FileCheck %s --check-prefix=CHECK2
+
+; CHECK2: error: Linking COMDATs named 'foo'
+
+; RUN: not llvm-link -S %t/non-var.ll %t/non-var.ll 2>&1 | FileCheck %s --check-prefix=NONVAR
+
+; NONVAR: error: Linking COMDATs named 'foo': nodeduplicate has been violated!
 
 ;--- 1.ll
 $foo = comdat nodeduplicate
 @foo = global i64 43, comdat($foo)
-; CHECK: Linking COMDATs named 'foo': nodeduplicate has been violated!
 
 ;--- 1-aux.ll
 $foo = comdat nodeduplicate
 @foo = global i64 43, comdat($foo)
+
+;--- 2.ll
+$foo = comdat nodeduplicate
+@foo = global i64 2, section "data", comdat($foo), align 8
+@bar = weak global i64 0, section "cnts", comdat($foo)
+@qux = weak_odr global i64 4, comdat($foo)
+
+;--- 2-aux.ll
+$foo = comdat nodeduplicate
+@foo = weak global i64 0, section "data", comdat($foo)
+@bar = dso_local global i64 3, section "cnts", comdat($foo), align 16
+@fred = linkonce global i64 5, comdat($foo)
+
+;--- non-var.ll
+$foo = comdat nodeduplicate
+define weak void @foo() comdat {
+  ret void
+}