[ORC] Merge MaterializationResponsibility notifyEmitted and addDependencies

Removes the MaterializationResponsibility::addDependencies and
addDependenciesForAll methods, and transfers dependency registration to
the notifyEmitted operation. The new dependency registration allows
dependencies to be specified for arbitrary subsets of the
MaterializationResponsibility's symbols (rather than just single symbols
or all symbols) via an array of SymbolDependenceGroups (pairs of symbol
sets and corresponding dependencies for that set).

This patch aims to both improve emission performance and simplify
dependence tracking. By eliminating some states (e.g. symbols having
registered dependencies but not yet being resolved or emitted) we make
some errors impossible by construction, and reduce the number of error
cases that we need to check. NonOwningSymbolStringPtrs are used for
dependence tracking under the session lock, which should reduce
ref-counting operations, and intra-emit dependencies are resolved
outside the session lock, which should provide better performance when
JITing concurrently (since some dependence tracking can happen in
parallel).

The Orc C API is updated to account for this change, with the
LLVMOrcMaterializationResponsibilityNotifyEmitted API being modified and
the LLVMOrcMaterializationResponsibilityAddDependencies and
LLVMOrcMaterializationResponsibilityAddDependenciesForAll operations
being removed.

Author: lhames

llvm/include/llvm-c/Orc.h

