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

include/llvm/ExecutionEngine/JITSymbol.h

@@ -333,7 +333,6 @@ class JITSymbolResolver {
 public:
   using LookupSet = std::set<StringRef>;
   using LookupResult = std::map<StringRef, JITEvaluatedSymbol>;
-  using OnResolvedFunction = std::function<void(Expected<LookupResult>)>;
 
   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<LookupResult> 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<LookupResult> lookup(const LookupSet &Symbols) final;
 
   /// Performs flags lookup by calling findSymbolInLogicalDylib and
   ///        returning the flags value for that symbol.

include/llvm/ExecutionEngine/Orc/Core.h

@@ -376,7 +376,6 @@ private:
 class AsynchronousSymbolQuery {
   friend class ExecutionSession;
   friend class JITDylib;
-  friend class JITSymbolResolverAdapter;
 
 public:
 

include/llvm/ExecutionEngine/Orc/Legacy.h

@@ -90,13 +90,12 @@ createSymbolResolver(GetResponsibilitySetFn &&GetResponsibilitySet,
       std::forward<LookupFn>(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<LookupSet> getResponsibilitySet(const LookupSet &Symbols) override;
-  void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) override;
+  Expected<LookupResult> lookup(const LookupSet &Symbols) override;
 
 private:
   ExecutionSession &ES;

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::LookupResult>
+JITSymbolResolverAdapter::lookup(const LookupSet &Symbols) {
   SymbolNameSet InternedSymbols;
   for (auto &S : Symbols)
     InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
 
-  auto OnResolvedWithUnwrap = [OnResolved](Expected<SymbolMap> InternedResult) {
-    if (!InternedResult) {
-      OnResolved(InternedResult.takeError());
-      return;
-    }
-
-    LookupResult Result;
-    for (auto &KV : *InternedResult)
-      Result[*KV.first] = std::move(KV.second);
-    OnResolved(Result);
+  auto LookupFn = [&, this](std::shared_ptr<AsynchronousSymbolQuery> Q,
+                            SymbolNameSet Unresolved) {
+    return R.lookup(std::move(Q), std::move(Unresolved));
   };
 
-  auto Q = std::make_shared<AsynchronousSymbolQuery>(
-      InternedSymbols, OnResolvedWithUnwrap,
-      [this](Error Err) { ES.reportError(std::move(Err)); });
-
-  auto Unresolved = R.lookup(Q, InternedSymbols);
-  if (Unresolved.empty()) {
+  auto RegisterDependencies = [&](const SymbolDependenceMap &Deps) {
     if (MR)
-      MR->addDependenciesForAll(Q->QueryRegistrations);
-  } else
-    ES.legacyFailQuery(*Q, make_error<SymbolsNotFound>(std::move(Unresolved)));
+      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;
 }
 
 Expected<JITSymbolResolverAdapter::LookupSet>

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<LookupResult> 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<SymbolMap> 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<LookupSet> getResponsibilitySet(const LookupSet &Symbols) {

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<JITSymbolResolver::LookupResult>
+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<StringError>("Symbol not found: " + Symbol,
-                                           inconvertibleErrorCode()));
-        return;
-      }
+        else
+          return AddrOrErr.takeError();
+      } else if (auto Err = Sym.takeError())
+        return std::move(Err);
+      else
+        return make_error<StringError>("Symbol not found: " + Symbol,
+                                       inconvertibleErrorCode());
     }
   }
 
-  OnResolved(std::move(Result));
+  return std::move(Result);
 }
 
 /// Performs flags lookup by calling findSymbolInLogicalDylib and

lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

@@ -23,8 +23,6 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MutexGuard.h"
 
-#include <future>
-
 using namespace llvm;
 using namespace llvm::object;
 
@@ -998,8 +996,42 @@ void RuntimeDyldImpl::resolveRelocationList(const RelocationList &Relocs,
   }
 }
 
-void RuntimeDyldImpl::applyExternalSymbolRelocations(
-    const StringMap<JITEvaluatedSymbol> ExternalSymbolMap) {
+Error RuntimeDyldImpl::resolveExternalSymbols() {
+  StringMap<JITEvaluatedSymbol> 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<RelocationList>::iterator i = ExternalSymbolRelocations.begin();
@@ -1061,54 +1093,6 @@ void RuntimeDyldImpl::applyExternalSymbolRelocations(
 
     ExternalSymbolRelocations.erase(i);
   }
-}
-
-Error RuntimeDyldImpl::resolveExternalSymbols() {
-  StringMap<JITEvaluatedSymbol> 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<Expected<JITSymbolResolver::LookupResult>>>();
-      auto NewSymbolsF = NewSymbolsP->get_future();
-      Resolver.lookup(NewSymbols,
-                      [=](Expected<JITSymbolResolver::LookupResult> 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();
 }

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp

@@ -16,7 +16,6 @@
 #include "llvm/MC/MCInst.h"
 #include "llvm/Support/Path.h"
 #include <cctype>
-#include <future>
 #include <memory>
 #include <utility>
 
@@ -730,29 +729,15 @@ bool RuntimeDyldCheckerImpl::checkAllRulesInBuffer(StringRef RulePrefix,
   return DidAllTestsPass && (NumRules != 0);
 }
 
-Expected<JITSymbolResolver::LookupResult> RuntimeDyldCheckerImpl::lookup(
-    const JITSymbolResolver::LookupSet &Symbols) const {
-  auto ResultP = std::make_shared<
-      std::promise<Expected<JITSymbolResolver::LookupResult>>>();
-  auto ResultF = ResultP->get_future();
-
-  getRTDyld().Resolver.lookup(
-      Symbols, [=](Expected<JITSymbolResolver::LookupResult> 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;

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h

@@ -41,9 +41,6 @@ private:
 
   RuntimeDyldImpl &getRTDyld() const { return *RTDyld.Dyld; }
 
-  Expected<JITSymbolResolver::LookupResult>
-  lookup(const JITSymbolResolver::LookupSet &Symbols) const;
-
   bool isSymbolValid(StringRef Symbol) const;
   uint64_t getSymbolLocalAddr(StringRef Symbol) const;
   uint64_t getSymbolRemoteAddr(StringRef Symbol) const;

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h

@@ -433,9 +433,6 @@ protected:
                        const ObjectFile &Obj, ObjSectionToIDMap &ObjSectionToID,
                        StubMap &Stubs) = 0;
 
-  void applyExternalSymbolRelocations(
-      const StringMap<JITEvaluatedSymbol> ExternalSymbolMap);
-
   /// Resolve relocations to external symbols.
   Error resolveExternalSymbols();