Revert r345077 "[ORC] Change how non-exported symbols are matched during lookup."

Doesn't build on Windows. The call to 'lookup' is ambiguous. Clang and
MSVC agree, anyway.

http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/787
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\unittests\ExecutionEngine\Orc\CoreAPIsTest.cpp(315): error C2668: 'llvm::orc::ExecutionSession::lookup': ambiguous call to overloaded function
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\include\llvm/ExecutionEngine/Orc/Core.h(823): note: could be 'llvm::Expected<llvm::JITEvaluatedSymbol> llvm::orc::ExecutionSession::lookup(llvm::ArrayRef<llvm::orc::JITDylib *>,llvm::orc::SymbolStringPtr)'
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\include\llvm/ExecutionEngine/Orc/Core.h(817): note: or       'llvm::Expected<llvm::JITEvaluatedSymbol> llvm::orc::ExecutionSession::lookup(const llvm::orc::JITDylibSearchList &,llvm::orc::SymbolStringPtr)'
C:\b\slave\clang-x64-windows-msvc\build\llvm.src\unittests\ExecutionEngine\Orc\CoreAPIsTest.cpp(315): note: while trying to match the argument list '(initializer list, llvm::orc::SymbolStringPtr)'

llvm-svn: 345078

Author: rnk

llvm/include/llvm/ExecutionEngine/JITSymbol.h

@@ -40,18 +40,6 @@ class SymbolRef;
 /// Represents an address in the target process's address space.
 using JITTargetAddress = uint64_t;
 
