Skip to content

Commit 0074cea

Browse files
lhameslhames
committed
[ORC] Get rid of ObjectLinkingLayer::Plugin::getSyntheticSymbolDependencies.
Instead, when a MaterializationResponsibility contains an initializer symbol, the Platform classes (MachO, COFF, ELFNix) will now add a defined symbol with the same name to an arbitary block within the initializer sections, and then add keep-alive edges from that symbol to all other init section blocks. ObjectLinkingLayer is updated to automatically discard symbols where the corresponding MaterializationResponsibility entry has the MaterializationSideEffecstsOnly flag. This change simplifies both the ObjectLinkingLayer::Plugin interface and the dependence tracking algorithm, which no longer needs a special case for "synthetic" (MaterializationSideEffectsOnly) symbols.
1 parent e4e3ff5 commit 0074cea

File tree

8 files changed

+92
-150
lines changed

8 files changed

+92
-150
lines changed

‎llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,6 @@ class COFFPlatform : public Platform {
9999
jitlink::LinkGraph &G,
100100
jitlink::PassConfiguration &Config) override;
101101

102-
SyntheticSymbolDependenciesMap
103-
getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
104-
105102
// FIXME: We should be tentatively tracking scraped sections and discarding
106103
// if the MR fails.
107104
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -116,9 +113,6 @@ class COFFPlatform : public Platform {
116113
ResourceKey SrcKey) override {}
117114

118115
private:
119-
using InitSymbolDepMap =
120-
DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
121-
122116
Error associateJITDylibHeaderSymbol(jitlink::LinkGraph &G,
123117
MaterializationResponsibility &MR,
124118
bool Bootstrap);
@@ -131,7 +125,6 @@ class COFFPlatform : public Platform {
131125

132126
std::mutex PluginMutex;
133127
COFFPlatform &CP;
134-
InitSymbolDepMap InitSymbolDeps;
135128
};
136129

