From: Lang Hames Date: Tue, 25 Sep 2018 04:54:03 +0000 (+0000) Subject: Revert "[ORC] Switch to asynchronous resolution in JITSymbolResolver." X-Git-Tag: llvmorg-8.0.0-rc1~8043 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=99bfa95ae3d03bd4a822377e21b7a4d22452c3a5;p=platform%2Fupstream%2Fllvm.git Revert "[ORC] Switch to asynchronous resolution in JITSymbolResolver." This reverts commit r342939. MSVC's promise/future implementation does not like types that are not default constructible. Reverting while I figure out a solution. llvm-svn: 342941 --- diff --git a/llvm/include/llvm/ExecutionEngine/JITSymbol.h b/llvm/include/llvm/ExecutionEngine/JITSymbol.h index 18b972e..8956033 100644 --- a/llvm/include/llvm/ExecutionEngine/JITSymbol.h +++ b/llvm/include/llvm/ExecutionEngine/JITSymbol.h @@ -333,7 +333,6 @@ class JITSymbolResolver { public: using LookupSet = std::set; using LookupResult = std::map; - using OnResolvedFunction = std::function)>; virtual ~JITSymbolResolver() = default; @@ -342,8 +341,7 @@ 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 void lookup(const LookupSet &Symbols, - OnResolvedFunction OnResolved) = 0; + virtual Expected lookup(const LookupSet &Symbols) = 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 @@ -361,7 +359,7 @@ public: /// Performs lookup by, for each symbol, first calling /// findSymbolInLogicalDylib and if that fails calling /// findSymbol. - void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) final; + Expected lookup(const LookupSet &Symbols) 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 c7a925c..6440d81 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h @@ -376,7 +376,6 @@ 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 4c6162a..b873049 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/Legacy.h @@ -90,13 +90,12 @@ 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; - void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) override; + Expected lookup(const LookupSet &Symbols) override; private: ExecutionSession &ES; diff --git a/llvm/lib/ExecutionEngine/Orc/Legacy.cpp b/llvm/lib/ExecutionEngine/Orc/Legacy.cpp index f61376a..925729e 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) {} -void JITSymbolResolverAdapter::lookup(const LookupSet &Symbols, - OnResolvedFunction OnResolved) { +Expected +JITSymbolResolverAdapter::lookup(const LookupSet &Symbols) { SymbolNameSet InternedSymbols; for (auto &S : Symbols) InternedSymbols.insert(ES.getSymbolStringPool().intern(S)); - auto OnResolvedWithUnwrap = [OnResolved](Expected InternedResult) { - if (!InternedResult) { - OnResolved(InternedResult.takeError()); - return; - } + auto LookupFn = [&, this](std::shared_ptr Q, + SymbolNameSet Unresolved) { + return R.lookup(std::move(Q), std::move(Unresolved)); + }; - LookupResult Result; - for (auto &KV : *InternedResult) - Result[*KV.first] = std::move(KV.second); - OnResolved(Result); + auto RegisterDependencies = [&](const SymbolDependenceMap &Deps) { + if (MR) + MR->addDependenciesForAll(Deps); }; - auto Q = std::make_shared( - InternedSymbols, OnResolvedWithUnwrap, - [this](Error Err) { ES.reportError(std::move(Err)); }); + auto InternedResult = + ES.legacyLookup(std::move(LookupFn), std::move(InternedSymbols), + false, RegisterDependencies); - auto Unresolved = R.lookup(Q, InternedSymbols); - if (Unresolved.empty()) { - if (MR) - MR->addDependenciesForAll(Q->QueryRegistrations); - } else - ES.legacyFailQuery(*Q, make_error(std::move(Unresolved))); + if (!InternedResult) + return InternedResult.takeError(); + + JITSymbolResolver::LookupResult Result; + for (auto &KV : *InternedResult) + Result[*KV.first] = KV.second; + + return Result; } Expected diff --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp index d87078f..3156897 100644 --- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp @@ -18,42 +18,30 @@ class JITDylibSearchOrderResolver : public JITSymbolResolver { public: JITDylibSearchOrderResolver(MaterializationResponsibility &MR) : MR(MR) {} - void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) { + Expected lookup(const LookupSet &Symbols) { 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); }; - MR.getTargetJITDylib().withSearchOrderDo([&](const JITDylibList &JDs) { - ES.lookup(JDs, InternedSymbols, OnResolvedWithUnwrap, OnReady, - RegisterDependencies); - }); + 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; } Expected getResponsibilitySet(const LookupSet &Symbols) { diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp index b6ef8ad..d865216 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp @@ -62,42 +62,34 @@ llvm::ARMJITSymbolFlags::fromObjectSymbol(const object::SymbolRef &Symbol) { /// Performs lookup by, for each symbol, first calling /// findSymbolInLogicalDylib and if that fails calling /// findSymbol. -void LegacyJITSymbolResolver::lookup(const LookupSet &Symbols, - OnResolvedFunction OnResolved) { +Expected +LegacyJITSymbolResolver::lookup(const LookupSet &Symbols) { 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 { - OnResolved(AddrOrErr.takeError()); - return; - } - } else if (auto Err = Sym.takeError()) { - OnResolved(std::move(Err)); - return; - } else { + else + return AddrOrErr.takeError(); + } else if (auto Err = Sym.takeError()) + return std::move(Err); + else { // findSymbolInLogicalDylib failed. Lets try findSymbol. if (auto Sym = findSymbol(SymName)) { if (auto AddrOrErr = Sym.getAddress()) Result[Symbol] = JITEvaluatedSymbol(*AddrOrErr, Sym.getFlags()); - 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; - } + else + return AddrOrErr.takeError(); + } else if (auto Err = Sym.takeError()) + return std::move(Err); + else + return make_error("Symbol not found: " + Symbol, + inconvertibleErrorCode()); } } - OnResolved(std::move(Result)); + return 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 a651e78..43c99b5 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -23,8 +23,6 @@ #include "llvm/Support/MathExtras.h" #include "llvm/Support/MutexGuard.h" -#include - using namespace llvm; using namespace llvm::object; @@ -998,8 +996,42 @@ void RuntimeDyldImpl::resolveRelocationList(const RelocationList &Relocs, } } -void RuntimeDyldImpl::applyExternalSymbolRelocations( - const StringMap ExternalSymbolMap) { +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); + } + } + } + while (!ExternalSymbolRelocations.empty()) { StringMap::iterator i = ExternalSymbolRelocations.begin(); @@ -1061,54 +1093,6 @@ void RuntimeDyldImpl::applyExternalSymbolRelocations( 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 79a1bbb..fa89068 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp @@ -16,7 +16,6 @@ #include "llvm/MC/MCInst.h" #include "llvm/Support/Path.h" #include -#include #include #include @@ -730,29 +729,15 @@ 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; - auto Result = lookup({Symbol}); - + JITSymbolResolver::LookupSet Symbols({Symbol}); + auto Result = getRTDyld().Resolver.lookup(Symbols); if (!Result) { logAllUnhandledErrors(Result.takeError(), errs(), "RTDyldChecker: "); return false; } - assert(Result->count(Symbol) && "Missing symbol result"); return true; } @@ -766,7 +751,8 @@ uint64_t RuntimeDyldCheckerImpl::getSymbolRemoteAddr(StringRef Symbol) const { if (auto InternalSymbol = getRTDyld().getSymbol(Symbol)) return InternalSymbol.getAddress(); - auto Result = lookup({Symbol}); + JITSymbolResolver::LookupSet Symbols({Symbol}); + auto Result = getRTDyld().Resolver.lookup(Symbols); 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 6da1a68..b462ef2 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h @@ -41,9 +41,6 @@ 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 f01d310..9b33952 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h @@ -433,9 +433,6 @@ protected: const ObjectFile &Obj, ObjSectionToIDMap &ObjSectionToID, StubMap &Stubs) = 0; - void applyExternalSymbolRelocations( - const StringMap ExternalSymbolMap); - /// Resolve relocations to external symbols. Error resolveExternalSymbols();