From 0e5b60326e306cba3f4434692e5d37ffee8a2ae9 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Tue, 25 Sep 2018 04:43:38 +0000 Subject: [PATCH] [ORC] Switch to asynchronous resolution in JITSymbolResolver. Asynchronous resolution (where the caller receives a callback once the requested set of symbols are resolved) is a core part of the new concurrent ORC APIs. This change extends the asynchronous resolution model down to RuntimeDyld, which is necessary to prevent deadlocks when compiling/linking on a fixed number of threads: If RuntimeDyld's linking process were a blocking operation, then any complete K-graph in a program will require at least K threads to link in the worst case, as each thread would block waiting for all the others to complete. Using callbacks instead allows the work to be passed between dependent threads until it is complete. For backwards compatibility, all existing RuntimeDyld functions will continue to operate in blocking mode as before. This change will enable the introduction of a new async finalization process in a subsequent patch to enable asynchronous JIT linking. llvm-svn: 342939 --- llvm/include/llvm/ExecutionEngine/JITSymbol.h | 6 +- llvm/include/llvm/ExecutionEngine/Orc/Core.h | 1 + .../include/llvm/ExecutionEngine/Orc/Legacy.h | 3 +- llvm/lib/ExecutionEngine/Orc/Legacy.cpp | 42 ++++----- .../Orc/RTDyldObjectLinkingLayer.cpp | 40 ++++++--- .../ExecutionEngine/RuntimeDyld/JITSymbol.cpp | 38 ++++---- .../RuntimeDyld/RuntimeDyld.cpp | 88 +++++++++++-------- .../RuntimeDyld/RuntimeDyldChecker.cpp | 22 ++++- .../RuntimeDyld/RuntimeDyldCheckerImpl.h | 3 + .../RuntimeDyld/RuntimeDyldImpl.h | 3 + 10 files changed, 153 insertions(+), 93 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/JITSymbol.h b/llvm/include/llvm/ExecutionEngine/JITSymbol.h index 8956033494b7..18b972ed8291 100644 --- a/llvm/include/llvm/ExecutionEngine/JITSymbol.h +++ b/llvm/include/llvm/ExecutionEngine/JITSymbol.h @@ -333,6 +333,7 @@ class JITSymbolResolver { public: using LookupSet = std::set; using LookupResult = std::map; + using OnResolvedFunction = std::function)>; virtual ~JITSymbolResolver() = default; @@ -341,7 +342,8 @@ public: /// /// This method will return an error if any of the given symbols can not be /// resolved, or if the resolution process itself triggers an error. - virtual Expected lookup(const LookupSet &Symbols) = 0; + virtual void lookup(const LookupSet &Symbols, + OnResolvedFunction OnResolved) = 0; /// Returns the subset of the given symbols that should be materialized by /// the caller. Only weak/common symbols should be looked up, as strong @@ -359,7 +361,7 @@ public: /// Performs lookup by, for each symbol, first calling /// findSymbolInLogicalDylib and if that fails calling /// findSymbol. - Expected lookup(const LookupSet &Symbols) final; + void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) final; /// Performs flags lookup by calling findSymbolInLogicalDylib and /// returning the flags value for that symbol. diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h index 6440d8197d4f..c7a925ced27f 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h @@ -376,6 +376,7 @@ private: class AsynchronousSymbolQuery { friend class ExecutionSession; friend class JITDylib; + friend class JITSymbolResolverAdapter; public: diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h b/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h index b87304962295..4c6162ac4b8b 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h @@ -90,12 +90,13 @@ createSymbolResolver(GetResponsibilitySetFn &&GetResponsibilitySet, std::forward(Lookup)); } +/// Legacy adapter. Remove once we kill off the old ORC layers. class JITSymbolResolverAdapter : public JITSymbolResolver { public: JITSymbolResolverAdapter(ExecutionSession &ES, SymbolResolver &R, MaterializationResponsibility *MR); Expected getResponsibilitySet(const LookupSet &Symbols) override; - Expected lookup(const LookupSet &Symbols) override; + void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) override; private: ExecutionSession &ES; diff --git a/llvm/lib/ExecutionEngine/Orc/Legacy.cpp b/llvm/lib/ExecutionEngine/Orc/Legacy.cpp index 925729e0eeee..f61376a909da 100644 --- a/llvm/lib/ExecutionEngine/Orc/Legacy.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Legacy.cpp @@ -18,34 +18,34 @@ JITSymbolResolverAdapter::JITSymbolResolverAdapter( ExecutionSession &ES, SymbolResolver &R, MaterializationResponsibility *MR) : ES(ES), R(R), MR(MR) {} -Expected -JITSymbolResolverAdapter::lookup(const LookupSet &Symbols) { +void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols, + OnResolvedFunction OnResolved) { SymbolNameSet InternedSymbols; for (auto &S : Symbols) InternedSymbols.insert(ES.getSymbolStringPool().intern(S)); - auto LookupFn = [&, this](std::shared_ptr Q, - SymbolNameSet Unresolved) { - return R.lookup(std::move(Q), std::move(Unresolved)); + auto OnResolvedWithUnwrap = [OnResolved](Expected InternedResult) { + if (!InternedResult) { + OnResolved(InternedResult.takeError()); + return; + } + + LookupResult Result; + for (auto &KV : *InternedResult) + Result[*KV.first] = std::move(KV.second); + OnResolved(Result); }; - auto RegisterDependencies = [&](const SymbolDependenceMap &Deps) { + auto Q = std::make_shared( + InternedSymbols, OnResolvedWithUnwrap, + [this](Error Err) { ES.reportError(std::move(Err)); }); + + auto Unresolved = R.lookup(Q, InternedSymbols); + if (Unresolved.empty()) { if (MR) - MR->addDependenciesForAll(Deps); - }; - - auto InternedResult = - ES.legacyLookup(std::move(LookupFn), std::move(InternedSymbols), - false, RegisterDependencies); - - if (!InternedResult) - return InternedResult.takeError(); - - JITSymbolResolver::LookupResult Result; - for (auto &KV : *InternedResult) - Result[*KV.first] = KV.second; - - return Result; + MR->addDependenciesForAll(Q->QueryRegistrations); + } else + ES.legacyFailQuery(*Q, make_error(std::move(Unresolved))); } Expected diff --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp index 31568977c33d..d87078f2e94f 100644 --- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp @@ -18,30 +18,42 @@ class JITDylibSearchOrderResolver : public JITSymbolResolver { public: JITDylibSearchOrderResolver(MaterializationResponsibility &MR) : MR(MR) {} - Expected lookup(const LookupSet &Symbols) { + void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) { auto &ES = MR.getTargetJITDylib().getExecutionSession(); SymbolNameSet InternedSymbols; + // Intern the requested symbols: lookup takes interned strings. for (auto &S : Symbols) InternedSymbols.insert(ES.getSymbolStringPool().intern(S)); + // Build an OnResolve callback to unwrap the interned strings and pass them + // to the OnResolved callback. + // FIXME: Switch to move capture of OnResolved once we have c++14. + auto OnResolvedWithUnwrap = + [OnResolved](Expected InternedResult) { + if (!InternedResult) { + OnResolved(InternedResult.takeError()); + return; + } + + LookupResult Result; + for (auto &KV : *InternedResult) + Result[*KV.first] = std::move(KV.second); + OnResolved(Result); + }; + + // We're not waiting for symbols to be ready. Just log any errors. + auto OnReady = [&ES](Error Err) { ES.reportError(std::move(Err)); }; + + // Register dependencies for all symbols contained in this set. auto RegisterDependencies = [&](const SymbolDependenceMap &Deps) { MR.addDependenciesForAll(Deps); }; - auto InternedResult = - MR.getTargetJITDylib().withSearchOrderDo([&](const JITDylibList &JDs) { - return ES.lookup(JDs, InternedSymbols, RegisterDependencies, false); - }); - - if (!InternedResult) - return InternedResult.takeError(); - - LookupResult Result; - for (auto &KV : *InternedResult) - Result[*KV.first] = std::move(KV.second); - - return Result; + MR.getTargetJITDylib().withSearchOrderDo([&](const JITDylibList &JDs) { + ES.lookup(JDs, InternedSymbols, OnResolvedWithUnwrap, OnReady, + RegisterDependencies); + }); } Expected getResponsibilitySet(const LookupSet &Symbols) { diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp index d865216cf31b..b6ef8ad9675a 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp @@ -62,34 +62,42 @@ llvm::ARMJITSymbolFlags::fromObjectSymbol(const object::SymbolRef &Symbol) { /// Performs lookup by, for each symbol, first calling /// findSymbolInLogicalDylib and if that fails calling /// findSymbol. -Expected -LegacyJITSymbolResolver::lookup(const LookupSet &Symbols) { +void LegacyJITSymbolResolver::lookup(const LookupSet &Symbols, + OnResolvedFunction OnResolved) { JITSymbolResolver::LookupResult Result; for (auto &Symbol : Symbols) { std::string SymName = Symbol.str(); if (auto Sym = findSymbolInLogicalDylib(SymName)) { if (auto AddrOrErr = Sym.getAddress()) Result[Symbol] = JITEvaluatedSymbol(*AddrOrErr, Sym.getFlags()); - else - return AddrOrErr.takeError(); - } else if (auto Err = Sym.takeError()) - return std::move(Err); - else { + else { + OnResolved(AddrOrErr.takeError()); + return; + } + } else if (auto Err = Sym.takeError()) { + OnResolved(std::move(Err)); + return; + } else { // findSymbolInLogicalDylib failed. Lets try findSymbol. if (auto Sym = findSymbol(SymName)) { if (auto AddrOrErr = Sym.getAddress()) Result[Symbol] = JITEvaluatedSymbol(*AddrOrErr, Sym.getFlags()); - else - return AddrOrErr.takeError(); - } else if (auto Err = Sym.takeError()) - return std::move(Err); - else - return make_error("Symbol not found: " + Symbol, - inconvertibleErrorCode()); + else { + OnResolved(AddrOrErr.takeError()); + return; + } + } else if (auto Err = Sym.takeError()) { + OnResolved(std::move(Err)); + return; + } else { + OnResolved(make_error("Symbol not found: " + Symbol, + inconvertibleErrorCode())); + return; + } } } - return std::move(Result); + OnResolved(std::move(Result)); } /// Performs flags lookup by calling findSymbolInLogicalDylib and diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp index 43c99b52c34d..a651e78f7f09 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -23,6 +23,8 @@ #include "llvm/Support/MathExtras.h" #include "llvm/Support/MutexGuard.h" +#include + using namespace llvm; using namespace llvm::object; @@ -996,42 +998,8 @@ void RuntimeDyldImpl::resolveRelocationList(const RelocationList &Relocs, } } -Error RuntimeDyldImpl::resolveExternalSymbols() { - StringMap ExternalSymbolMap; - - // Resolution can trigger emission of more symbols, so iterate until - // we've resolved *everything*. - { - JITSymbolResolver::LookupSet ResolvedSymbols; - - while (true) { - JITSymbolResolver::LookupSet NewSymbols; - - for (auto &RelocKV : ExternalSymbolRelocations) { - StringRef Name = RelocKV.first(); - if (!Name.empty() && !GlobalSymbolTable.count(Name) && - !ResolvedSymbols.count(Name)) - NewSymbols.insert(Name); - } - - if (NewSymbols.empty()) - break; - - auto NewResolverResults = Resolver.lookup(NewSymbols); - if (!NewResolverResults) - return NewResolverResults.takeError(); - - assert(NewResolverResults->size() == NewSymbols.size() && - "Should have errored on unresolved symbols"); - - for (auto &RRKV : *NewResolverResults) { - assert(!ResolvedSymbols.count(RRKV.first) && "Redundant resolution?"); - ExternalSymbolMap.insert(RRKV); - ResolvedSymbols.insert(RRKV.first); - } - } - } - +void RuntimeDyldImpl::applyExternalSymbolRelocations( + const StringMap ExternalSymbolMap) { while (!ExternalSymbolRelocations.empty()) { StringMap::iterator i = ExternalSymbolRelocations.begin(); @@ -1093,6 +1061,54 @@ Error RuntimeDyldImpl::resolveExternalSymbols() { ExternalSymbolRelocations.erase(i); } +} + +Error RuntimeDyldImpl::resolveExternalSymbols() { + StringMap ExternalSymbolMap; + + // Resolution can trigger emission of more symbols, so iterate until + // we've resolved *everything*. + { + JITSymbolResolver::LookupSet ResolvedSymbols; + + while (true) { + JITSymbolResolver::LookupSet NewSymbols; + + for (auto &RelocKV : ExternalSymbolRelocations) { + StringRef Name = RelocKV.first(); + if (!Name.empty() && !GlobalSymbolTable.count(Name) && + !ResolvedSymbols.count(Name)) + NewSymbols.insert(Name); + } + + if (NewSymbols.empty()) + break; + + auto NewSymbolsP = std::make_shared< + std::promise>>(); + auto NewSymbolsF = NewSymbolsP->get_future(); + Resolver.lookup(NewSymbols, + [=](Expected Result) { + NewSymbolsP->set_value(std::move(Result)); + }); + + auto NewResolverResults = NewSymbolsF.get(); + + if (!NewResolverResults) + return NewResolverResults.takeError(); + + assert(NewResolverResults->size() == NewSymbols.size() && + "Should have errored on unresolved symbols"); + + for (auto &RRKV : *NewResolverResults) { + assert(!ResolvedSymbols.count(RRKV.first) && "Redundant resolution?"); + ExternalSymbolMap.insert(RRKV); + ResolvedSymbols.insert(RRKV.first); + } + } + } + + applyExternalSymbolRelocations(ExternalSymbolMap); return Error::success(); } diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp index fa8906869b3a..79a1bbb1a77f 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp @@ -16,6 +16,7 @@ #include "llvm/MC/MCInst.h" #include "llvm/Support/Path.h" #include +#include #include #include @@ -729,15 +730,29 @@ bool RuntimeDyldCheckerImpl::checkAllRulesInBuffer(StringRef RulePrefix, return DidAllTestsPass && (NumRules != 0); } +Expected RuntimeDyldCheckerImpl::lookup( + const JITSymbolResolver::LookupSet &Symbols) const { + auto ResultP = std::make_shared< + std::promise>>(); + auto ResultF = ResultP->get_future(); + + getRTDyld().Resolver.lookup( + Symbols, [=](Expected Result) { + ResultP->set_value(std::move(Result)); + }); + return ResultF.get(); +} + bool RuntimeDyldCheckerImpl::isSymbolValid(StringRef Symbol) const { if (getRTDyld().getSymbol(Symbol)) return true; - JITSymbolResolver::LookupSet Symbols({Symbol}); - auto Result = getRTDyld().Resolver.lookup(Symbols); + auto Result = lookup({Symbol}); + if (!Result) { logAllUnhandledErrors(Result.takeError(), errs(), "RTDyldChecker: "); return false; } + assert(Result->count(Symbol) && "Missing symbol result"); return true; } @@ -751,8 +766,7 @@ uint64_t RuntimeDyldCheckerImpl::getSymbolRemoteAddr(StringRef Symbol) const { if (auto InternalSymbol = getRTDyld().getSymbol(Symbol)) return InternalSymbol.getAddress(); - JITSymbolResolver::LookupSet Symbols({Symbol}); - auto Result = getRTDyld().Resolver.lookup(Symbols); + auto Result = lookup({Symbol}); if (!Result) { logAllUnhandledErrors(Result.takeError(), errs(), "RTDyldChecker: "); return 0; diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h index b462ef2c00ce..6da1a68d06d6 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h @@ -41,6 +41,9 @@ private: RuntimeDyldImpl &getRTDyld() const { return *RTDyld.Dyld; } + Expected + lookup(const JITSymbolResolver::LookupSet &Symbols) const; + bool isSymbolValid(StringRef Symbol) const; uint64_t getSymbolLocalAddr(StringRef Symbol) const; uint64_t getSymbolRemoteAddr(StringRef Symbol) const; diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h index 9b3395241620..f01d3103f829 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h @@ -433,6 +433,9 @@ protected: const ObjectFile &Obj, ObjSectionToIDMap &ObjSectionToID, StubMap &Stubs) = 0; + void applyExternalSymbolRelocations( + const StringMap ExternalSymbolMap); + /// Resolve relocations to external symbols. Error resolveExternalSymbols();