-/// Convert a JITTargetAddress to a pointer.
-template <typename T> T jitTargetAddressToPointer(JITTargetAddress Addr) {
-  static_assert(std::is_pointer<T>::value, "T must be a pointer type");
-  uintptr_t IntPtr = static_cast<uintptr_t>(Addr);
-  assert(IntPtr == Addr && "JITTargetAddress value out of range for uintptr_t");
-  return reinterpret_cast<T>(IntPtr);
-}
-
-template <typename T> JITTargetAddress pointerToJITTargetAddress(T *Ptr) {
-  return static_cast<JITTargetAddress>(reinterpret_cast<uintptr_t>(Ptr));
-}
-
 /// Flags for symbols in the JIT.
 class JITSymbolFlags {
 public:

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

@@ -54,8 +54,8 @@ using SymbolFlagsMap = DenseMap<SymbolStringPtr, JITSymbolFlags>;
 ///        symbols to be obtained for logging.
 using SymbolDependenceMap = DenseMap<JITDylib *, SymbolNameSet>;
 
-/// A list of (JITDylib*, bool) pairs.
-using JITDylibSearchList = std::vector<std::pair<JITDylib *, bool>>;
+/// A list of JITDylib pointers.
+using JITDylibList = std::vector<JITDylib *>;
 
 /// Render a SymbolStringPtr.
 raw_ostream &operator<<(raw_ostream &OS, const SymbolStringPtr &Sym);
@@ -85,8 +85,8 @@ raw_ostream &operator<<(raw_ostream &OS, const SymbolDependenceMap &Deps);
 /// Render a MaterializationUnit.
 raw_ostream &operator<<(raw_ostream &OS, const MaterializationUnit &MU);
 
-/// Render a JITDylibSearchList.
-raw_ostream &operator<<(raw_ostream &OS, const JITDylibSearchList &JDs);
+/// Render a JITDylibList.
+raw_ostream &operator<<(raw_ostream &OS, const JITDylibList &JDs);
 
 /// Callback to notify client that symbols have been resolved.
 using SymbolsResolvedCallback = std::function<void(Expected<SymbolMap>)>;
@@ -351,15 +351,14 @@ using SymbolAliasMap = DenseMap<SymbolStringPtr, SymbolAliasMapEntry>;
 class ReExportsMaterializationUnit : public MaterializationUnit {
 public:
   /// SourceJD is allowed to be nullptr, in which case the source JITDylib is
-  /// taken to be whatever JITDylib these definitions are materialized in (and
-  /// MatchNonExported has no effect). This is useful for defining aliases
-  /// within a JITDylib.
+  /// taken to be whatever JITDylib these definitions are materialized in. This
+  /// is useful for defining aliases within a JITDylib.
   ///
   /// Note: Care must be taken that no sets of aliases form a cycle, as such
   ///       a cycle will result in a deadlock when any symbol in the cycle is
   ///       resolved.
-  ReExportsMaterializationUnit(JITDylib *SourceJD, bool MatchNonExported,
-                               SymbolAliasMap Aliases, VModuleKey K);
+  ReExportsMaterializationUnit(JITDylib *SourceJD, SymbolAliasMap Aliases,
+                               VModuleKey K);
 
   StringRef getName() const override;
 
@@ -369,7 +368,6 @@ class ReExportsMaterializationUnit : public MaterializationUnit {
   static SymbolFlagsMap extractFlags(const SymbolAliasMap &Aliases);
 
   JITDylib *SourceJD = nullptr;
-  bool MatchNonExported = false;
   SymbolAliasMap Aliases;
 };
 
@@ -387,19 +385,16 @@ class ReExportsMaterializationUnit : public MaterializationUnit {
 inline std::unique_ptr<ReExportsMaterializationUnit>
 symbolAliases(SymbolAliasMap Aliases, VModuleKey K = VModuleKey()) {
   return llvm::make_unique<ReExportsMaterializationUnit>(
-      nullptr, true, std::move(Aliases), std::move(K));
+      nullptr, std::move(Aliases), std::move(K));
 }
 
 /// Create a materialization unit for re-exporting symbols from another JITDylib
 /// with alternative names/flags.
-/// If MatchNonExported is true then non-exported symbols from SourceJD can be
-/// re-exported. If it is false, attempts to re-export a non-exported symbol
-/// will result in a "symbol not found" error.
 inline std::unique_ptr<ReExportsMaterializationUnit>
 reexports(JITDylib &SourceJD, SymbolAliasMap Aliases,
-          bool MatchNonExported = false, VModuleKey K = VModuleKey()) {
+          VModuleKey K = VModuleKey()) {
   return llvm::make_unique<ReExportsMaterializationUnit>(
-      &SourceJD, MatchNonExported, std::move(Aliases), std::move(K));
+      &SourceJD, std::move(Aliases), std::move(K));
 }
 
 /// Build a SymbolAliasMap for the common case where you want to re-export
@@ -416,14 +411,13 @@ class ReexportsGenerator {
   /// Create a reexports generator. If an Allow predicate is passed, only
   /// symbols for which the predicate returns true will be reexported. If no
   /// Allow predicate is passed, all symbols will be exported.
-  ReexportsGenerator(JITDylib &SourceJD, bool MatchNonExported = false,
+  ReexportsGenerator(JITDylib &SourceJD,
                      SymbolPredicate Allow = SymbolPredicate());
 
   SymbolNameSet operator()(JITDylib &JD, const SymbolNameSet &Names);
 
 private:
   JITDylib &SourceJD;
-  bool MatchNonExported = false;
   SymbolPredicate Allow;
 };
 
@@ -542,18 +536,16 @@ class JITDylib {
   /// as the first in the search order (instead of this dylib) ensures that
   /// definitions within this dylib resolve to the lazy-compiling stubs,
   /// rather than immediately materializing the definitions in this dylib.
-  void setSearchOrder(JITDylibSearchList NewSearchOrder,
-                      bool SearchThisJITDylibFirst = true,
-                      bool MatchNonExportedInThisDylib = true);
+  void setSearchOrder(JITDylibList NewSearchOrder,
+                      bool SearchThisJITDylibFirst = true);
 
   /// Add the given JITDylib to the search order for definitions in this
   /// JITDylib.
-  void addToSearchOrder(JITDylib &JD, bool MatcNonExported = false);
+  void addToSearchOrder(JITDylib &JD);
 
   /// Replace OldJD with NewJD in the search order if OldJD is present.
   /// Otherwise this operation is a no-op.
-  void replaceInSearchOrder(JITDylib &OldJD, JITDylib &NewJD,
-                            bool MatchNonExported = false);
+  void replaceInSearchOrder(JITDylib &OldJD, JITDylib &NewJD);
 
   /// Remove the given JITDylib from the search order for this JITDylib if it is
   /// present. Otherwise this operation is a no-op.
@@ -562,7 +554,7 @@ class JITDylib {
   /// Do something with the search order (run under the session lock).
   template <typename Func>
   auto withSearchOrderDo(Func &&F)
-      -> decltype(F(std::declval<const JITDylibSearchList &>()));
+      -> decltype(F(std::declval<const JITDylibList &>()));
 
   /// Define all symbols provided by the materialization unit to be part of this
   /// JITDylib.
@@ -650,12 +642,12 @@ class JITDylib {
                                 const SymbolNameSet &Names);
 
   void lodgeQuery(std::shared_ptr<AsynchronousSymbolQuery> &Q,
-                  SymbolNameSet &Unresolved, bool MatchNonExported,
-                  MaterializationUnitList &MUs);
+                  SymbolNameSet &Unresolved, JITDylib *MatchNonExportedInJD,
+                  bool MatchNonExported, MaterializationUnitList &MUs);
 
   void lodgeQueryImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
-                      SymbolNameSet &Unresolved, bool MatchNonExported,
-                      MaterializationUnitList &MUs);
+                      SymbolNameSet &Unresolved, JITDylib *MatchNonExportedInJD,
+                      bool MatchNonExported, MaterializationUnitList &MUs);
 
   LookupImplActionFlags
   lookupImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
@@ -690,7 +682,7 @@ class JITDylib {
   UnmaterializedInfosMap UnmaterializedInfos;
   MaterializingInfosMap MaterializingInfos;
   GeneratorFunction DefGenerator;
-  JITDylibSearchList SearchOrder;
+  JITDylibList SearchOrder;
 };
 
 /// An ExecutionSession represents a running JIT program.
@@ -774,10 +766,6 @@ class ExecutionSession {
 
   /// Search the given JITDylib list for the given symbols.
   ///
-  /// SearchOrder lists the JITDylibs to search. For each dylib, the associated
-  /// boolean indicates whether the search should match against non-exported
-  /// (hidden visibility) symbols in that dylib (true means match against
-  /// non-exported symbols, false means do not match).
   ///
   /// The OnResolve callback will be called once all requested symbols are
   /// resolved, or if an error occurs prior to resolution.
@@ -794,9 +782,19 @@ class ExecutionSession {
   /// dependenant symbols for this query (e.g. it is being made by a top level
   /// client to get an address to call) then the value NoDependenciesToRegister
   /// can be used.
-  void lookup(const JITDylibSearchList &SearchOrder, SymbolNameSet Symbols,
+  ///
+  /// If the MatchNonExportedInJD pointer is non-null, then the lookup will find
+  /// non-exported symbols defined in the JITDylib pointed to by
+  /// MatchNonExportedInJD.
+  /// If MatchNonExported is true the lookup will find non-exported symbols in
+  /// any JITDylib (setting MatchNonExportedInJD is redundant in such cases).
+  /// If MatchNonExported is false and MatchNonExportedInJD is null,
+  /// non-exported symbols will never be found.
+  void lookup(const JITDylibList &JDs, SymbolNameSet Symbols,
               SymbolsResolvedCallback OnResolve, SymbolsReadyCallback OnReady,
-              RegisterDependenciesFunction RegisterDependencies);
+              RegisterDependenciesFunction RegisterDependencies,
+              JITDylib *MatchNonExportedInJD = nullptr,
+              bool MatchNonExported = false);
 
   /// Blocking version of lookup above. Returns the resolved symbol map.
   /// If WaitUntilReady is true (the default), will not return until all
@@ -805,29 +803,24 @@ class ExecutionSession {
   /// or an error occurs. If WaitUntilReady is false and an error occurs
   /// after resolution, the function will return a success value, but the
   /// error will be reported via reportErrors.
-  Expected<SymbolMap> lookup(const JITDylibSearchList &SearchOrder,
+  Expected<SymbolMap> lookup(const JITDylibList &JDs,
                              const SymbolNameSet &Symbols,
                              RegisterDependenciesFunction RegisterDependencies =
                                  NoDependenciesToRegister,
-                             bool WaitUntilReady = true);
-
-  /// Convenience version of blocking lookup.
-  /// Searches each of the JITDylibs in the search order in turn for the given
-  /// symbol.
-  Expected<JITEvaluatedSymbol> lookup(const JITDylibSearchList &SearchOrder,
-                                      SymbolStringPtr Symbol);
+                             bool WaitUntilReady = true,
+                             JITDylib *MatchNonExportedInJD = nullptr,
+                             bool MatchNonExported = false);
 
   /// Convenience version of blocking lookup.
-  /// Searches each of the JITDylibs in the search order in turn for the given
-  /// symbol. The search will not find non-exported symbols.
-  Expected<JITEvaluatedSymbol> lookup(ArrayRef<JITDylib *> SearchOrder,
-                                      SymbolStringPtr Symbol);
+  /// Performs a single-symbol lookup.
+  Expected<JITEvaluatedSymbol> lookup(const JITDylibList &JDs,
+                                      SymbolStringPtr Symbol,
+                                      bool MatchNonExported = false);
 
   /// Convenience version of blocking lookup.
-  /// Searches each of the JITDylibs in the search order in turn for the given
-  /// symbol. The search will not find non-exported symbols.
-  Expected<JITEvaluatedSymbol> lookup(ArrayRef<JITDylib *> SearchOrder,
-                                      StringRef Symbol);
+  /// Performs a single-symbol lookup, auto-interning the given symbol name.
+  Expected<JITEvaluatedSymbol> lookup(const JITDylibList &JDs, StringRef Symbol,
+                                      bool MatchNonExported = false);
 
   /// Materialize the given unit.
   void dispatchMaterialization(JITDylib &JD,
@@ -873,7 +866,7 @@ class ExecutionSession {
 
 template <typename Func>
 auto JITDylib::withSearchOrderDo(Func &&F)
-    -> decltype(F(std::declval<const JITDylibSearchList &>())) {
+    -> decltype(F(std::declval<const JITDylibList &>())) {
   return ES.runSessionLocked([&]() { return F(SearchOrder); });
 }
 

llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h

@@ -144,7 +144,7 @@ class LLLazyJIT : public LLJIT {
   /// LLLazyJIT with the given number of compile threads.
   static Expected<std::unique_ptr<LLLazyJIT>>
   Create(JITTargetMachineBuilder JTMB, DataLayout DL,
-         JITTargetAddress ErrorAddr, unsigned NumCompileThreads = 0);
+         unsigned NumCompileThreads = 0);
 
   /// Set an IR transform (e.g. pass manager pipeline) to run on each function
   /// when it is compiled.

llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp

@@ -157,7 +157,7 @@ void CompileOnDemandLayer::emit(MaterializationResponsibility R,
     return;
   }
 
-  R.replace(reexports(PDR.getImplDylib(), std::move(NonCallables), true));
+  R.replace(reexports(PDR.getImplDylib(), std::move(NonCallables)));
   R.replace(lazyReexports(LCTMgr, PDR.getISManager(), PDR.getImplDylib(),
                           std::move(Callables)));
 }
@@ -166,17 +166,10 @@ CompileOnDemandLayer::PerDylibResources &
 CompileOnDemandLayer::getPerDylibResources(JITDylib &TargetD) {
   auto I = DylibResources.find(&TargetD);
   if (I == DylibResources.end()) {
-    auto &ImplD = getExecutionSession().createJITDylib(
-        TargetD.getName() + ".impl", false);
-    TargetD.withSearchOrderDo([&](const JITDylibSearchList &TargetSearchOrder) {
-      auto NewSearchOrder = TargetSearchOrder;
-      assert(!NewSearchOrder.empty() &&
-             NewSearchOrder.front().first == &TargetD &&
-             NewSearchOrder.front().second == true &&
-             "TargetD must be at the front of its own search order and match "
-             "non-exported symbol");
-      NewSearchOrder.insert(std::next(NewSearchOrder.begin()), {&ImplD, true});
-      ImplD.setSearchOrder(std::move(NewSearchOrder), false);
+    auto &ImplD =
+        getExecutionSession().createJITDylib(TargetD.getName() + ".impl");
+    TargetD.withSearchOrderDo([&](const JITDylibList &TargetSearchOrder) {
+      ImplD.setSearchOrder(TargetSearchOrder, false);
     });
     PerDylibResources PDR(ImplD, BuildIndirectStubsManager());
     I = DylibResources.insert(std::make_pair(&TargetD, std::move(PDR))).first;