137130
struct JDBootstrapState {

‎llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,6 @@ class ELFNixPlatform : public Platform {
137137
jitlink::LinkGraph &G,
138138
jitlink::PassConfiguration &Config) override;
139139

140-
SyntheticSymbolDependenciesMap
141-
getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
142-
143140
// FIXME: We should be tentatively tracking scraped sections and discarding
144141
// if the MR fails.
145142
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -154,9 +151,6 @@ class ELFNixPlatform : public Platform {
154151
ResourceKey SrcKey) override {}
155152

156153
private:
157-
using InitSymbolDepMap =
158-
DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
159-
160154
void addInitializerSupportPasses(MaterializationResponsibility &MR,
161155
jitlink::PassConfiguration &Config);
162156

@@ -175,7 +169,6 @@ class ELFNixPlatform : public Platform {
175169

176170
std::mutex PluginMutex;
177171
ELFNixPlatform &MP;
178-
InitSymbolDepMap InitSymbolDeps;
179172
};
180173

181174
using SendInitializerSequenceFn =

‎llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,6 @@ class MachOPlatform : public Platform {
202202
jitlink::LinkGraph &G,
203203
jitlink::PassConfiguration &Config) override;
204204

205-
SyntheticSymbolDependenciesMap
206-
getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
207-
208205
// FIXME: We should be tentatively tracking scraped sections and discarding
209206
// if the MR fails.
210207
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -219,9 +216,6 @@ class MachOPlatform : public Platform {
219216
ResourceKey SrcKey) override {}
220217

221218
private:
222-
using InitSymbolDepMap =
223-
DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
224-
225219
struct UnwindSections {
226220
SmallVector<ExecutorAddrRange> CodeRanges;
227221
ExecutorAddrRange DwarfSection;
@@ -282,7 +276,6 @@ class MachOPlatform : public Platform {
282276
// JITDylibs are removed.
283277
DenseMap<JITDylib *, ObjCImageInfo> ObjCImageInfos;
284278
DenseMap<JITDylib *, ExecutorAddr> HeaderAddrs;
285-
InitSymbolDepMap InitSymbolDeps;
286279
};
287280

288281
using GetJITDylibHeaderSendResultFn =

‎llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,6 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
5858
/// configured.
5959
class Plugin {
6060
public:
61-
using JITLinkSymbolSet = DenseSet<jitlink::Symbol *>;
62-
using SyntheticSymbolDependenciesMap =
63-
DenseMap<SymbolStringPtr, JITLinkSymbolSet>;
64-
6561
virtual ~Plugin();
6662
virtual void modifyPassConfig(MaterializationResponsibility &MR,
6763
jitlink::LinkGraph &G,
@@ -82,15 +78,6 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
8278
virtual Error notifyRemovingResources(JITDylib &JD, ResourceKey K) = 0;
8379
virtual void notifyTransferringResources(JITDylib &JD, ResourceKey DstKey,
8480
ResourceKey SrcKey) = 0;
85-
86-
/// Return any dependencies that synthetic symbols (e.g. init symbols)
87-
/// have on symbols in the LinkGraph.
88-
/// This is used by the ObjectLinkingLayer to update the dependencies for
89-
/// the synthetic symbols.
90-
virtual SyntheticSymbolDependenciesMap
91-
getSyntheticSymbolDependencies(MaterializationResponsibility &MR) {
92-
return SyntheticSymbolDependenciesMap();
93-
}
9481
};
9582

9683
using ReturnObjectBufferFunction =

‎llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -786,20 +786,6 @@ void COFFPlatform::COFFPlatformPlugin::modifyPassConfig(
786786
});
787787
}
788788

789-
ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
790-
COFFPlatform::COFFPlatformPlugin::getSyntheticSymbolDependencies(
791-
MaterializationResponsibility &MR) {
792-
std::lock_guard<std::mutex> Lock(PluginMutex);
793-
auto I = InitSymbolDeps.find(&MR);
794-
if (I != InitSymbolDeps.end()) {
795-
SyntheticSymbolDependenciesMap Result;
796-
Result[MR.getInitializerSymbol()] = std::move(I->second);
797-
InitSymbolDeps.erase(&MR);
798-
return Result;
799-
}
800-
return SyntheticSymbolDependenciesMap();
801-
}
802-
803789
Error COFFPlatform::COFFPlatformPlugin::associateJITDylibHeaderSymbol(
804790
jitlink::LinkGraph &G, MaterializationResponsibility &MR,
805791
bool IsBootstraping) {
@@ -859,16 +845,37 @@ Error COFFPlatform::COFFPlatformPlugin::registerObjectPlatformSections(
859845

860846
Error COFFPlatform::COFFPlatformPlugin::preserveInitializerSections(
861847
jitlink::LinkGraph &G, MaterializationResponsibility &MR) {
862-
JITLinkSymbolSet InitSectionSymbols;
863-
for (auto &Sec : G.sections())
864-
if (isCOFFInitializerSection(Sec.getName()))
865-
for (auto *B : Sec.blocks())
866-
if (!B->edges_empty())
867-
InitSectionSymbols.insert(
868-
&G.addAnonymousSymbol(*B, 0, 0, false, true));
869-
870-
std::lock_guard<std::mutex> Lock(PluginMutex);
871-
InitSymbolDeps[&MR] = InitSectionSymbols;
848+
849+
if (const auto &InitSymName = MR.getInitializerSymbol()) {
850+
851+
jitlink::Symbol *InitSym = nullptr;
852+
853+
for (auto &InitSection : G.sections()) {
854+
// Skip non-init sections.
855+
if (!isCOFFInitializerSection(InitSection.getName()) ||
856+
InitSection.empty())
857+
continue;
858+
859+
// Create the init symbol if it has not been created already and attach it
860+
// to the first block.
861+
if (!InitSym) {
862+
auto &B = **InitSection.blocks().begin();
863+
InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
864+
jitlink::Linkage::Strong,
865+
jitlink::Scope::Default, false, true);
866+
}
867+
868+
// Add keep-alive edges to anonymous symbols in all other init blocks.
869+
for (auto *B : InitSection.blocks()) {
870+
if (B == &InitSym->getBlock())
871+
continue;
872+
873+
auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
874+
InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
875+
}
876+
}
877+
}
878+
872879
return Error::success();
873880
}
874881

‎llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -616,20 +616,6 @@ void ELFNixPlatform::ELFNixPlatformPlugin::modifyPassConfig(
616616
addEHAndTLVSupportPasses(MR, Config);
617617
}
618618

619-
ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
620-
ELFNixPlatform::ELFNixPlatformPlugin::getSyntheticSymbolDependencies(
621-
MaterializationResponsibility &MR) {
622-
std::lock_guard<std::mutex> Lock(PluginMutex);
623-
auto I = InitSymbolDeps.find(&MR);
624-
if (I != InitSymbolDeps.end()) {
625-
SyntheticSymbolDependenciesMap Result;
626-
Result[MR.getInitializerSymbol()] = std::move(I->second);
627-
InitSymbolDeps.erase(&MR);
628-
return Result;
629-
}
630-
return SyntheticSymbolDependenciesMap();
631-
}
632-
633619
void ELFNixPlatform::ELFNixPlatformPlugin::addInitializerSupportPasses(
634620
MaterializationResponsibility &MR, jitlink::PassConfiguration &Config) {
635621

@@ -737,34 +723,34 @@ void ELFNixPlatform::ELFNixPlatformPlugin::addEHAndTLVSupportPasses(
737723
Error ELFNixPlatform::ELFNixPlatformPlugin::preserveInitSections(
738724
jitlink::LinkGraph &G, MaterializationResponsibility &MR) {
739725

740-
JITLinkSymbolSet InitSectionSymbols;
741-
for (auto &InitSection : G.sections()) {
742-
// Skip non-init sections.
743-
if (!isELFInitializerSection(InitSection.getName()))
744-
continue;
745-
746-
// Make a pass over live symbols in the section: those blocks are already
747-
// preserved.
748-
DenseSet<jitlink::Block *> AlreadyLiveBlocks;
749-
for (auto &Sym : InitSection.symbols()) {
750-
auto &B = Sym->getBlock();
751-
if (Sym->isLive() && Sym->getOffset() == 0 &&
752-
Sym->getSize() == B.getSize() && !AlreadyLiveBlocks.count(&B)) {
753-
InitSectionSymbols.insert(Sym);
754-
AlreadyLiveBlocks.insert(&B);
726+
if (const auto &InitSymName = MR.getInitializerSymbol()) {
727+
728+
jitlink::Symbol *InitSym = nullptr;
729+
730+
for (auto &InitSection : G.sections()) {
731+
// Skip non-init sections.
732+
if (!isELFInitializerSection(InitSection.getName()) ||
733+
InitSection.empty())
734+
continue;
735+
736+
// Create the init symbol if it has not been created already and attach it
737+
// to the first block.
738+
if (!InitSym) {
739+
auto &B = **InitSection.blocks().begin();
740+
InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
741+
jitlink::Linkage::Strong,
742+
jitlink::Scope::Default, false, true);
755743
}
756-
}
757744

758-
// Add anonymous symbols to preserve any not-already-preserved blocks.
759-
for (auto *B : InitSection.blocks())
760-
if (!AlreadyLiveBlocks.count(B))
761-
InitSectionSymbols.insert(
762-
&G.addAnonymousSymbol(*B, 0, B->getSize(), false, true));
763-
}
745+
// Add keep-alive edges to anonymous symbols in all other init blocks.
746+
for (auto *B : InitSection.blocks()) {
747+
if (B == &InitSym->getBlock())
748+
continue;
764749

765-
if (!InitSectionSymbols.empty()) {
766-
std::lock_guard<std::mutex> Lock(PluginMutex);
767-
InitSymbolDeps[&MR] = std::move(InitSectionSymbols);
750+
auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
751+
InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
752+
}
753+
}
768754
}
769755

770756
return Error::success();

‎llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp

Lines changed: 27 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -861,20 +861,6 @@ void MachOPlatform::MachOPlatformPlugin::modifyPassConfig(
861861
[this](LinkGraph &G) { return bootstrapPipelineEnd(G); });
862862
}
863863

864-
ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
865-
MachOPlatform::MachOPlatformPlugin::getSyntheticSymbolDependencies(
866-
MaterializationResponsibility &MR) {
867-
std::lock_guard<std::mutex> Lock(PluginMutex);
868-
auto I = InitSymbolDeps.find(&MR);
869-
if (I != InitSymbolDeps.end()) {
870-
SyntheticSymbolDependenciesMap Result;
871-
Result[MR.getInitializerSymbol()] = std::move(I->second);
872-
InitSymbolDeps.erase(&MR);
873-
return Result;
874-
}
875-
return SyntheticSymbolDependenciesMap();
876-
}
877-
878864
Error MachOPlatform::MachOPlatformPlugin::bootstrapPipelineStart(
879865
jitlink::LinkGraph &G) {
880866
// Increment the active graphs count in BootstrapInfo.
@@ -998,40 +984,38 @@ Error MachOPlatform::MachOPlatformPlugin::preserveImportantSections(
998984
// Init sections are important: We need to preserve them and so that their
999985
// addresses can be captured and reported to the ORC runtime in
1000986
// registerObjectPlatformSections.
1001-
JITLinkSymbolSet InitSectionSymbols;
1002-
for (auto &InitSectionName : MachOInitSectionNames) {
1003-
// Skip ObjCImageInfo -- this shouldn't have any dependencies, and we may
1004-
// remove it later.
1005-
if (InitSectionName == MachOObjCImageInfoSectionName)
1006-
continue;
987+
if (const auto &InitSymName = MR.getInitializerSymbol()) {
1007988

1008-
// Skip non-init sections.
1009-
auto *InitSection = G.findSectionByName(InitSectionName);
1010-
if (!InitSection)
1011-
continue;
989+
jitlink::Symbol *InitSym = nullptr;
990+
for (auto &InitSectionName : MachOInitSectionNames) {
991+
// Skip ObjCImageInfo -- this shouldn't have any dependencies, and we may
992+
// remove it later.
993+
if (InitSectionName == MachOObjCImageInfoSectionName)
994+
continue;
1012995

1013-
// Make a pass over live symbols in the section: those blocks are already
1014-
// preserved.
1015-
DenseSet<jitlink::Block *> AlreadyLiveBlocks;
1016-
for (auto &Sym : InitSection->symbols()) {
1017-
auto &B = Sym->getBlock();
1018-
if (Sym->isLive() && Sym->getOffset() == 0 &&
1019-
Sym->getSize() == B.getSize() && !AlreadyLiveBlocks.count(&B)) {
1020-
InitSectionSymbols.insert(Sym);
1021-
AlreadyLiveBlocks.insert(&B);
996+
// Skip non-init sections.
997+
auto *InitSection = G.findSectionByName(InitSectionName);
998+
if (!InitSection || InitSection->empty())
999+
continue;
1000+
1001+
// Create the init symbol if it has not been created already and attach it
1002+
// to the first block.
1003+
if (!InitSym) {
1004+
auto &B = **InitSection->blocks().begin();
1005+
InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
1006+
jitlink::Linkage::Strong,
1007+
jitlink::Scope::Default, false, true);
10221008
}
1023-
}
10241009

1025-
// Add anonymous symbols to preserve any not-already-preserved blocks.
1026-
for (auto *B : InitSection->blocks())
1027-
if (!AlreadyLiveBlocks.count(B))
1028-
InitSectionSymbols.insert(
1029-
&G.addAnonymousSymbol(*B, 0, B->getSize(), false, true));
1030-
}
1010+
// Add keep-alive edges to anonymous symbols in all other init blocks.
1011+
for (auto *B : InitSection->blocks()) {
1012+
if (B == &InitSym->getBlock())
1013+
continue;
10311014

1032-
if (!InitSectionSymbols.empty()) {
1033-
std::lock_guard<std::mutex> Lock(PluginMutex);
1034-
InitSymbolDeps[&MR] = std::move(InitSectionSymbols);
1015+
auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
1016+
InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
1017+
}
1018+
}
10351019
}
10361020

10371021
return Error::success();

‎llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -275,24 +275,22 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
275275

276276
// First check that there aren't any missing symbols.
277277
size_t NumMaterializationSideEffectsOnlySymbols = 0;
278-
SymbolNameVector ExtraSymbols;
279278
SymbolNameVector MissingSymbols;
280-
for (auto &KV : MR->getSymbols()) {
279+
for (auto &[Sym, Flags] : MR->getSymbols()) {
281280

282-
auto I = InternedResult.find(KV.first);
281+
auto I = InternedResult.find(Sym);
283282

284283
// If this is a materialization-side-effects only symbol then bump
285-
// the counter and make sure it's *not* defined, otherwise make
286-
// sure that it is defined.
287-
if (KV.second.hasMaterializationSideEffectsOnly()) {
284+
// the counter and remove in from the result, otherwise make sure that
285+
// it's defined.
286+
if (Flags.hasMaterializationSideEffectsOnly()) {
288287
++NumMaterializationSideEffectsOnlySymbols;
289-
if (I != InternedResult.end())
290-
ExtraSymbols.push_back(KV.first);
288+
InternedResult.erase(Sym);
291289
continue;
292290
} else if (I == InternedResult.end())
293-
MissingSymbols.push_back(KV.first);
291+
MissingSymbols.push_back(Sym);
294292
else if (Layer.OverrideObjectFlags)
295-
I->second.setFlags(KV.second);
293+
I->second.setFlags(Flags);
296294
}
297295

298296
// If there were missing symbols then report the error.
@@ -303,6 +301,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
303301

304302
// If there are more definitions than expected, add them to the
305303
// ExtraSymbols vector.
304+
SymbolNameVector ExtraSymbols;
306305
if (InternedResult.size() >
307306
MR->getSymbols().size() - NumMaterializationSideEffectsOnlySymbols) {
308307
for (auto &KV : InternedResult)

0 commit comments

Comments
 (0)