[llvm] [ORC] Make dynamic library search generation async (PR #81205)
Ben Langmuir via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 9 11:28:29 PST 2024
https://github.com/benlangmuir updated https://github.com/llvm/llvm-project/pull/81205
>From 779a8c064c9516bca1b752f78a45dc7ea2bc9af2 Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir at apple.com>
Date: Fri, 12 Jan 2024 15:15:33 -0800
Subject: [PATCH 1/2] [ORC] Make EPCDynamicLibrarySearchGenerator async
Switch the primary implementation of EPC lookupSymbols to be async,
keeping a synchronous wrapper for compatibility. Use the new async
implementation inside EPCDynamicLibrarySearchGenerator to work
working towards a fully async search generator (remainining is the
EPCGenericDylibManager::lookup).
---
.../Orc/ExecutorProcessControl.h | 30 ++++++++---
.../ExecutionEngine/Orc/SimpleRemoteEPC.h | 4 +-
.../Orc/EPCDynamicLibrarySearchGenerator.cpp | 53 +++++++++++--------
.../Orc/ExecutorProcessControl.cpp | 10 ++--
.../ExecutionEngine/Orc/SimpleRemoteEPC.cpp | 8 +--
.../Orc/ObjectLinkingLayerTest.cpp | 6 +--
6 files changed, 69 insertions(+), 42 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
index 9e42d6dd615dfd..51064cf2b7acd6 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h
@@ -290,8 +290,26 @@ class ExecutorProcessControl {
/// that correspond to the lookup order. If a required symbol is not
/// found then this method will return an error. If a weakly referenced
/// symbol is not found then it be assigned a '0' value.
- virtual Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) = 0;
+ Expected<std::vector<tpctypes::LookupResult>>
+ lookupSymbols(ArrayRef<LookupRequest> Request) {
+ std::promise<Expected<std::vector<tpctypes::LookupResult>>> RP;
+ auto RF = RP.get_future();
+ lookupSymbolsAsync(Request,
+ [&RP](auto Result) { RP.set_value(std::move(Result)); });
+ return RF.get();
+ }
+
+ using SymbolLookupCompleteFn =
+ unique_function<void(Expected<std::vector<tpctypes::LookupResult>>)>;
+
+ /// Search for symbols in the target process.
+ ///
+ /// The result of the lookup is a 2-dimensional array of target addresses
+ /// that correspond to the lookup order. If a required symbol is not
+ /// found then this method will return an error. If a weakly referenced
+ /// symbol is not found then it be assigned a '0' value.
+ virtual void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn F) = 0;
/// Run function with a main-like signature.
virtual Expected<int32_t> runAsMain(ExecutorAddr MainFnAddr,
@@ -462,8 +480,8 @@ class UnsupportedExecutorProcessControl : public ExecutorProcessControl,
llvm_unreachable("Unsupported");
}
- Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) override {
+ void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn F) override {
llvm_unreachable("Unsupported");
}
@@ -510,8 +528,8 @@ class SelfExecutorProcessControl : public ExecutorProcessControl,
Expected<tpctypes::DylibHandle> loadDylib(const char *DylibPath) override;
- Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) override;
+ void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn F) override;
Expected<int32_t> runAsMain(ExecutorAddr MainFnAddr,
ArrayRef<std::string> Args) override;
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h b/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
index 25b79be48810c6..c10b8df01cc0a4 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/SimpleRemoteEPC.h
@@ -71,8 +71,8 @@ class SimpleRemoteEPC : public ExecutorProcessControl,
Expected<tpctypes::DylibHandle> loadDylib(const char *DylibPath) override;
- Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) override;
+ void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn F) override;
Expected<int32_t> runAsMain(ExecutorAddr MainFnAddr,
ArrayRef<std::string> Args) override;
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
index 460f4e1c448e67..88cc3b04fb6425 100644
--- a/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.h"
+#include "llvm/Support/Error.h"
namespace llvm {
namespace orc {
@@ -39,32 +40,38 @@ Error EPCDynamicLibrarySearchGenerator::tryToGenerate(
LookupSymbols.add(KV.first, SymbolLookupFlags::WeaklyReferencedSymbol);
}
- SymbolMap NewSymbols;
-
ExecutorProcessControl::LookupRequest Request(H, LookupSymbols);
- auto Result = EPC.lookupSymbols(Request);
- if (!Result)
- return Result.takeError();
-
- assert(Result->size() == 1 && "Results for more than one library returned");
- assert(Result->front().size() == LookupSymbols.size() &&
- "Result has incorrect number of elements");
-
- auto ResultI = Result->front().begin();
- for (auto &KV : LookupSymbols) {
- if (ResultI->getAddress())
- NewSymbols[KV.first] = *ResultI;
- ++ResultI;
- }
+ // Copy-capture LookupSymbols, since LookupRequest keeps a reference.
+ EPC.lookupSymbolsAsync(Request, [this, &JD, LS = std::move(LS),
+ LookupSymbols](auto Result) mutable {
+ if (!Result)
+ return LS.continueLookup(Result.takeError());
- // If there were no resolved symbols bail out.
- if (NewSymbols.empty())
- return Error::success();
+ assert(Result->size() == 1 && "Results for more than one library returned");
+ assert(Result->front().size() == LookupSymbols.size() &&
+ "Result has incorrect number of elements");
+
+ SymbolMap NewSymbols;
+ auto ResultI = Result->front().begin();
+ for (auto &KV : LookupSymbols) {
+ if (ResultI->getAddress())
+ NewSymbols[KV.first] = *ResultI;
+ ++ResultI;
+ }
+
+ // If there were no resolved symbols bail out.
+ if (NewSymbols.empty())
+ return LS.continueLookup(Error::success());
+
+ // Define resolved symbols.
+ Error Err = AddAbsoluteSymbols
+ ? AddAbsoluteSymbols(JD, std::move(NewSymbols))
+ : JD.define(absoluteSymbols(std::move(NewSymbols)));
+
+ LS.continueLookup(std::move(Err));
+ });
- // Define resolved symbols.
- if (AddAbsoluteSymbols)
- return AddAbsoluteSymbols(JD, std::move(NewSymbols));
- return JD.define(absoluteSymbols(std::move(NewSymbols)));
+ return Error::success();
}
} // end namespace orc
diff --git a/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
index f0c551cd778047..efafca949e61ef 100644
--- a/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ExecutorProcessControl.cpp
@@ -89,8 +89,9 @@ SelfExecutorProcessControl::loadDylib(const char *DylibPath) {
return ExecutorAddr::fromPtr(Dylib.getOSSpecificHandle());
}
-Expected<std::vector<tpctypes::LookupResult>>
-SelfExecutorProcessControl::lookupSymbols(ArrayRef<LookupRequest> Request) {
+void SelfExecutorProcessControl::lookupSymbolsAsync(
+ ArrayRef<LookupRequest> Request,
+ ExecutorProcessControl::SymbolLookupCompleteFn Complete) {
std::vector<tpctypes::LookupResult> R;
for (auto &Elem : Request) {
@@ -105,7 +106,8 @@ SelfExecutorProcessControl::lookupSymbols(ArrayRef<LookupRequest> Request) {
// FIXME: Collect all failing symbols before erroring out.
SymbolNameVector MissingSymbols;
MissingSymbols.push_back(Sym);
- return make_error<SymbolsNotFound>(SSP, std::move(MissingSymbols));
+ return Complete(
+ make_error<SymbolsNotFound>(SSP, std::move(MissingSymbols)));
}
// FIXME: determine accurate JITSymbolFlags.
R.back().push_back(
@@ -113,7 +115,7 @@ SelfExecutorProcessControl::lookupSymbols(ArrayRef<LookupRequest> Request) {
}
}
- return R;
+ Complete(std::move(R));
}
Expected<int32_t>
diff --git a/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp b/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
index 3d3ca891d88108..c681d8fc8c5fb1 100644
--- a/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
@@ -29,8 +29,8 @@ SimpleRemoteEPC::loadDylib(const char *DylibPath) {
return DylibMgr->open(DylibPath, 0);
}
-Expected<std::vector<tpctypes::LookupResult>>
-SimpleRemoteEPC::lookupSymbols(ArrayRef<LookupRequest> Request) {
+void SimpleRemoteEPC::lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn Complete) {
std::vector<tpctypes::LookupResult> Result;
for (auto &Element : Request) {
@@ -40,9 +40,9 @@ SimpleRemoteEPC::lookupSymbols(ArrayRef<LookupRequest> Request) {
for (auto Addr : *R)
Result.back().push_back(Addr);
} else
- return R.takeError();
+ return Complete(R.takeError());
}
- return std::move(Result);
+ Complete(std::move(Result));
}
Expected<int32_t> SimpleRemoteEPC::runAsMain(ExecutorAddr MainFnAddr,
diff --git a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
index edd12ebb62e1a4..7ab3e40df7459d 100644
--- a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
@@ -189,8 +189,8 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
return ExecutorAddr::fromPtr((void *)nullptr);
}
- Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) override {
+ void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ SymbolLookupCompleteFn Complete) override {
std::vector<ExecutorSymbolDef> Result;
EXPECT_EQ(Request.size(), 1u);
for (auto &LR : Request) {
@@ -205,7 +205,7 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
}
}
}
- return std::vector<tpctypes::LookupResult>{1, Result};
+ Complete(std::vector<tpctypes::LookupResult>{1, Result});
}
};
>From ade04526d0e62d75805ecc1128f26584a925dabd Mon Sep 17 00:00:00 2001
From: Ben Langmuir <blangmuir at apple.com>
Date: Thu, 8 Feb 2024 14:20:15 -0800
Subject: [PATCH 2/2] [ORC] Add EPCGenericDylibManager::lookupAsync and use
from SimpleRemoteEPC
Provide an asynchronous lookup API for EPCGenericDylibManager and adopt
that from the SimpleRemoteEPC. This enables an end-to-end async
EPCDynamicLibrarySearchGenerator. Note: currently we keep the current
per-dlhandle lookup model, but a future improvement could do a single
async call for a given lookup operation.
---
.../Orc/EPCGenericDylibManager.h | 25 +++++++++-
.../Orc/EPCGenericDylibManager.cpp | 50 +++++++++++--------
.../ExecutionEngine/Orc/SimpleRemoteEPC.cpp | 41 ++++++++++-----
3 files changed, 82 insertions(+), 34 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h b/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
index 6ee2deef04d091..e0bce5d246129d 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
@@ -51,11 +51,32 @@ class EPCGenericDylibManager {
/// Looks up symbols within the given dylib.
Expected<std::vector<ExecutorSymbolDef>>
- lookup(tpctypes::DylibHandle H, const SymbolLookupSet &Lookup);
+ lookup(tpctypes::DylibHandle H, const SymbolLookupSet &Lookup) {
+ std::promise<Expected<std::vector<ExecutorSymbolDef>>> RP;
+ auto RF = RP.get_future();
+ lookupAsync(H, Lookup, [&RP](auto R) { RP.set_value(std::move(R)); });
+ return RF.get();
+ }
/// Looks up symbols within the given dylib.
Expected<std::vector<ExecutorSymbolDef>>
- lookup(tpctypes::DylibHandle H, const RemoteSymbolLookupSet &Lookup);
+ lookup(tpctypes::DylibHandle H, const RemoteSymbolLookupSet &Lookup) {
+ std::promise<Expected<std::vector<ExecutorSymbolDef>>> RP;
+ auto RF = RP.get_future();
+ lookupAsync(H, Lookup, [&RP](auto R) { RP.set_value(std::move(R)); });
+ return RF.get();
+ }
+
+ using SymbolLookupCompleteFn =
+ unique_function<void(Expected<std::vector<ExecutorSymbolDef>>)>;
+
+ /// Looks up symbols within the given dylib.
+ void lookupAsync(tpctypes::DylibHandle H, const SymbolLookupSet &Lookup,
+ SymbolLookupCompleteFn Complete);
+
+ /// Looks up symbols within the given dylib.
+ void lookupAsync(tpctypes::DylibHandle H, const RemoteSymbolLookupSet &Lookup,
+ SymbolLookupCompleteFn Complete);
private:
ExecutorProcessControl &EPC;
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp b/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
index da185c80c6c7d3..6a7cab4a55100b 100644
--- a/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
@@ -81,28 +81,38 @@ Expected<tpctypes::DylibHandle> EPCGenericDylibManager::open(StringRef Path,
return H;
}
-Expected<std::vector<ExecutorSymbolDef>>
-EPCGenericDylibManager::lookup(tpctypes::DylibHandle H,
- const SymbolLookupSet &Lookup) {
- Expected<std::vector<ExecutorSymbolDef>> Result(
- (std::vector<ExecutorSymbolDef>()));
- if (auto Err =
- EPC.callSPSWrapper<rt::SPSSimpleExecutorDylibManagerLookupSignature>(
- SAs.Lookup, Result, SAs.Instance, H, Lookup))
- return std::move(Err);
- return Result;
+void EPCGenericDylibManager::lookupAsync(tpctypes::DylibHandle H,
+ const SymbolLookupSet &Lookup,
+ SymbolLookupCompleteFn Complete) {
+ EPC.callSPSWrapperAsync<rt::SPSSimpleExecutorDylibManagerLookupSignature>(
+ SAs.Lookup,
+ [Complete = std::move(Complete)](
+ Error SerializationErr,
+ Expected<std::vector<ExecutorSymbolDef>> Result) mutable {
+ if (SerializationErr) {
+ cantFail(Result.takeError());
+ Complete(std::move(SerializationErr));
+ }
+ Complete(std::move(Result));
+ },
+ SAs.Instance, H, Lookup);
}
-Expected<std::vector<ExecutorSymbolDef>>
-EPCGenericDylibManager::lookup(tpctypes::DylibHandle H,
- const RemoteSymbolLookupSet &Lookup) {
- Expected<std::vector<ExecutorSymbolDef>> Result(
- (std::vector<ExecutorSymbolDef>()));
- if (auto Err =
- EPC.callSPSWrapper<rt::SPSSimpleExecutorDylibManagerLookupSignature>(
- SAs.Lookup, Result, SAs.Instance, H, Lookup))
- return std::move(Err);
- return Result;
+void EPCGenericDylibManager::lookupAsync(tpctypes::DylibHandle H,
+ const RemoteSymbolLookupSet &Lookup,
+ SymbolLookupCompleteFn Complete) {
+ EPC.callSPSWrapperAsync<rt::SPSSimpleExecutorDylibManagerLookupSignature>(
+ SAs.Lookup,
+ [Complete = std::move(Complete)](
+ Error SerializationErr,
+ Expected<std::vector<ExecutorSymbolDef>> Result) mutable {
+ if (SerializationErr) {
+ cantFail(Result.takeError());
+ Complete(std::move(SerializationErr));
+ }
+ Complete(std::move(Result));
+ },
+ SAs.Instance, H, Lookup);
}
} // end namespace orc
diff --git a/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp b/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
index c681d8fc8c5fb1..a81019cb1dabb0 100644
--- a/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/SimpleRemoteEPC.cpp
@@ -29,20 +29,37 @@ SimpleRemoteEPC::loadDylib(const char *DylibPath) {
return DylibMgr->open(DylibPath, 0);
}
+/// Async helper to chain together calls to DylibMgr::lookupAsync to fulfill all
+/// all the requests.
+/// FIXME: The dylib manager should support multiple LookupRequests natively.
+static void
+lookupSymbolsAsyncHelper(EPCGenericDylibManager &DylibMgr,
+ ArrayRef<SimpleRemoteEPC::LookupRequest> Request,
+ std::vector<tpctypes::LookupResult> Result,
+ SimpleRemoteEPC::SymbolLookupCompleteFn Complete) {
+ if (Request.empty())
+ return Complete(std::move(Result));
+
+ auto &Element = Request.front();
+ DylibMgr.lookupAsync(Element.Handle, Element.Symbols,
+ [&DylibMgr, Request, Complete = std::move(Complete),
+ Result = std::move(Result)](auto R) mutable {
+ if (!R)
+ return Complete(R.takeError());
+ Result.push_back({});
+ Result.back().reserve(R->size());
+ for (auto Addr : *R)
+ Result.back().push_back(Addr);
+
+ lookupSymbolsAsyncHelper(
+ DylibMgr, Request.drop_front(), std::move(Result),
+ std::move(Complete));
+ });
+}
+
void SimpleRemoteEPC::lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
SymbolLookupCompleteFn Complete) {
- std::vector<tpctypes::LookupResult> Result;
-
- for (auto &Element : Request) {
- if (auto R = DylibMgr->lookup(Element.Handle, Element.Symbols)) {
- Result.push_back({});
- Result.back().reserve(R->size());
- for (auto Addr : *R)
- Result.back().push_back(Addr);
- } else
- return Complete(R.takeError());
- }
- Complete(std::move(Result));
+ lookupSymbolsAsyncHelper(*DylibMgr, Request, {}, std::move(Complete));
}
Expected<int32_t> SimpleRemoteEPC::runAsMain(ExecutorAddr MainFnAddr,
More information about the llvm-commits
mailing list