[llvm] [ORC] Simplify DylibManager::lookupSymbols, remove LookupRequest. (PR #195954)
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 1 18:54:33 PDT 2026
https://github.com/lhames updated https://github.com/llvm/llvm-project/pull/195954
>From 77d0cc9e4e6d00d7768eb8bdccf0efcf05414b80 Mon Sep 17 00:00:00 2001
From: Lang Hames <lhames at gmail.com>
Date: Tue, 5 May 2026 11:44:40 +1000
Subject: [PATCH] [ORC] Simplify DylibManager::lookupSymbols, remove
LookupRequest.
DylibManager::lookupSymbols used to take an array of LookupRequests, where each
request specified a handle and list of symbols to lookup within that handle.
This commit replaces the array of lookup requests with a single handle and list
of symbols passed directly to lookupSymbols.
In practice all clients were passing a singlton array anyway, and simplifying
this signature significantly simplifies implementations.
---
.../llvm/ExecutionEngine/Orc/DylibManager.h | 39 ++++-----
.../Orc/EPCGenericDylibManager.h | 2 +-
.../Orc/EPCDynamicLibrarySearchGenerator.cpp | 87 +++++++++----------
.../Orc/EPCGenericDylibManager.cpp | 49 +----------
.../Orc/ExecutorResolutionGenerator.cpp | 14 ++-
.../Orc/SelfExecutorProcessControl.cpp | 34 ++++----
.../Orc/ObjectLinkingLayerTest.cpp | 26 +++---
7 files changed, 93 insertions(+), 158 deletions(-)
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/DylibManager.h b/llvm/include/llvm/ExecutionEngine/Orc/DylibManager.h
index 4cef8524c1477..2e691b5eec82b 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/DylibManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/DylibManager.h
@@ -19,8 +19,6 @@
#include "llvm/Support/MSVCErrorWorkarounds.h"
#include <future>
-#include <mutex>
-#include <vector>
namespace llvm::orc {
@@ -28,14 +26,6 @@ class SymbolLookupSet;
class LLVM_ABI DylibManager {
public:
- /// A pair of a dylib and a set of symbols to be looked up.
- struct LookupRequest {
- LookupRequest(tpctypes::DylibHandle Handle, const SymbolLookupSet &Symbols)
- : Handle(Handle), Symbols(Symbols) {}
- tpctypes::DylibHandle Handle;
- const SymbolLookupSet &Symbols;
- };
-
virtual ~DylibManager();
/// Load the dynamic library at the given path and return a handle to it.
@@ -45,29 +35,30 @@ class LLVM_ABI DylibManager {
/// 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.
- Expected<std::vector<tpctypes::LookupResult>>
- lookupSymbols(ArrayRef<LookupRequest> Request) {
- std::promise<MSVCPExpected<std::vector<tpctypes::LookupResult>>> RP;
+ /// The result of the lookup is an 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.
+ Expected<tpctypes::LookupResult>
+ lookupSymbols(tpctypes::DylibHandle H, const SymbolLookupSet &Symbols) {
+ std::promise<MSVCPExpected<tpctypes::LookupResult>> RP;
auto RF = RP.get_future();
- lookupSymbolsAsync(Request,
+ lookupSymbolsAsync(H, Symbols,
[&RP](auto Result) { RP.set_value(std::move(Result)); });
return RF.get();
}
using SymbolLookupCompleteFn =
- unique_function<void(Expected<std::vector<tpctypes::LookupResult>>)>;
+ unique_function<void(Expected<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,
+ /// The result of the lookup is an 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(tpctypes::DylibHandle H,
+ const SymbolLookupSet &Symbols,
SymbolLookupCompleteFn F) = 0;
};
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h b/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
index 2836467f07df3..3153e1a1afe22 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
@@ -90,7 +90,7 @@ class EPCGenericDylibManager : public DylibManager {
/// Search for symbols in the target process.
LLVM_ABI void
- lookupSymbolsAsync(ArrayRef<DylibManager::LookupRequest> Request,
+ lookupSymbolsAsync(tpctypes::DylibHandle H, const SymbolLookupSet &Symbols,
DylibManager::SymbolLookupCompleteFn Complete) override;
private:
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
index fbee8cc2a96b9..b2db44a625b1b 100644
--- a/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
@@ -62,51 +62,50 @@ Error EPCDynamicLibrarySearchGenerator::tryToGenerate(
LookupSymbols.add(KV.first, SymbolLookupFlags::WeaklyReferencedSymbol);
}
- DylibManager::LookupRequest Request(*H, LookupSymbols);
- // Copy-capture LookupSymbols, since LookupRequest keeps a reference.
- DylibMgr.lookupSymbolsAsync(Request, [this, &JD, LS = std::move(LS),
- LookupSymbols](auto Result) mutable {
- if (!Result) {
- LLVM_DEBUG({
- dbgs() << "EPCDynamicLibrarySearchGenerator lookup failed due to error";
+ DylibMgr.lookupSymbolsAsync(
+ *H, LookupSymbols,
+ [this, &JD, LS = std::move(LS), LookupSymbols](auto Result) mutable {
+ if (!Result) {
+ LLVM_DEBUG({
+ dbgs() << "EPCDynamicLibrarySearchGenerator lookup failed due to "
+ "error";
+ });
+ return LS.continueLookup(Result.takeError());
+ }
+
+ assert(Result->size() == LookupSymbols.size() &&
+ "Result has incorrect number of elements");
+
+ auto SymsIt = Result->begin();
+ SymbolNameSet MissingSymbols;
+ SymbolMap NewSymbols;
+ for (auto &[Name, Flags] : LookupSymbols) {
+ const auto &Sym = *SymsIt++;
+ if (Sym && Sym->getAddress())
+ NewSymbols[Name] = *Sym;
+ else if (LLVM_UNLIKELY(!Sym &&
+ Flags == SymbolLookupFlags::RequiredSymbol))
+ MissingSymbols.insert(Name);
+ }
+
+ LLVM_DEBUG({
+ dbgs() << "EPCDynamicLibrarySearchGenerator lookup returned "
+ << NewSymbols << "\n";
+ });
+
+ // If there were no resolved symbols bail out.
+ if (NewSymbols.empty())
+ return LS.continueLookup(Error::success());
+
+ if (LLVM_UNLIKELY(!MissingSymbols.empty()))
+ return LS.continueLookup(make_error<SymbolsNotFound>(
+ this->ES.getSymbolStringPool(), std::move(MissingSymbols)));
+
+ // Define resolved symbols.
+ Error Err = addAbsolutes(JD, std::move(NewSymbols));
+
+ LS.continueLookup(std::move(Err));
});
- return LS.continueLookup(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 SymsIt = Result->front().begin();
- SymbolNameSet MissingSymbols;
- SymbolMap NewSymbols;
- for (auto &[Name, Flags] : LookupSymbols) {
- const auto &Sym = *SymsIt++;
- if (Sym && Sym->getAddress())
- NewSymbols[Name] = *Sym;
- else if (LLVM_UNLIKELY(!Sym &&
- Flags == SymbolLookupFlags::RequiredSymbol))
- MissingSymbols.insert(Name);
- }
-
- LLVM_DEBUG({
- dbgs() << "EPCDynamicLibrarySearchGenerator lookup returned "
- << NewSymbols << "\n";
- });
-
- // If there were no resolved symbols bail out.
- if (NewSymbols.empty())
- return LS.continueLookup(Error::success());
-
- if (LLVM_UNLIKELY(!MissingSymbols.empty()))
- return LS.continueLookup(make_error<SymbolsNotFound>(
- this->ES.getSymbolStringPool(), std::move(MissingSymbols)));
-
- // Define resolved symbols.
- Error Err = addAbsolutes(JD, std::move(NewSymbols));
-
- LS.continueLookup(std::move(Err));
- });
return Error::success();
}
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp b/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
index 4b751c224ef05..ba023f6fd723b 100644
--- a/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
@@ -39,24 +39,6 @@ class TrivialSPSSequenceSerialization<SPSRemoteSymbolLookupSetElement,
static constexpr bool available = true;
};
-template <>
-class SPSSerializationTraits<SPSRemoteSymbolLookup,
- DylibManager::LookupRequest> {
- using MemberSerialization =
- SPSArgList<SPSExecutorAddr, SPSRemoteSymbolLookupSet>;
-
-public:
- static size_t size(const DylibManager::LookupRequest &LR) {
- return MemberSerialization::size(ExecutorAddr(LR.Handle), LR.Symbols);
- }
-
- static bool serialize(SPSOutputBuffer &OB,
- const DylibManager::LookupRequest &LR) {
- return MemberSerialization::serialize(OB, ExecutorAddr(LR.Handle),
- LR.Symbols);
- }
-};
-
} // end namespace shared
Expected<EPCGenericDylibManager>
@@ -124,37 +106,10 @@ EPCGenericDylibManager::loadDylib(const char *DylibPath) {
return open(DylibPath, 0);
}
-/// Async helper to chain together calls to lookupAsync to fulfill all
-/// the requests.
-/// FIXME: The dylib manager should support multiple LookupRequests natively.
-static void
-lookupSymbolsAsyncHelper(EPCGenericDylibManager &DylibMgr,
- ArrayRef<DylibManager::LookupRequest> Request,
- std::vector<tpctypes::LookupResult> Result,
- DylibManager::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());
- llvm::append_range(Result.back(), *R);
-
- lookupSymbolsAsyncHelper(
- DylibMgr, Request.drop_front(), std::move(Result),
- std::move(Complete));
- });
-}
-
void EPCGenericDylibManager::lookupSymbolsAsync(
- ArrayRef<DylibManager::LookupRequest> Request,
+ tpctypes::DylibHandle H, const SymbolLookupSet &Symbols,
DylibManager::SymbolLookupCompleteFn Complete) {
- lookupSymbolsAsyncHelper(*this, Request, {}, std::move(Complete));
+ lookupAsync(H, Symbols, std::move(Complete));
}
} // end namespace orc
diff --git a/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp
index b63698a553763..e01a0b3612637 100644
--- a/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp
@@ -48,24 +48,20 @@ Error ExecutorResolutionGenerator::tryToGenerate(
LookupSymbols.add(Name, LookupFlag);
}
- DylibManager::LookupRequest LR(H, LookupSymbols);
DylibMgr.lookupSymbolsAsync(
- LR, [this, LS = std::move(LS), JD = JITDylibSP(&JD),
- LookupSymbols](auto Result) mutable {
+ H, LookupSymbols,
+ [this, LS = std::move(LS), JD = JITDylibSP(&JD),
+ LookupSymbols](auto Result) mutable {
if (Result) {
LLVM_DEBUG({
dbgs() << "ExecutorResolutionGenerator lookup failed due to error";
});
return LS.continueLookup(Result.takeError());
}
- assert(Result->size() == 1 &&
- "Results for more than one library returned");
- assert(Result->front().size() == LookupSymbols.size() &&
+ assert(Result->size() == LookupSymbols.size() &&
"Result has incorrect number of elements");
- // const tpctypes::LookupResult &Syms = Result->front();
- // size_t SymIdx = 0;
- auto Syms = Result->front().begin();
+ auto Syms = Result->begin();
SymbolNameSet MissingSymbols;
SymbolMap NewSyms;
for (auto &[Name, Flags] : LookupSymbols) {
diff --git a/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp b/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp
index 7c8a6054f03ae..e8236a2499f8a 100644
--- a/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp
@@ -27,7 +27,7 @@ class SelfExecutorProcessControl::InProcessDylibManager : public DylibManager {
InProcessDylibManager(char GlobalManglingPrefix);
Expected<tpctypes::DylibHandle> loadDylib(const char *DylibPath) override;
void
- lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ lookupSymbolsAsync(tpctypes::DylibHandle H, const SymbolLookupSet &Symbols,
DylibManager::SymbolLookupCompleteFn Complete) override;
private:
@@ -165,25 +165,21 @@ SelfExecutorProcessControl::InProcessDylibManager::loadDylib(
}
void SelfExecutorProcessControl::InProcessDylibManager::lookupSymbolsAsync(
- ArrayRef<LookupRequest> Request,
+ tpctypes::DylibHandle H, const SymbolLookupSet &Symbols,
DylibManager::SymbolLookupCompleteFn Complete) {
- std::vector<tpctypes::LookupResult> R;
-
- for (auto &Elem : Request) {
- sys::DynamicLibrary Dylib(Elem.Handle.toPtr<void *>());
- R.push_back(tpctypes::LookupResult());
- for (auto &KV : Elem.Symbols) {
- auto &Sym = KV.first;
- std::string Tmp((*Sym).data() + !!GlobalManglingPrefix,
- (*Sym).size() - !!GlobalManglingPrefix);
- void *Addr = Dylib.getAddressOfSymbol(Tmp.c_str());
- if (!Addr && KV.second == SymbolLookupFlags::RequiredSymbol)
- R.back().emplace_back();
- else
- // FIXME: determine accurate JITSymbolFlags.
- R.back().emplace_back(ExecutorSymbolDef(ExecutorAddr::fromPtr(Addr),
- JITSymbolFlags::Exported));
- }
+ tpctypes::LookupResult R;
+
+ sys::DynamicLibrary Dylib(H.toPtr<void *>());
+ for (auto &KV : Symbols) {
+ auto &Sym = KV.first;
+ std::string Tmp((*Sym).data() + !!GlobalManglingPrefix,
+ (*Sym).size() - !!GlobalManglingPrefix);
+ void *Addr = Dylib.getAddressOfSymbol(Tmp.c_str());
+ if (!Addr && KV.second == SymbolLookupFlags::RequiredSymbol)
+ R.emplace_back();
+ else
+ R.emplace_back(ExecutorSymbolDef(ExecutorAddr::fromPtr(Addr),
+ JITSymbolFlags::Exported));
}
Complete(std::move(R));
}
diff --git a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
index 88aabb54bf285..6edc85456e641 100644
--- a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
+++ b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
@@ -297,23 +297,21 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
return ExecutorAddr::fromPtr((void *)nullptr);
}
- void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
+ void lookupSymbolsAsync(tpctypes::DylibHandle H,
+ const SymbolLookupSet &Symbols,
SymbolLookupCompleteFn Complete) override {
- std::vector<std::optional<ExecutorSymbolDef>> Result;
- EXPECT_EQ(Request.size(), 1u);
- for (auto &LR : Request) {
- EXPECT_EQ(LR.Symbols.size(), 1u);
- for (auto &Sym : LR.Symbols) {
- if (*Sym.first == "_testFunc") {
- ExecutorSymbolDef Def{ExecutorAddr::fromPtr((void *)0x1000),
- JITSymbolFlags::Exported};
- Result.emplace_back(Def);
- } else {
- ADD_FAILURE() << "unexpected symbol request " << *Sym.first;
- }
+ tpctypes::LookupResult Result;
+ EXPECT_EQ(Symbols.size(), 1u);
+ for (auto &Sym : Symbols) {
+ if (*Sym.first == "_testFunc") {
+ ExecutorSymbolDef Def{ExecutorAddr::fromPtr((void *)0x1000),
+ JITSymbolFlags::Exported};
+ Result.emplace_back(Def);
+ } else {
+ ADD_FAILURE() << "unexpected symbol request " << *Sym.first;
}
}
- Complete(std::vector<tpctypes::LookupResult>{1, Result});
+ Complete(std::move(Result));
}
Expected<std::unique_ptr<DylibManager>> createDefaultDylibMgr() override {
More information about the llvm-commits
mailing list