From 510e106fa8635e7f9c51c896180b971de6309b2f Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sat, 28 Aug 2021 13:56:31 -0700 Subject: [PATCH] [Linker] Replace comdat based bool LinkFromSrc with enum class LinkFrom and improve nodeduplicate tests. NFC 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. --- llvm/lib/Linker/LinkModules.cpp | 42 ++++++++++++++++---------------- llvm/test/Linker/comdat-nodeduplicate.ll | 32 ++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp index 971d3f0..555aeea 100644 --- 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> + LinkFrom &From); + DenseMap> 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> 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 --- 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 +} -- 2.7.4