[ORC] Move DefinitionGenerator out of JITDylib.

This will make it easier to implement asynchronous definition generators.
davezarzycki · Oct 19, 2020 · 5d2e359 · 5d2e359
1 parent 680845e
commit 5d2e359
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 27 deletions.
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -829,6 +829,23 @@ class AsynchronousSymbolQuery {
   SymbolState RequiredState;
 };
 
+/// Definition generators can be attached to JITDylibs to generate new
+/// definitions for otherwise unresolved symbols during lookup.
+class DefinitionGenerator {
+public:
+  virtual ~DefinitionGenerator();
+
+  /// DefinitionGenerators should override this method to insert new
+  /// definitions into the parent JITDylib. K specifies the kind of this
+  /// lookup. JD specifies the target JITDylib being searched, and
+  /// JDLookupFlags specifies whether the search should match against
+  /// hidden symbols. Finally, Symbols describes the set of unresolved
+  /// symbols and their associated lookup flags.
+  virtual Error tryToGenerate(LookupKind K, JITDylib &JD,
+                              JITDylibLookupFlags JDLookupFlags,
+                              const SymbolLookupSet &LookupSet) = 0;
+};
+
 /// A symbol table that supports asynchoronous symbol queries.
 ///
 /// Represents a virtual shared object. Instances can not be copied or moved, so
