[llvm] 510e106 - [Linker] Replace comdat based bool LinkFromSrc with enum class LinkFrom and improve nodeduplicate tests. NFC
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 28 13:56:38 PDT 2021
Author: Fangrui Song
Date: 2021-08-28T13:56:32-07:00
New Revision: 510e106fa8635e7f9c51c896180b971de6309b2f
URL: https://github.com/llvm/llvm-project/commit/510e106fa8635e7f9c51c896180b971de6309b2f
DIFF: https://github.com/llvm/llvm-project/commit/510e106fa8635e7f9c51c896180b971de6309b2f.diff
LOG: [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.
Added:
Modified:
llvm/lib/Linker/LinkModules.cpp
llvm/test/Linker/comdat-nodeduplicate.ll
Removed:
################################################################################
diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp
index 971d3f0b21683..555aeea6e9d59 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<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 @@ class ModuleLinker {
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 ae59909e92504..2724311a93021 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
+ at foo = global i64 2, section "data", comdat($foo), align 8
+ at bar = weak global i64 0, section "cnts", comdat($foo)
+ at qux = weak_odr global i64 4, comdat($foo)
+
+;--- 2-aux.ll
+$foo = comdat nodeduplicate
+ at foo = weak global i64 0, section "data", comdat($foo)
+ at bar = dso_local global i64 3, section "cnts", comdat($foo), align 16
+ at fred = linkonce global i64 5, comdat($foo)
+
+;--- non-var.ll
+$foo = comdat nodeduplicate
+define weak void @foo() comdat {
+ ret void
+}
More information about the llvm-commits
mailing list