[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.
This commit is contained in:
Fangrui Song 2021-08-28 13:56:31 -07:00
parent ae5e5f2011
commit 510e106fa8
2 changed files with 51 additions and 23 deletions

View File

@ -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();

View File

@ -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
}