@@ -841,22 +858,6 @@ class JITDylib : public ThreadSafeRefCountedBase<JITDylib> {
   friend class Platform;
   friend class MaterializationResponsibility;
 public:
-  /// Definition generators can be attached to JITDylibs to generate new
-  /// definitions for otherwise unresolved symbols during lookup.
-  class DefinitionGenerator {
-  public:
-    virtual ~DefinitionGenerator();
-
-    /// DefinitionGenerators should override this method to insert new
-    /// definitions into the parent JITDylib. K specifies the kind of this
-    /// lookup. JD specifies the target JITDylib being searched, and
-    /// JDLookupFlags specifies whether the search should match against
-    /// hidden symbols. Finally, Symbols describes the set of unresolved
-    /// symbols and their associated lookup flags.
-    virtual Error tryToGenerate(LookupKind K, JITDylib &JD,
-                                JITDylibLookupFlags JDLookupFlags,
-                                const SymbolLookupSet &LookupSet) = 0;
-  };
 
   using AsynchronousSymbolQuerySet =
     std::set<std::shared_ptr<AsynchronousSymbolQuery>>;
@@ -1534,7 +1535,7 @@ Error JITDylib::define(std::unique_ptr<MaterializationUnitType> &MU,
 
 /// ReexportsGenerator can be used with JITDylib::addGenerator to automatically
 /// re-export a subset of the source JITDylib's symbols in the target.
-class ReexportsGenerator : public JITDylib::DefinitionGenerator {
+class ReexportsGenerator : public DefinitionGenerator {
 public:
   using SymbolPredicate = std::function<bool(SymbolStringPtr)>;
 

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h b/llvm/include/llvm/ExecutionEngine/Orc/ExecutionUtils.h
@@ -226,7 +226,7 @@ class ItaniumCXAAtExitSupport {
 /// If an instance of this class is attached to a JITDylib as a fallback
 /// definition generator, then any symbol found in the given DynamicLibrary that
 /// passes the 'Allow' predicate will be added to the JITDylib.
-class DynamicLibrarySearchGenerator : public JITDylib::DefinitionGenerator {
+class DynamicLibrarySearchGenerator : public DefinitionGenerator {
 public:
   using SymbolPredicate = std::function<bool(const SymbolStringPtr &)>;
 
@@ -269,7 +269,7 @@ class DynamicLibrarySearchGenerator : public JITDylib::DefinitionGenerator {
 /// If an instance of this class is attached to a JITDylib as a fallback
 /// definition generator, then any symbol found in the archive will result in
 /// the containing object being added to the JITDylib.
-class StaticLibraryDefinitionGenerator : public JITDylib::DefinitionGenerator {
+class StaticLibraryDefinitionGenerator : public DefinitionGenerator {
 public:
   /// Try to create a StaticLibraryDefinitionGenerator from the given path.
   ///

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h b/llvm/include/llvm/ExecutionEngine/Orc/TPCDynamicLibrarySearchGenerator.h
@@ -20,7 +20,7 @@
 namespace llvm {
 namespace orc {
 
-class TPCDynamicLibrarySearchGenerator : public JITDylib::DefinitionGenerator {
+class TPCDynamicLibrarySearchGenerator : public DefinitionGenerator {
 public:
   using SymbolPredicate = unique_function<bool(const SymbolStringPtr &)>;
 

diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -496,7 +496,7 @@ Error ReexportsGenerator::tryToGenerate(LookupKind K, JITDylib &JD,
   return JD.define(reexports(SourceJD, AliasMap, SourceJDLookupFlags));
 }
 
-JITDylib::DefinitionGenerator::~DefinitionGenerator() {}
+DefinitionGenerator::~DefinitionGenerator() {}
 
 Error JITDylib::clear() {
   std::vector<ResourceTrackerSP> TrackersToRemove;
@@ -1299,6 +1299,10 @@ Error JITDylib::lodgeQuery(UnmaterializedInfosList &UMIs,
     if (Unresolved.empty())
       break;
 
+    // FIXME: The generator should only be run for symbols that satisfy
+    // the JDLookupFlags. E.g. if we failed to match a hidden symbol we
+    // shouldn't try to generate it.
+
     // Run the generator.
     if (auto Err = DG->tryToGenerate(K, *this, JDLookupFlags, Unresolved))
       return Err;

diff --git a/llvm/lib/ExecutionEngine/Orc/OrcV2CBindings.cpp b/llvm/lib/ExecutionEngine/Orc/OrcV2CBindings.cpp
@@ -46,7 +46,7 @@ DEFINE_SIMPLE_CONVERSION_FUNCTIONS(ExecutionSession, LLVMOrcExecutionSessionRef)
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(OrcV2CAPIHelper::PoolEntry,
                                    LLVMOrcSymbolStringPoolEntryRef)
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(JITDylib, LLVMOrcJITDylibRef)
-DEFINE_SIMPLE_CONVERSION_FUNCTIONS(JITDylib::DefinitionGenerator,
+DEFINE_SIMPLE_CONVERSION_FUNCTIONS(DefinitionGenerator,
                                    LLVMOrcJITDylibDefinitionGeneratorRef)
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(ThreadSafeContext,
                                    LLVMOrcThreadSafeContextRef)
@@ -98,8 +98,7 @@ void LLVMOrcDisposeJITDylibDefinitionGenerator(
 
 void LLVMOrcJITDylibAddGenerator(LLVMOrcJITDylibRef JD,
                                  LLVMOrcJITDylibDefinitionGeneratorRef DG) {
-  unwrap(JD)->addGenerator(
-      std::unique_ptr<JITDylib::DefinitionGenerator>(unwrap(DG)));
+  unwrap(JD)->addGenerator(std::unique_ptr<DefinitionGenerator>(unwrap(DG)));
 }
 
 LLVMErrorRef LLVMOrcCreateDynamicLibrarySearchGeneratorForProcess(

diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink.cpp
@@ -561,7 +561,7 @@ Error LLVMJITLinkObjectLinkingLayer::add(ResourceTrackerSP RT,
   return JD.define(std::move(MU), std::move(RT));
 }
 
-class PhonyExternalsGenerator : public JITDylib::DefinitionGenerator {
+class PhonyExternalsGenerator : public DefinitionGenerator {
 public:
   Error tryToGenerate(LookupKind K, JITDylib &JD,
                       JITDylibLookupFlags JDLookupFlags,

diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -289,7 +289,7 @@ TEST_F(CoreAPIsStandardTest, LookupFlagsTest) {
 
 TEST_F(CoreAPIsStandardTest, LookupWithGeneratorFailure) {
 
-  class BadGenerator : public JITDylib::DefinitionGenerator {
+  class BadGenerator : public DefinitionGenerator {
   public:
     Error tryToGenerate(LookupKind K, JITDylib &, JITDylibLookupFlags,
                         const SymbolLookupSet &) override {
@@ -1047,7 +1047,7 @@ TEST_F(CoreAPIsStandardTest, DefineMaterializingSymbol) {
 TEST_F(CoreAPIsStandardTest, GeneratorTest) {
   cantFail(JD.define(absoluteSymbols({{Foo, FooSym}})));
 
-  class TestGenerator : public JITDylib::DefinitionGenerator {
+  class TestGenerator : public DefinitionGenerator {
   public:
     TestGenerator(SymbolMap Symbols) : Symbols(std::move(Symbols)) {}
     Error tryToGenerate(LookupKind K, JITDylib &JD,