[llvm] r342939 - [ORC] Switch to asynchronous resolution in JITSymbolResolver.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 24 21:43:38 PDT 2018


Author: lhames
Date: Mon Sep 24 21:43:38 2018
New Revision: 342939

URL: http://llvm.org/viewvc/llvm-project?rev=342939&view=rev
Log:
[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.

Modified:
    llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h
    llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
    llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h
    llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h

Modified: llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h Mon Sep 24 21:43:38 2018
@@ -333,6 +333,7 @@ class JITSymbolResolver {
 public:
   using LookupSet = std::set<StringRef>;
   using LookupResult = std::map<StringRef, JITEvaluatedSymbol>;
+  using OnResolvedFunction = std::function<void(Expected<LookupResult>)>;
 
   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<LookupResult> 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<LookupResult> 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.

Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h Mon Sep 24 21:43:38 2018
@@ -376,6 +376,7 @@ private:
 class AsynchronousSymbolQuery {
   friend class ExecutionSession;
   friend class JITDylib;
+  friend class JITSymbolResolverAdapter;
 
 public:
 

Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h Mon Sep 24 21:43:38 2018
@@ -90,12 +90,13 @@ createSymbolResolver(GetResponsibilitySe
       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;
-  Expected<LookupResult> lookup(const LookupSet &Symbols) override;
+  void lookup(const LookupSet &Symbols, OnResolvedFunction OnResolved) override;
 
 private:
   ExecutionSession &ES;

Modified: llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp Mon Sep 24 21:43:38 2018
@@ -18,34 +18,34 @@ JITSymbolResolverAdapter::JITSymbolResol
     ExecutionSession &ES, SymbolResolver &R, MaterializationResponsibility *MR)
     : ES(ES), R(R), MR(MR) {}
 
-Expected<JITSymbolResolverAdapter::LookupResult>
-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<AsynchronousSymbolQuery> Q,
-                            SymbolNameSet Unresolved) {
-    return R.lookup(std::move(Q), std::move(Unresolved));
+  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 RegisterDependencies = [&](const SymbolDependenceMap &Deps) {
-    if (MR)
-      MR->addDependenciesForAll(Deps);
-  };
+  auto Q = std::make_shared<AsynchronousSymbolQuery>(
+      InternedSymbols, OnResolvedWithUnwrap,
+      [this](Error Err) { ES.reportError(std::move(Err)); });
 
-  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;
+  auto Unresolved = R.lookup(Q, InternedSymbols);
+  if (Unresolved.empty()) {
+    if (MR)
+      MR->addDependenciesForAll(Q->QueryRegistrations);
+  } else
+    ES.legacyFailQuery(*Q, make_error<SymbolsNotFound>(std::move(Unresolved)));
 }
 
 Expected<JITSymbolResolverAdapter::LookupSet>

Modified: llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp Mon Sep 24 21:43:38 2018
@@ -18,30 +18,42 @@ class JITDylibSearchOrderResolver : publ
 public:
   JITDylibSearchOrderResolver(MaterializationResponsibility &MR) : MR(MR) {}
 
-  Expected<LookupResult> 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<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);
     };
 
-    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<LookupSet> getResponsibilitySet(const LookupSet &Symbols) {

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp Mon Sep 24 21:43:38 2018
@@ -62,34 +62,42 @@ llvm::ARMJITSymbolFlags::fromObjectSymbo
 /// Performs lookup by, for each symbol, first calling
 ///        findSymbolInLogicalDylib and if that fails calling
 ///        findSymbol.
-Expected<JITSymbolResolver::LookupResult>
-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<StringError>("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<StringError>("Symbol not found: " + Symbol,
+                                           inconvertibleErrorCode()));
+        return;
+      }
     }
   }
 
-  return std::move(Result);
+  OnResolved(std::move(Result));
 }
 
 /// Performs flags lookup by calling findSymbolInLogicalDylib and

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Mon Sep 24 21:43:38 2018
@@ -23,6 +23,8 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MutexGuard.h"
 
+#include <future>
+
 using namespace llvm;
 using namespace llvm::object;
 
@@ -996,42 +998,8 @@ void RuntimeDyldImpl::resolveRelocationL
   }
 }
 
-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);
-      }
-    }
-  }
-
+void RuntimeDyldImpl::applyExternalSymbolRelocations(
+    const StringMap<JITEvaluatedSymbol> ExternalSymbolMap) {
   while (!ExternalSymbolRelocations.empty()) {
 
     StringMap<RelocationList>::iterator i = ExternalSymbolRelocations.begin();
@@ -1093,6 +1061,54 @@ Error RuntimeDyldImpl::resolveExternalSy
 
     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();
 }

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp Mon Sep 24 21:43:38 2018
@@ -16,6 +16,7 @@
 #include "llvm/MC/MCInst.h"
 #include "llvm/Support/Path.h"
 #include <cctype>
+#include <future>
 #include <memory>
 #include <utility>
 
@@ -729,15 +730,29 @@ bool RuntimeDyldCheckerImpl::checkAllRul
   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;
-  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::getSymb
   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;

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCheckerImpl.h Mon Sep 24 21:43:38 2018
@@ -41,6 +41,9 @@ 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;

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h?rev=342939&r1=342938&r2=342939&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h Mon Sep 24 21:43:38 2018
@@ -433,6 +433,9 @@ protected:
                        const ObjectFile &Obj, ObjSectionToIDMap &ObjSectionToID,
                        StubMap &Stubs) = 0;
 
+  void applyExternalSymbolRelocations(
+      const StringMap<JITEvaluatedSymbol> ExternalSymbolMap);
+
   /// Resolve relocations to external symbols.
   Error resolveExternalSymbols();
 




More information about the llvm-commits mailing list