[llvm] 0074cea - [ORC] Get rid of ObjectLinkingLayer::Plugin::getSyntheticSymbolDependencies.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 22 01:58:54 PDT 2024
Author: Lang Hames
Date: 2024-09-22T18:51:17+10:00
New Revision: 0074cea432e268ed126b12c6e7fd4df2e1707a77
URL: https://github.com/llvm/llvm-project/commit/0074cea432e268ed126b12c6e7fd4df2e1707a77
DIFF: https://github.com/llvm/llvm-project/commit/0074cea432e268ed126b12c6e7fd4df2e1707a77.diff
LOG: [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.
Added:
Modified:
llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h
llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h
llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h
index 4ef208dbbca22c..6bbc9b211333d5 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/COFFPlatform.h
@@ -99,9 +99,6 @@ class COFFPlatform : public Platform {
jitlink::LinkGraph &G,
jitlink::PassConfiguration &Config) override;
- SyntheticSymbolDependenciesMap
- getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
-
// FIXME: We should be tentatively tracking scraped sections and discarding
// if the MR fails.
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -116,9 +113,6 @@ class COFFPlatform : public Platform {
ResourceKey SrcKey) override {}
private:
- using InitSymbolDepMap =
- DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
-
Error associateJITDylibHeaderSymbol(jitlink::LinkGraph &G,
MaterializationResponsibility &MR,
bool Bootstrap);
@@ -131,7 +125,6 @@ class COFFPlatform : public Platform {
std::mutex PluginMutex;
COFFPlatform &CP;
- InitSymbolDepMap InitSymbolDeps;
};
struct JDBootstrapState {
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h
index 9b635064fc6a54..2c5b1104926960 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ELFNixPlatform.h
@@ -137,9 +137,6 @@ class ELFNixPlatform : public Platform {
jitlink::LinkGraph &G,
jitlink::PassConfiguration &Config) override;
- SyntheticSymbolDependenciesMap
- getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
-
// FIXME: We should be tentatively tracking scraped sections and discarding
// if the MR fails.
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -154,9 +151,6 @@ class ELFNixPlatform : public Platform {
ResourceKey SrcKey) override {}
private:
- using InitSymbolDepMap =
- DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
-
void addInitializerSupportPasses(MaterializationResponsibility &MR,
jitlink::PassConfiguration &Config);
@@ -175,7 +169,6 @@ class ELFNixPlatform : public Platform {
std::mutex PluginMutex;
ELFNixPlatform &MP;
- InitSymbolDepMap InitSymbolDeps;
};
using SendInitializerSequenceFn =
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
index 2ffde1f9eb10e8..8c7d94a3b1f3bc 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
@@ -202,9 +202,6 @@ class MachOPlatform : public Platform {
jitlink::LinkGraph &G,
jitlink::PassConfiguration &Config) override;
- SyntheticSymbolDependenciesMap
- getSyntheticSymbolDependencies(MaterializationResponsibility &MR) override;
-
// FIXME: We should be tentatively tracking scraped sections and discarding
// if the MR fails.
Error notifyFailed(MaterializationResponsibility &MR) override {
@@ -219,9 +216,6 @@ class MachOPlatform : public Platform {
ResourceKey SrcKey) override {}
private:
- using InitSymbolDepMap =
- DenseMap<MaterializationResponsibility *, JITLinkSymbolSet>;
-
struct UnwindSections {
SmallVector<ExecutorAddrRange> CodeRanges;
ExecutorAddrRange DwarfSection;
@@ -282,7 +276,6 @@ class MachOPlatform : public Platform {
// JITDylibs are removed.
DenseMap<JITDylib *, ObjCImageInfo> ObjCImageInfos;
DenseMap<JITDylib *, ExecutorAddr> HeaderAddrs;
- InitSymbolDepMap InitSymbolDeps;
};
using GetJITDylibHeaderSendResultFn =
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
index 689daba8ae73b8..6e0c46bfa8c4a4 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
@@ -58,10 +58,6 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
/// configured.
class Plugin {
public:
- using JITLinkSymbolSet = DenseSet<jitlink::Symbol *>;
- using SyntheticSymbolDependenciesMap =
- DenseMap<SymbolStringPtr, JITLinkSymbolSet>;
-
virtual ~Plugin();
virtual void modifyPassConfig(MaterializationResponsibility &MR,
jitlink::LinkGraph &G,
@@ -82,15 +78,6 @@ class ObjectLinkingLayer : public RTTIExtends<ObjectLinkingLayer, ObjectLayer>,
virtual Error notifyRemovingResources(JITDylib &JD, ResourceKey K) = 0;
virtual void notifyTransferringResources(JITDylib &JD, ResourceKey DstKey,
ResourceKey SrcKey) = 0;
-
- /// Return any dependencies that synthetic symbols (e.g. init symbols)
- /// have on symbols in the LinkGraph.
- /// This is used by the ObjectLinkingLayer to update the dependencies for
- /// the synthetic symbols.
- virtual SyntheticSymbolDependenciesMap
- getSyntheticSymbolDependencies(MaterializationResponsibility &MR) {
- return SyntheticSymbolDependenciesMap();
- }
};
using ReturnObjectBufferFunction =
diff --git a/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
index c8f5a99099eaed..eab0dfa47e1e7d 100644
--- a/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/COFFPlatform.cpp
@@ -786,20 +786,6 @@ void COFFPlatform::COFFPlatformPlugin::modifyPassConfig(
});
}
-ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
-COFFPlatform::COFFPlatformPlugin::getSyntheticSymbolDependencies(
- MaterializationResponsibility &MR) {
- std::lock_guard<std::mutex> Lock(PluginMutex);
- auto I = InitSymbolDeps.find(&MR);
- if (I != InitSymbolDeps.end()) {
- SyntheticSymbolDependenciesMap Result;
- Result[MR.getInitializerSymbol()] = std::move(I->second);
- InitSymbolDeps.erase(&MR);
- return Result;
- }
- return SyntheticSymbolDependenciesMap();
-}
-
Error COFFPlatform::COFFPlatformPlugin::associateJITDylibHeaderSymbol(
jitlink::LinkGraph &G, MaterializationResponsibility &MR,
bool IsBootstraping) {
@@ -859,16 +845,37 @@ Error COFFPlatform::COFFPlatformPlugin::registerObjectPlatformSections(
Error COFFPlatform::COFFPlatformPlugin::preserveInitializerSections(
jitlink::LinkGraph &G, MaterializationResponsibility &MR) {
- JITLinkSymbolSet InitSectionSymbols;
- for (auto &Sec : G.sections())
- if (isCOFFInitializerSection(Sec.getName()))
- for (auto *B : Sec.blocks())
- if (!B->edges_empty())
- InitSectionSymbols.insert(
- &G.addAnonymousSymbol(*B, 0, 0, false, true));
-
- std::lock_guard<std::mutex> Lock(PluginMutex);
- InitSymbolDeps[&MR] = InitSectionSymbols;
+
+ if (const auto &InitSymName = MR.getInitializerSymbol()) {
+
+ jitlink::Symbol *InitSym = nullptr;
+
+ for (auto &InitSection : G.sections()) {
+ // Skip non-init sections.
+ if (!isCOFFInitializerSection(InitSection.getName()) ||
+ InitSection.empty())
+ continue;
+
+ // Create the init symbol if it has not been created already and attach it
+ // to the first block.
+ if (!InitSym) {
+ auto &B = **InitSection.blocks().begin();
+ InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
+ jitlink::Linkage::Strong,
+ jitlink::Scope::Default, false, true);
+ }
+
+ // Add keep-alive edges to anonymous symbols in all other init blocks.
+ for (auto *B : InitSection.blocks()) {
+ if (B == &InitSym->getBlock())
+ continue;
+
+ auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
+ InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
+ }
+ }
+ }
+
return Error::success();
}
diff --git a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
index 2b6c4b9e7f4316..67c920a40ea2e5 100644
--- a/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ELFNixPlatform.cpp
@@ -616,20 +616,6 @@ void ELFNixPlatform::ELFNixPlatformPlugin::modifyPassConfig(
addEHAndTLVSupportPasses(MR, Config);
}
-ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
-ELFNixPlatform::ELFNixPlatformPlugin::getSyntheticSymbolDependencies(
- MaterializationResponsibility &MR) {
- std::lock_guard<std::mutex> Lock(PluginMutex);
- auto I = InitSymbolDeps.find(&MR);
- if (I != InitSymbolDeps.end()) {
- SyntheticSymbolDependenciesMap Result;
- Result[MR.getInitializerSymbol()] = std::move(I->second);
- InitSymbolDeps.erase(&MR);
- return Result;
- }
- return SyntheticSymbolDependenciesMap();
-}
-
void ELFNixPlatform::ELFNixPlatformPlugin::addInitializerSupportPasses(
MaterializationResponsibility &MR, jitlink::PassConfiguration &Config) {
@@ -737,34 +723,34 @@ void ELFNixPlatform::ELFNixPlatformPlugin::addEHAndTLVSupportPasses(
Error ELFNixPlatform::ELFNixPlatformPlugin::preserveInitSections(
jitlink::LinkGraph &G, MaterializationResponsibility &MR) {
- JITLinkSymbolSet InitSectionSymbols;
- for (auto &InitSection : G.sections()) {
- // Skip non-init sections.
- if (!isELFInitializerSection(InitSection.getName()))
- continue;
-
- // Make a pass over live symbols in the section: those blocks are already
- // preserved.
- DenseSet<jitlink::Block *> AlreadyLiveBlocks;
- for (auto &Sym : InitSection.symbols()) {
- auto &B = Sym->getBlock();
- if (Sym->isLive() && Sym->getOffset() == 0 &&
- Sym->getSize() == B.getSize() && !AlreadyLiveBlocks.count(&B)) {
- InitSectionSymbols.insert(Sym);
- AlreadyLiveBlocks.insert(&B);
+ if (const auto &InitSymName = MR.getInitializerSymbol()) {
+
+ jitlink::Symbol *InitSym = nullptr;
+
+ for (auto &InitSection : G.sections()) {
+ // Skip non-init sections.
+ if (!isELFInitializerSection(InitSection.getName()) ||
+ InitSection.empty())
+ continue;
+
+ // Create the init symbol if it has not been created already and attach it
+ // to the first block.
+ if (!InitSym) {
+ auto &B = **InitSection.blocks().begin();
+ InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
+ jitlink::Linkage::Strong,
+ jitlink::Scope::Default, false, true);
}
- }
- // Add anonymous symbols to preserve any not-already-preserved blocks.
- for (auto *B : InitSection.blocks())
- if (!AlreadyLiveBlocks.count(B))
- InitSectionSymbols.insert(
- &G.addAnonymousSymbol(*B, 0, B->getSize(), false, true));
- }
+ // Add keep-alive edges to anonymous symbols in all other init blocks.
+ for (auto *B : InitSection.blocks()) {
+ if (B == &InitSym->getBlock())
+ continue;
- if (!InitSectionSymbols.empty()) {
- std::lock_guard<std::mutex> Lock(PluginMutex);
- InitSymbolDeps[&MR] = std::move(InitSectionSymbols);
+ auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
+ InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
+ }
+ }
}
return Error::success();
diff --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index e56d6b47799c0b..0248043493c23f 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -861,20 +861,6 @@ void MachOPlatform::MachOPlatformPlugin::modifyPassConfig(
[this](LinkGraph &G) { return bootstrapPipelineEnd(G); });
}
-ObjectLinkingLayer::Plugin::SyntheticSymbolDependenciesMap
-MachOPlatform::MachOPlatformPlugin::getSyntheticSymbolDependencies(
- MaterializationResponsibility &MR) {
- std::lock_guard<std::mutex> Lock(PluginMutex);
- auto I = InitSymbolDeps.find(&MR);
- if (I != InitSymbolDeps.end()) {
- SyntheticSymbolDependenciesMap Result;
- Result[MR.getInitializerSymbol()] = std::move(I->second);
- InitSymbolDeps.erase(&MR);
- return Result;
- }
- return SyntheticSymbolDependenciesMap();
-}
-
Error MachOPlatform::MachOPlatformPlugin::bootstrapPipelineStart(
jitlink::LinkGraph &G) {
// Increment the active graphs count in BootstrapInfo.
@@ -998,40 +984,38 @@ Error MachOPlatform::MachOPlatformPlugin::preserveImportantSections(
// Init sections are important: We need to preserve them and so that their
// addresses can be captured and reported to the ORC runtime in
// registerObjectPlatformSections.
- JITLinkSymbolSet InitSectionSymbols;
- for (auto &InitSectionName : MachOInitSectionNames) {
- // Skip ObjCImageInfo -- this shouldn't have any dependencies, and we may
- // remove it later.
- if (InitSectionName == MachOObjCImageInfoSectionName)
- continue;
+ if (const auto &InitSymName = MR.getInitializerSymbol()) {
- // Skip non-init sections.
- auto *InitSection = G.findSectionByName(InitSectionName);
- if (!InitSection)
- continue;
+ jitlink::Symbol *InitSym = nullptr;
+ for (auto &InitSectionName : MachOInitSectionNames) {
+ // Skip ObjCImageInfo -- this shouldn't have any dependencies, and we may
+ // remove it later.
+ if (InitSectionName == MachOObjCImageInfoSectionName)
+ continue;
- // Make a pass over live symbols in the section: those blocks are already
- // preserved.
- DenseSet<jitlink::Block *> AlreadyLiveBlocks;
- for (auto &Sym : InitSection->symbols()) {
- auto &B = Sym->getBlock();
- if (Sym->isLive() && Sym->getOffset() == 0 &&
- Sym->getSize() == B.getSize() && !AlreadyLiveBlocks.count(&B)) {
- InitSectionSymbols.insert(Sym);
- AlreadyLiveBlocks.insert(&B);
+ // Skip non-init sections.
+ auto *InitSection = G.findSectionByName(InitSectionName);
+ if (!InitSection || InitSection->empty())
+ continue;
+
+ // Create the init symbol if it has not been created already and attach it
+ // to the first block.
+ if (!InitSym) {
+ auto &B = **InitSection->blocks().begin();
+ InitSym = &G.addDefinedSymbol(B, 0, *InitSymName, B.getSize(),
+ jitlink::Linkage::Strong,
+ jitlink::Scope::Default, false, true);
}
- }
- // Add anonymous symbols to preserve any not-already-preserved blocks.
- for (auto *B : InitSection->blocks())
- if (!AlreadyLiveBlocks.count(B))
- InitSectionSymbols.insert(
- &G.addAnonymousSymbol(*B, 0, B->getSize(), false, true));
- }
+ // Add keep-alive edges to anonymous symbols in all other init blocks.
+ for (auto *B : InitSection->blocks()) {
+ if (B == &InitSym->getBlock())
+ continue;
- if (!InitSectionSymbols.empty()) {
- std::lock_guard<std::mutex> Lock(PluginMutex);
- InitSymbolDeps[&MR] = std::move(InitSectionSymbols);
+ auto &S = G.addAnonymousSymbol(*B, 0, B->getSize(), false, true);
+ InitSym->getBlock().addEdge(jitlink::Edge::KeepAlive, 0, S, 0);
+ }
+ }
}
return Error::success();
diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index 19d83072169aa8..25ab154a01d674 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -275,24 +275,22 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
// First check that there aren't any missing symbols.
size_t NumMaterializationSideEffectsOnlySymbols = 0;
- SymbolNameVector ExtraSymbols;
SymbolNameVector MissingSymbols;
- for (auto &KV : MR->getSymbols()) {
+ for (auto &[Sym, Flags] : MR->getSymbols()) {
- auto I = InternedResult.find(KV.first);
+ auto I = InternedResult.find(Sym);
// If this is a materialization-side-effects only symbol then bump
- // the counter and make sure it's *not* defined, otherwise make
- // sure that it is defined.
- if (KV.second.hasMaterializationSideEffectsOnly()) {
+ // the counter and remove in from the result, otherwise make sure that
+ // it's defined.
+ if (Flags.hasMaterializationSideEffectsOnly()) {
++NumMaterializationSideEffectsOnlySymbols;
- if (I != InternedResult.end())
- ExtraSymbols.push_back(KV.first);
+ InternedResult.erase(Sym);
continue;
} else if (I == InternedResult.end())
- MissingSymbols.push_back(KV.first);
+ MissingSymbols.push_back(Sym);
else if (Layer.OverrideObjectFlags)
- I->second.setFlags(KV.second);
+ I->second.setFlags(Flags);
}
// If there were missing symbols then report the error.
@@ -303,6 +301,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
// If there are more definitions than expected, add them to the
// ExtraSymbols vector.
+ SymbolNameVector ExtraSymbols;
if (InternedResult.size() >
MR->getSymbols().size() - NumMaterializationSideEffectsOnlySymbols) {
for (auto &KV : InternedResult)
More information about the llvm-commits
mailing list