Skip to content

Commit 3044d64

Browse files
Jezurkofrabert
Jezurko
authored and
frabert
committed
[MLIR][mlir-link] Add COMDAT resolution to LLVM dialect linker.
1 parent 3437c68 commit 3044d64

File tree

14 files changed

+374
-124
lines changed

14 files changed

+374
-124
lines changed

‎clang/include/clang/CIR/Interfaces/CIRLinkerInterface.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,13 @@ class CIRSymbolLinkerInterface
4545

4646
static bool isComdat(Operation *op);
4747

48-
static std::optional<mlir::link::ComdatSelector>
49-
getComdatSelector(Operation *op);
48+
static bool hasComdat(Operation *op);
49+
50+
static const link::Comdat *getComdatResolution(Operation *op);
51+
52+
static bool selectedByComdat(Operation *op);
53+
54+
static void updateNoDeduplicate(Operation *op);
5055

5156
static Visibility getVisibility(Operation *op);
5257

‎clang/lib/CIR/Dialect/IR/CIRLinkerInterface.cpp‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,23 @@ bool CIRSymbolLinkerInterface::isComdat(Operation *op) {
9999
return false;
100100
}
101101

102-
std::optional<link::ComdatSelector>
103-
CIRSymbolLinkerInterface::getComdatSelector(Operation *op) {
104-
// TODO(frabert): Extracting comdat info from CIR is not implemented yet
105-
return std::nullopt;
102+
bool CIRSymbolLinkerInterface::hasComdat(Operation *op) {
103+
// TODO: Extracting comdat info from CIR is not implemented yet
104+
return false;
105+
}
106+
107+
const link::Comdat *
108+
CIRSymbolLinkerInterface::getComdatResolution(Operation *op) {
109+
return nullptr;
106110
}
107111

112+
bool CIRSymbolLinkerInterface::selectedByComdat(Operation *op) {
113+
// TODO: Extracting comdat info from CIR is not implemented yet
114+
llvm_unreachable("comdat resolution not implemented for CIR");
115+
}
116+
117+
void CIRSymbolLinkerInterface::updateNoDeduplicate(Operation *op) {}
118+
108119
Visibility CIRSymbolLinkerInterface::getVisibility(Operation *op) {
109120
if (auto gv = dyn_cast<GlobalOp>(op))
110121
return toLLVMVisibility(gv.getGlobalVisibility());

‎mlir/include/mlir/Dialect/LLVMIR/LLVMLinkerInterface.h‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ class LLVMSymbolLinkerInterface
1616
static Visibility getVisibility(Operation *op);
1717
static void setVisibility(Operation *op, Visibility visibility);
1818
static bool isComdat(Operation *op);
19-
static std::optional<link::ComdatSelector> getComdatSelector(Operation *op);
19+
static bool hasComdat(Operation *op);
20+
static SymbolRefAttr getComdatSymbol(Operation *op);
21+
static LLVM::comdat::Comdat getComdatSelectionKind(Operation *op);
2022
static bool isDeclaration(Operation *op);
2123
static unsigned getBitWidth(Operation *op);
2224
static UnnamedAddr getUnnamedAddr(Operation *op);
@@ -34,7 +36,19 @@ class LLVMSymbolLinkerInterface
3436
dependencies(Operation *op, SymbolTableCollection &collection) const override;
3537
LogicalResult initialize(ModuleOp src) override;
3638
LogicalResult finalize(ModuleOp dst) const override;
39+
LogicalResult moduleOpSummary(ModuleOp src,
40+
SymbolTableCollection &collection) override;
3741
Operation *appendGlobals(llvm::StringRef glob, link::LinkState &state);
42+
Operation *appendComdatOps(ArrayRef<Operation *> globs, LLVM::ComdatOp comdat,
43+
link::LinkState &state);
44+
link::ComdatResolution
45+
computeComdatResolution(Operation *, SymbolTableCollection &, link::Comdat *);
46+
LogicalResult resolveComdats(ModuleOp srcMod,
47+
SymbolTableCollection &collection);
48+
const link::Comdat *getComdatResolution(Operation *op) const;
49+
bool selectedByComdat(Operation *op) const;
50+
void dropReplacedComdat(Operation *op) const;
51+
static void updateNoDeduplicate(Operation *op);
3852

3953
template <typename structor_t>
4054
Operation *appendGlobalStructors(link::LinkState &state) {
@@ -79,22 +93,27 @@ class LLVMSymbolLinkerInterface
7993
ArrayRef<Attribute> priorities = structor.getPriorities().getValue();
8094
ArrayRef<Attribute> data = structor.getData().getValue();
8195

82-
for (auto [idx, dataAttr] : llvm::enumerate(data)) {
96+
for (auto [idx, structor] : llvm::enumerate(structorList)) {
97+
auto structorSymbol = cast<FlatSymbolRefAttr>(structor);
98+
// Skip constructors not included based on COMDAT
99+
if (!summary.contains(structorSymbol.getValue()))
100+
continue;
101+
102+
auto dataAttr = data[idx];
83103
// data value is either #llvm.zero or symbol ref
84104
// if it is zero, we always have to include the value
85105
// if it is a symbol ref, we have to check if the symbol
86106
// from the same module is being used
87-
//
88107
if (auto globalSymbol = dyn_cast<FlatSymbolRefAttr>(dataAttr)) {
89-
auto globalOp = summary.lookup(globalSymbol.getValue());
108+
Operation *globalOp = summary.lookup(globalSymbol.getValue());
90109
assert(globalOp && "structor referenced global not in summary?");
91110
// globals are definde at module level
92111
if (globalOp->getParentOp() != op->getParentOp())
93112
continue;
94113
}
95114

96115
newData.push_back(dataAttr);
97-
newStructorList.push_back(structorList[idx]);
116+
newStructorList.push_back(structor);
98117
newPriorities.push_back(priorities[idx]);
99118
}
100119
}
@@ -125,6 +144,7 @@ class LLVMSymbolLinkerInterface
125144
private:
126145
DataLayoutSpecInterface dtla = {};
127146
TargetSystemSpecInterface targetSys = {};
147+
llvm::StringMap<link::Comdat> comdatResolution;
128148
};
129149

130150
} // namespace LLVM

‎mlir/include/mlir/Linker/LLVMLinkerMixin.h‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,17 @@ static UnnamedAddr getMinUnnamedAddr(UnnamedAddr lhs, UnnamedAddr rhs) {
139139

140140
using ComdatKind = LLVM::comdat::Comdat;
141141

142-
struct ComdatSelector {
143-
StringRef name;
142+
struct Comdat {
144143
ComdatKind kind;
144+
Operation *selectorOp;
145+
llvm::SmallPtrSet<Operation *, 2> users;
146+
};
147+
148+
enum class ComdatResolution {
149+
LinkFromSrc,
150+
LinkFromDst,
151+
LinkFromBoth,
152+
Failure,
145153
};
146154

147155
//===----------------------------------------------------------------------===//
@@ -176,6 +184,20 @@ class LLVMLinkerMixin {
176184
if (derived.isComdat(pair.src))
177185
return true;
178186

187+
// Thrown away symbol can affect the visibility
188+
if (pair.dst) {
189+
Visibility srcVisibility = derived.getVisibility(pair.src);
190+
Visibility dstVisibility = derived.getVisibility(pair.dst);
191+
Visibility visibility = getMinVisibility(srcVisibility, dstVisibility);
192+
193+
derived.setVisibility(pair.src, visibility);
194+
derived.setVisibility(pair.dst, visibility);
195+
}
196+
if (derived.hasComdat(pair.src)) {
197+
// operations with COMDAT are selected as a group
198+
return derived.selectedByComdat(pair.src);
199+
}
200+
179201
Linkage srcLinkage = derived.getLinkage(pair.src);
180202

181203
// Always import variables with appending linkage.
@@ -222,6 +244,10 @@ class LLVMLinkerMixin {
222244
return pair.src->emitError(error) << " dst: " << pair.dst->getLoc();
223245
};
224246

247+
if (derived.isComdat(pair.src) != derived.isComdat(pair.dst)) {
248+
return linkError("Linking ComdatOp with non-comdat op");
249+
}
250+
225251
Linkage srcLinkage = derived.getLinkage(pair.src);
226252
Linkage dstLinkage = derived.getLinkage(pair.dst);
227253

@@ -344,6 +370,11 @@ class LLVMLinkerMixin {
344370
if (isWeakForLinker(srcLinkage)) {
345371
assert(!isExternalWeakLinkage(dstLinkage));
346372
assert(!isAvailableExternallyLinkage(dstLinkage));
373+
const Comdat *comdat = derived.getComdatResolution(pair.src);
374+
if (comdat && comdat->kind == ComdatKind::NoDeduplicate) {
375+
derived.updateNoDeduplicate(pair.src);
376+
return ConflictResolution::LinkFromBothAndRenameSrc;
377+
}
347378
if (isLinkOnceLinkage(dstLinkage) && isWeakLinkage(srcLinkage)) {
348379
return ConflictResolution::LinkFromSrc;
349380
}
@@ -353,38 +384,12 @@ class LLVMLinkerMixin {
353384

354385
if (isWeakForLinker(dstLinkage)) {
355386
assert(isExternalLinkage(srcLinkage));
356-
return ConflictResolution::LinkFromSrc;
357-
}
358-
359-
std::optional<ComdatSelector> srcComdatSel =
360-
derived.getComdatSelector(pair.src);
361-
std::optional<ComdatSelector> dstComdatSel =
362-
derived.getComdatSelector(pair.dst);
363-
if (srcComdatSel.has_value() && dstComdatSel.has_value()) {
364-
auto srcComdatName = srcComdatSel->name;
365-
auto dstComdatName = dstComdatSel->name;
366-
auto srcComdat = srcComdatSel->kind;
367-
auto dstComdat = dstComdatSel->kind;
368-
if (srcComdatName != dstComdatName) {
369-
llvm_unreachable("Comdat selector names don't match");
370-
}
371-
if (srcComdat != dstComdat) {
372-
llvm_unreachable("Comdat selector kinds don't match");
373-
}
374-
375-
if (srcComdat == mlir::LLVM::comdat::Comdat::Any) {
376-
return ConflictResolution::LinkFromDst;
377-
}
378-
if (srcComdat == mlir::LLVM::comdat::Comdat::NoDeduplicate) {
379-
return ConflictResolution::Failure;
387+
const Comdat *comdat = derived.getComdatResolution(pair.dst);
388+
if (comdat && comdat->kind == ComdatKind::NoDeduplicate) {
389+
derived.updateNoDeduplicate(pair.dst);
390+
return ConflictResolution::LinkFromBothAndRenameDst;
380391
}
381-
if (srcComdat == mlir::LLVM::comdat::Comdat::ExactMatch) {
382-
return ConflictResolution::LinkFromDst;
383-
}
384-
if (srcComdat == mlir::LLVM::comdat::Comdat::SameSize) {
385-
return ConflictResolution::LinkFromDst;
386-
}
387-
llvm_unreachable("unimplemented comdat kind");
392+
return ConflictResolution::LinkFromSrc;
388393
}
389394

390395
// If we reach here, we have two external definitions that can't be resolved

‎mlir/include/mlir/Linker/LinkerInterface.h‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ class LinkState {
6363
return builder.create<Op>(location, std::forward<Args>(args)...);
6464
}
6565

66+
OpBuilder &getBuilder() { return builder; };
67+
6668
private:
6769
// Private constructor used by nest()
6870
LinkState(ModuleOp dst, std::shared_ptr<IRMapping> mapping,
@@ -152,7 +154,8 @@ class SymbolLinkerInterface : public LinkerInterface<SymbolLinkerInterface> {
152154

153155
/// Perform tasks that need to be computed on whole-module basis before actual summary.
154156
/// E.g. Pre-compute COMDAT resolution before actually linking the modules.
155-
virtual LogicalResult moduleOpSummary(ModuleOp module) {
157+
virtual LogicalResult moduleOpSummary(ModuleOp module,
158+
SymbolTableCollection &collection) {
156159
return success();
157160
}
158161

@@ -293,10 +296,11 @@ class SymbolLinkerInterfaces {
293296
return Conflict::noConflict(src);
294297
}
295298

296-
LogicalResult moduleOpSummary(ModuleOp src) {
299+
LogicalResult moduleOpSummary(ModuleOp src,
300+
SymbolTableCollection &collection) {
297301
return failableParallelForEach(src.getContext(), interfaces,
298-
[src](SymbolLinkerInterface *linker) {
299-
return linker->moduleOpSummary(src);
302+
[&](SymbolLinkerInterface *linker) {
303+
return linker->moduleOpSummary(src, collection);
300304
});
301305
}
302306

0 commit comments

Comments
 (0)