@@ -181,6 +181,15 @@ typedef struct {
  */
 typedef LLVMOrcCDependenceMapPair *LLVMOrcCDependenceMapPairs;
 
+/**
+ * A set of symbols that share dependencies.
+ */
+typedef struct {
+  LLVMOrcCSymbolsList Symbols;
+  LLVMOrcCDependenceMapPairs Dependencies;
+  size_t NumDependencies;
+} LLVMOrcCSymbolDependenceGroup;
+
 /**
  * Lookup kind. This can be used by definition generators when deciding whether
  * to produce a definition for a requested symbol.
@@ -808,6 +817,19 @@ LLVMErrorRef LLVMOrcMaterializationResponsibilityNotifyResolved(
  * that all symbols covered by this MaterializationResponsibility instance
  * have been emitted.
  *
+ * This function takes ownership of the symbols in the Dependencies struct.
+ * This allows the following pattern...
+ *
+ *   LLVMOrcSymbolStringPoolEntryRef Names[] = {...};
+ *   LLVMOrcCDependenceMapPair Dependence = {JD, {Names, sizeof(Names)}}
+ *   LLVMOrcMaterializationResponsibilityAddDependencies(JD, Name, &Dependence,
+ * 1);
+ *
+ * ... without requiring cleanup of the elements of the Names array afterwards.
+ *
+ * The client is still responsible for deleting the Dependencies.Names arrays,
+ * and the Dependencies array itself.
+ *
  * This method will return an error if any symbols being resolved have been
  * moved to the error state due to the failure of a dependency. If this
  * method returns an error then clients should log it and call
@@ -817,7 +839,8 @@ LLVMErrorRef LLVMOrcMaterializationResponsibilityNotifyResolved(
  * LLVMErrorSuccess.
  */
 LLVMErrorRef LLVMOrcMaterializationResponsibilityNotifyEmitted(
-    LLVMOrcMaterializationResponsibilityRef MR);
+    LLVMOrcMaterializationResponsibilityRef MR,
+    LLVMOrcCSymbolDependenceGroup *SymbolDepGroups, size_t NumSymbolDepGroups);
 
 /**
  * Attempt to claim responsibility for new definitions. This method can be
@@ -870,38 +893,6 @@ LLVMErrorRef LLVMOrcMaterializationResponsibilityDelegate(
     LLVMOrcSymbolStringPoolEntryRef *Symbols, size_t NumSymbols,
     LLVMOrcMaterializationResponsibilityRef *Result);
 
-/**
- * Adds dependencies to a symbol that the MaterializationResponsibility is
- * responsible for.
- *
- * This function takes ownership of Dependencies struct. The Names
- * array have been retained for this function. This allows the following
- * pattern...
- *
- *   LLVMOrcSymbolStringPoolEntryRef Names[] = {...};
- *   LLVMOrcCDependenceMapPair Dependence = {JD, {Names, sizeof(Names)}}
- *   LLVMOrcMaterializationResponsibilityAddDependencies(JD, Name, &Dependence,
- * 1);
- *
- * ... without requiring cleanup of the elements of the Names array afterwards.
- *
- * The client is still responsible for deleting the Dependencies.Names array
- * itself.
- */
-void LLVMOrcMaterializationResponsibilityAddDependencies(
-    LLVMOrcMaterializationResponsibilityRef MR,
-    LLVMOrcSymbolStringPoolEntryRef Name,
-    LLVMOrcCDependenceMapPairs Dependencies, size_t NumPairs);
-
-/**
- * Adds dependencies to all symbols that the MaterializationResponsibility is
- * responsible for. See LLVMOrcMaterializationResponsibilityAddDependencies for
- * notes about memory responsibility.
- */
-void LLVMOrcMaterializationResponsibilityAddDependenciesForAll(
-    LLVMOrcMaterializationResponsibilityRef MR,
-    LLVMOrcCDependenceMapPairs Dependencies, size_t NumPairs);
-
 /**
  * Create a "bare" JITDylib.
  *

llvm/include/llvm/ExecutionEngine/Orc/Core.h

@@ -436,6 +436,27 @@ class FailedToMaterialize : public ErrorInfo<FailedToMaterialize> {
   std::shared_ptr<SymbolDependenceMap> Symbols;
 };
 
+/// Used to report failure due to unsatisfiable symbol dependencies.
+class UnsatisfiedSymbolDependencies
+    : public ErrorInfo<UnsatisfiedSymbolDependencies> {
+public:
+  static char ID;
+
+  UnsatisfiedSymbolDependencies(std::shared_ptr<SymbolStringPool> SSP,
+                                JITDylibSP JD, SymbolNameSet FailedSymbols,
+                                SymbolDependenceMap BadDeps,
+                                std::string Explanation);
+  std::error_code convertToErrorCode() const override;
+  void log(raw_ostream &OS) const override;
+
+private:
+  std::shared_ptr<SymbolStringPool> SSP;
+  JITDylibSP JD;
+  SymbolNameSet FailedSymbols;
+  SymbolDependenceMap BadDeps;
+  std::string Explanation;
+};
+
 /// Used to notify clients when symbols can not be found during a lookup.
 class SymbolsNotFound : public ErrorInfo<SymbolsNotFound> {
 public:
@@ -517,6 +538,13 @@ class UnexpectedSymbolDefinitions : public ErrorInfo<UnexpectedSymbolDefinitions
   SymbolNameVector Symbols;
 };
 
+/// A set of symbols and the their dependencies. Used to describe dependencies
+/// for the MaterializationResponsibility::notifyEmitted operation.
+struct SymbolDependenceGroup {
+  SymbolNameSet Symbols;
+  SymbolDependenceMap Dependencies;
+};
+
 /// Tracks responsibility for materialization, and mediates interactions between
 /// MaterializationUnits and JDs.
 ///
@@ -587,13 +615,22 @@ class MaterializationResponsibility {
   /// that all symbols covered by this MaterializationResponsibility instance
   /// have been emitted.
   ///
+  /// The DepGroups array describes the dependencies of symbols being emitted on
+  /// symbols that are outside this MaterializationResponsibility object. Each
+  /// group consists of a pair of a set of symbols and a SymbolDependenceMap
+  /// that describes the dependencies for the symbols in the first set. The
+  /// elements of DepGroups must be non-overlapping (no symbol should appear in
+  /// more than one of hte symbol sets), but do not have to be exhaustive. Any
+  /// symbol in this MaterializationResponsibility object that is not covered
+  /// by an entry will be treated as having no dependencies.
+  ///
   /// This method will return an error if any symbols being resolved have been
   /// moved to the error state due to the failure of a dependency. If this
   /// method returns an error then clients should log it and call
   /// failMaterialize. If no dependencies have been registered for the
   /// symbols covered by this MaterializationResponsibility then this method
   /// is guaranteed to return Error::success() and can be wrapped with cantFail.
-  Error notifyEmitted();
+  Error notifyEmitted(ArrayRef<SymbolDependenceGroup> DepGroups);
 
   /// Attempt to claim responsibility for new definitions. This method can be
   /// used to claim responsibility for symbols that are added to a
@@ -628,12 +665,6 @@ class MaterializationResponsibility {
   Expected<std::unique_ptr<MaterializationResponsibility>>
   delegate(const SymbolNameSet &Symbols);
 
-  void addDependencies(const SymbolStringPtr &Name,
-                       const SymbolDependenceMap &Dependencies);
-
-  /// Add dependencies that apply to all symbols covered by this instance.
-  void addDependenciesForAll(const SymbolDependenceMap &Dependencies);
-
 private:
   /// Create a MaterializationResponsibility for the given JITDylib and
   ///        initial symbols.
@@ -1185,9 +1216,29 @@ class JITDylib : public ThreadSafeRefCountedBase<JITDylib>,
   using UnmaterializedInfosList =
       std::vector<std::shared_ptr<UnmaterializedInfo>>;
 
+  struct EmissionDepUnit {
+    EmissionDepUnit(JITDylib &JD) : JD(&JD) {}
+
+    JITDylib *JD = nullptr;
+    DenseMap<NonOwningSymbolStringPtr, JITSymbolFlags> Symbols;
+    DenseMap<JITDylib *, DenseSet<NonOwningSymbolStringPtr>> Dependencies;
+  };
+
+  struct EmissionDepUnitInfo {
+    std::shared_ptr<EmissionDepUnit> EDU;
+    DenseSet<EmissionDepUnit *> IntraEmitUsers;
+    DenseMap<JITDylib *, DenseSet<NonOwningSymbolStringPtr>> NewDeps;
+  };
+
+  // Information about not-yet-ready symbol.
+  // * DefiningEDU will point to the EmissionDepUnit that defines the symbol.
+  // * DependantEDUs will hold pointers to any EmissionDepUnits currently
+  //   waiting on this symbol.
+  // * Pending queries holds any not-yet-completed queries that include this
+  //   symbol.
   struct MaterializingInfo {
-    SymbolDependenceMap Dependants;
-    SymbolDependenceMap UnemittedDependencies;
+    std::shared_ptr<EmissionDepUnit> DefiningEDU;
+    DenseSet<EmissionDepUnit *> DependantEDUs;
 
     void addQuery(std::shared_ptr<AsynchronousSymbolQuery> Q);
     void removeQuery(const AsynchronousSymbolQuery &Q);
@@ -1278,17 +1329,8 @@ class JITDylib : public ThreadSafeRefCountedBase<JITDylib>,
 
   Error resolve(MaterializationResponsibility &MR, const SymbolMap &Resolved);
 
-  Error emit(MaterializationResponsibility &MR, const SymbolFlagsMap &Emitted);
-
   void unlinkMaterializationResponsibility(MaterializationResponsibility &MR);
 
-  using FailedSymbolsWorklist =
-      std::vector<std::pair<JITDylib *, SymbolStringPtr>>;
-
-  static std::pair<AsynchronousSymbolQuerySet,
-                   std::shared_ptr<SymbolDependenceMap>>
-      failSymbols(FailedSymbolsWorklist);
-
   ExecutionSession &ES;
   enum { Open, Closing, Closed } State = Open;
   std::mutex GeneratorsMutex;
@@ -1767,19 +1809,45 @@ class ExecutionSession {
   SymbolNameSet OL_getRequestedSymbols(const MaterializationResponsibility &MR);
   Error OL_notifyResolved(MaterializationResponsibility &MR,
                           const SymbolMap &Symbols);
-  Error OL_notifyEmitted(MaterializationResponsibility &MR);
+
+  using EDUInfosMap =
+      DenseMap<JITDylib::EmissionDepUnit *, JITDylib::EmissionDepUnitInfo>;
+
+  template <typename HandleNewDepFn>
+  void propagateExtraEmitDeps(std::deque<JITDylib::EmissionDepUnit *> Worklist,
+                              EDUInfosMap &EDUInfos,
+                              HandleNewDepFn HandleNewDep);
+  EDUInfosMap simplifyDepGroups(MaterializationResponsibility &MR,
+                                ArrayRef<SymbolDependenceGroup> EmittedDeps);
+  void IL_makeEDUReady(std::shared_ptr<JITDylib::EmissionDepUnit> EDU,
+                       JITDylib::AsynchronousSymbolQuerySet &Queries);
+  void IL_makeEDUEmitted(std::shared_ptr<JITDylib::EmissionDepUnit> EDU,
+                         JITDylib::AsynchronousSymbolQuerySet &Queries);
+  bool IL_removeEDUDependence(JITDylib::EmissionDepUnit &EDU, JITDylib &DepJD,
+                              NonOwningSymbolStringPtr DepSym,
+                              EDUInfosMap &EDUInfos);
+
+  static Error makeJDClosedError(JITDylib::EmissionDepUnit &EDU,
+                                 JITDylib &ClosedJD);
+  static Error makeUnsatisfiedDepsError(JITDylib::EmissionDepUnit &EDU,
+                                        JITDylib &BadJD, SymbolNameSet BadDeps);
+
+  Expected<JITDylib::AsynchronousSymbolQuerySet>
+  IL_emit(MaterializationResponsibility &MR, EDUInfosMap EDUInfos);
+  Error OL_notifyEmitted(MaterializationResponsibility &MR,
+                         ArrayRef<SymbolDependenceGroup> EmittedDeps);
+
   Error OL_defineMaterializing(MaterializationResponsibility &MR,
                                SymbolFlagsMap SymbolFlags);
+
+  std::pair<JITDylib::AsynchronousSymbolQuerySet,
+            std::shared_ptr<SymbolDependenceMap>>
+  IL_failSymbols(JITDylib &JD, const SymbolNameVector &SymbolsToFail);
   void OL_notifyFailed(MaterializationResponsibility &MR);
   Error OL_replace(MaterializationResponsibility &MR,
                    std::unique_ptr<MaterializationUnit> MU);
   Expected<std::unique_ptr<MaterializationResponsibility>>
   OL_delegate(MaterializationResponsibility &MR, const SymbolNameSet &Symbols);
-  void OL_addDependencies(MaterializationResponsibility &MR,
-                          const SymbolStringPtr &Name,
-                          const SymbolDependenceMap &Dependencies);
-  void OL_addDependenciesForAll(MaterializationResponsibility &MR,
-                                const SymbolDependenceMap &Dependencies);
 
 #ifndef NDEBUG
   void dumpDispatchInfo(Task &T);
@@ -1965,8 +2033,9 @@ inline Error MaterializationResponsibility::notifyResolved(
   return getExecutionSession().OL_notifyResolved(*this, Symbols);
 }
 
-inline Error MaterializationResponsibility::notifyEmitted() {
-  return getExecutionSession().OL_notifyEmitted(*this);
+inline Error MaterializationResponsibility::notifyEmitted(
+    ArrayRef<SymbolDependenceGroup> EmittedDeps) {
+  return getExecutionSession().OL_notifyEmitted(*this, EmittedDeps);
 }
 
 inline Error MaterializationResponsibility::defineMaterializing(
@@ -1989,16 +2058,6 @@ MaterializationResponsibility::delegate(const SymbolNameSet &Symbols) {
   return getExecutionSession().OL_delegate(*this, Symbols);
 }
 
-inline void MaterializationResponsibility::addDependencies(
-    const SymbolStringPtr &Name, const SymbolDependenceMap &Dependencies) {
-  getExecutionSession().OL_addDependencies(*this, Name, Dependencies);
-}
-
-inline void MaterializationResponsibility::addDependenciesForAll(
-    const SymbolDependenceMap &Dependencies) {
-  getExecutionSession().OL_addDependenciesForAll(*this, Dependencies);
-}
-
 } // End namespace orc
 } // End namespace llvm
 

llvm/include/llvm/ExecutionEngine/Orc/DebugUtils.h

@@ -32,6 +32,9 @@ namespace orc {
 /// Render a SymbolStringPtr.
 raw_ostream &operator<<(raw_ostream &OS, const SymbolStringPtr &Sym);
 
+/// Render a NonOwningSymbolStringPtr.
+raw_ostream &operator<<(raw_ostream &OS, NonOwningSymbolStringPtr Sym);
+
 /// Render a SymbolNameSet.
 raw_ostream &operator<<(raw_ostream &OS, const SymbolNameSet &Symbols);
 

llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h

@@ -137,7 +137,7 @@ class RTDyldObjectLinkingLayer
                  object::OwningBinary<object::ObjectFile> O,
                  std::unique_ptr<RuntimeDyld::MemoryManager> MemMgr,
                  std::unique_ptr<RuntimeDyld::LoadedObjectInfo> LoadedObjInfo,
-                 Error Err);
+                 std::unique_ptr<SymbolDependenceMap> Deps, Error Err);
 
   Error handleRemoveResources(JITDylib &JD, ResourceKey K) override;
   void handleTransferResources(JITDylib &JD, ResourceKey DstKey,