[ORC] Simplify VSO::lookupFlags to return the flags map.

This discards the unresolved symbols set and returns the flags map directly
(rather than mutating it via the first argument).

The unresolved symbols result made it easy to chain lookupFlags calls, but such
chaining should be rare to non-existant (especially now that symbol resolvers
are being deprecated) so the simpler method signature is preferable.

llvm-svn: 337594

llvm/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h

@@ -499,20 +499,29 @@ private:
     };
 
     auto GVsResolver = createSymbolResolver(
-        [&LD, LegacyLookup](SymbolFlagsMap &SymbolFlags,
+        [&LD, LegacyLookup](const SymbolNameSet &Symbols) {
-                            const SymbolNameSet &Symbols) {
+          auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
-          auto NotFoundViaLegacyLookup =
-              lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
 
-          if (!NotFoundViaLegacyLookup) {
+          if (!SymbolFlags) {
-            logAllUnhandledErrors(NotFoundViaLegacyLookup.takeError(), errs(),
+            logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
                                   "CODLayer/GVsResolver flags lookup failed: ");
-            SymbolFlags.clear();
+            return SymbolFlagsMap();
-            return SymbolNameSet();
           }
 
-          return LD.BackingResolver->lookupFlags(SymbolFlags,
+          if (SymbolFlags->size() == Symbols.size())
-                                                 *NotFoundViaLegacyLookup);
+            return *SymbolFlags;
+
+          SymbolNameSet NotFoundViaLegacyLookup;
+          for (auto &S : Symbols)
+            if (!SymbolFlags->count(S))
+              NotFoundViaLegacyLookup.insert(S);
+          auto SymbolFlags2 =
+              LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+
+          for (auto &KV : SymbolFlags2)
+            (*SymbolFlags)[KV.first] = std::move(KV.second);
+
+          return *SymbolFlags;
         },
         [this, &LD,
          LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -659,18 +668,29 @@ private:
 
     // Create memory manager and symbol resolver.
     auto Resolver = createSymbolResolver(
-        [&LD, LegacyLookup](SymbolFlagsMap &SymbolFlags,
+        [&LD, LegacyLookup](const SymbolNameSet &Symbols) {
-                            const SymbolNameSet &Symbols) {
+          auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
-          auto NotFoundViaLegacyLookup =
+          if (!SymbolFlags) {
-              lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
+            logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
-          if (!NotFoundViaLegacyLookup) {
-            logAllUnhandledErrors(NotFoundViaLegacyLookup.takeError(), errs(),
                                   "CODLayer/SubResolver flags lookup failed: ");
-            SymbolFlags.clear();
+            return SymbolFlagsMap();
-            return SymbolNameSet();
           }
-          return LD.BackingResolver->lookupFlags(SymbolFlags,
+
-                                                 *NotFoundViaLegacyLookup);
+          if (SymbolFlags->size() == Symbols.size())
+            return *SymbolFlags;
+
+          SymbolNameSet NotFoundViaLegacyLookup;
+          for (auto &S : Symbols)
+            if (!SymbolFlags->count(S))
+              NotFoundViaLegacyLookup.insert(S);
+
+          auto SymbolFlags2 =
+              LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+
+          for (auto &KV : SymbolFlags2)
+            (*SymbolFlags)[KV.first] = std::move(KV.second);
+
+          return *SymbolFlags;
         },
         [this, &LD, LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Q,
                                   SymbolNameSet Symbols) {

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

@@ -577,7 +577,7 @@ public:
 
   /// Search the given VSO for the symbols in Symbols. If found, store
   ///        the flags for each symbol in Flags. Returns any unresolved symbols.
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags, const SymbolNameSet &Names);
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Names);
 
   /// Search the given VSOs in order for the symbols in Symbols. Results
   ///        (once they become available) will be returned via the given Query.

llvm/include/llvm/ExecutionEngine/Orc/Legacy.h

@@ -33,8 +33,7 @@ public:
 
   /// Returns the flags for each symbol in Symbols that can be found,
   ///        along with the set of symbol that could not be found.
-  virtual SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+  virtual SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) = 0;
-                                    const SymbolNameSet &Symbols) = 0;
 
   /// For each symbol in Symbols that can be found, assigns that symbols
   ///        value in Query. Returns the set of symbols that could not be found.
@@ -55,9 +54,8 @@ public:
       : LookupFlags(std::forward<LookupFlagsFnRef>(LookupFlags)),
         Lookup(std::forward<LookupFnRef>(Lookup)) {}
 
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
-                            const SymbolNameSet &Symbols) final {
+    return LookupFlags(Symbols);
-    return LookupFlags(Flags, Symbols);
   }
 
   SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -111,21 +109,18 @@ private:
 ///
 /// Useful for implementing lookupFlags bodies that query legacy resolvers.
 template <typename FindSymbolFn>
-Expected<SymbolNameSet> lookupFlagsWithLegacyFn(SymbolFlagsMap &SymbolFlags,
+Expected<SymbolFlagsMap> lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols,
-                                                const SymbolNameSet &Symbols,
+                                                 FindSymbolFn FindSymbol) {
-                                                FindSymbolFn FindSymbol) {
+  SymbolFlagsMap SymbolFlags;
-  SymbolNameSet SymbolsNotFound;
 
   for (auto &S : Symbols) {
     if (JITSymbol Sym = FindSymbol(*S))
       SymbolFlags[S] = Sym.getFlags();
     else if (auto Err = Sym.takeError())
       return std::move(Err);
-    else
-      SymbolsNotFound.insert(S);
   }
 
-  return SymbolsNotFound;
+  return SymbolFlags;
 }
 
 /// Use the given legacy-style FindSymbol function (i.e. a function that
@@ -182,14 +177,12 @@ public:
       : ES(ES), LegacyLookup(std::move(LegacyLookup)),
         ReportError(std::move(ReportError)) {}
 
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
-                            const SymbolNameSet &Symbols) final {
+    if (auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup))
-    if (auto RemainingSymbols =
+      return std::move(*SymbolFlags);
-            lookupFlagsWithLegacyFn(Flags, Symbols, LegacyLookup))
-      return std::move(*RemainingSymbols);
     else {
-      ReportError(RemainingSymbols.takeError());
+      ReportError(SymbolFlags.takeError());
-      return Symbols;
+      return SymbolFlagsMap();
     }
   }
 

llvm/include/llvm/ExecutionEngine/Orc/NullResolver.h

@@ -23,8 +23,7 @@ namespace orc {
 
 class NullResolver : public SymbolResolver {
 public:
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override;
-                            const SymbolNameSet &Symbols) override;
 
   SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                        SymbolNameSet Symbols) override;

llvm/lib/ExecutionEngine/Orc/Core.cpp

@@ -522,11 +522,14 @@ ReExportsMaterializationUnit::extractFlags(const SymbolAliasMap &Aliases) {
 
 Expected<SymbolAliasMap>
 buildSimpleReexportsAliasMap(VSO &SourceV, const SymbolNameSet &Symbols) {
-  SymbolFlagsMap Flags;
+  auto Flags = SourceV.lookupFlags(Symbols);
-  auto Unresolved = SourceV.lookupFlags(Flags, Symbols);
 
-  if (!Unresolved.empty())
+  if (Flags.size() != Symbols.size()) {
+    SymbolNameSet Unresolved = Symbols;
+    for (auto &KV : Flags)
+      Unresolved.erase(KV.first);
     return make_error<SymbolsNotFound>(std::move(Unresolved));
+  }
 
   SymbolAliasMap Result;
   for (auto &Name : Symbols) {
@@ -900,22 +903,20 @@ void VSO::removeFromSearchOrder(VSO &V) {
   });
 }
 
-SymbolNameSet VSO::lookupFlags(SymbolFlagsMap &Flags,
+SymbolFlagsMap VSO::lookupFlags(const SymbolNameSet &Names) {
-                               const SymbolNameSet &Names) {
   return ES.runSessionLocked([&, this]() {
-    auto Unresolved = lookupFlagsImpl(Flags, Names);
+    SymbolFlagsMap Result;
+    auto Unresolved = lookupFlagsImpl(Result, Names);
     if (FallbackDefinitionGenerator && !Unresolved.empty()) {
       auto FallbackDefs = FallbackDefinitionGenerator(*this, Unresolved);
       if (!FallbackDefs.empty()) {
-        auto Unresolved2 = lookupFlagsImpl(Flags, FallbackDefs);
+        auto Unresolved2 = lookupFlagsImpl(Result, FallbackDefs);
         (void)Unresolved2;
         assert(Unresolved2.empty() &&
                "All fallback defs should have been found by lookupFlagsImpl");
-        for (auto &D : FallbackDefs)
-          Unresolved.erase(D);
       }
     };
-    return Unresolved;
+    return Result;
   });
 }
 

llvm/lib/ExecutionEngine/Orc/Legacy.cpp

@@ -48,8 +48,7 @@ JITSymbolResolverAdapter::lookupFlags(const LookupSet &Symbols) {
   for (auto &S : Symbols)
     InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
 
-  SymbolFlagsMap SymbolFlags;
+  SymbolFlagsMap SymbolFlags = R.lookupFlags(InternedSymbols);
-  R.lookupFlags(SymbolFlags, InternedSymbols);
   LookupFlagsResult Result;
   for (auto &KV : SymbolFlags) {
     ResolvedStrings.insert(KV.first);

llvm/lib/ExecutionEngine/Orc/NullResolver.cpp

@@ -14,9 +14,8 @@
 namespace llvm {
 namespace orc {
 
-SymbolNameSet NullResolver::lookupFlags(SymbolFlagsMap &Flags,
+SymbolFlagsMap NullResolver::lookupFlags(const SymbolNameSet &Symbols) {
-                                        const SymbolNameSet &Symbols) {
+  return SymbolFlagsMap();
-  return Symbols;
 }
 
 SymbolNameSet

llvm/lib/ExecutionEngine/Orc/OrcCBindingsStack.h

@@ -129,21 +129,20 @@ private:
         : Stack(Stack), ExternalResolver(std::move(ExternalResolver)),
           ExternalResolverCtx(std::move(ExternalResolverCtx)) {}
 
-    orc::SymbolNameSet lookupFlags(orc::SymbolFlagsMap &SymbolFlags,
+    orc::SymbolFlagsMap
-                                   const orc::SymbolNameSet &Symbols) override {
+    lookupFlags(const orc::SymbolNameSet &Symbols) override {
-      orc::SymbolNameSet SymbolsNotFound;
+      orc::SymbolFlagsMap SymbolFlags;
 
       for (auto &S : Symbols) {
         if (auto Sym = findSymbol(*S))
           SymbolFlags[S] = Sym.getFlags();
         else if (auto Err = Sym.takeError()) {
           Stack.reportError(std::move(Err));
-          return orc::SymbolNameSet();
+          return orc::SymbolFlagsMap();
-        } else
+        }
-          SymbolsNotFound.insert(S);
       }
 
-      return SymbolsNotFound;
+      return SymbolFlags;
     }
 
     orc::SymbolNameSet

llvm/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h

@@ -144,28 +144,26 @@ class OrcMCJITReplacement : public ExecutionEngine {
   public:
     LinkingORCResolver(OrcMCJITReplacement &M) : M(M) {}
 
-    SymbolNameSet lookupFlags(SymbolFlagsMap &SymbolFlags,
+    SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override {
-                              const SymbolNameSet &Symbols) override {
+      SymbolFlagsMap SymbolFlags;
-      SymbolNameSet UnresolvedSymbols;
 
       for (auto &S : Symbols) {
         if (auto Sym = M.findMangledSymbol(*S)) {
           SymbolFlags[S] = Sym.getFlags();
         } else if (auto Err = Sym.takeError()) {
           M.reportError(std::move(Err));
-          return SymbolNameSet();
+          return SymbolFlagsMap();
         } else {
           if (auto Sym2 = M.ClientResolver->findSymbolInLogicalDylib(*S)) {
             SymbolFlags[S] = Sym2.getFlags();
           } else if (auto Err = Sym2.takeError()) {
             M.reportError(std::move(Err));
-            return SymbolNameSet();
+            return SymbolFlagsMap();
-          } else
+          }
-            UnresolvedSymbols.insert(S);
         }
       }
 
-      return UnresolvedSymbols;
+      return SymbolFlags;
     }
 
     SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,

llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp

@@ -64,7 +64,7 @@ public:
         return;
 
       assert(VSOs.front() && "VSOList entry can not be null");
-      VSOs.front()->lookupFlags(InternedResult, InternedSymbols);
+      InternedResult = VSOs.front()->lookupFlags(InternedSymbols);
     });
 
     LookupFlagsResult Result;

llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

@@ -215,11 +215,8 @@ TEST_F(CoreAPIsStandardTest, LookupFlagsTest) {
 
   SymbolNameSet Names({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
+  auto SymbolFlags = V.lookupFlags(Names);
-  auto SymbolsNotFound = V.lookupFlags(SymbolFlags, Names);
 
-  EXPECT_EQ(SymbolsNotFound.size(), 1U) << "Expected one not-found symbol";
-  EXPECT_EQ(SymbolsNotFound.count(Baz), 1U) << "Expected Baz to be not-found";
   EXPECT_EQ(SymbolFlags.size(), 2U)
       << "Returned symbol flags contains unexpected results";
   EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Missing lookupFlags result for Foo";

llvm/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp

@@ -22,17 +22,14 @@ TEST_F(LegacyAPIsStandardTest, TestLambdaSymbolResolver) {
   cantFail(V.define(absoluteSymbols({{Foo, FooSym}, {Bar, BarSym}})));
 
   auto Resolver = createSymbolResolver(
-      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
+      [&](const SymbolNameSet &Symbols) { return V.lookupFlags(Symbols); },
-        return V.lookupFlags(SymbolFlags, Symbols);
-      },
       [&](std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Symbols) {
         return V.lookup(std::move(Q), Symbols);
       });
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
+  SymbolFlagsMap SymbolFlags = Resolver->lookupFlags(Symbols);
-  SymbolNameSet SymbolsNotFound = Resolver->lookupFlags(SymbolFlags, Symbols);
 
   EXPECT_EQ(SymbolFlags.size(), 2U)
       << "lookupFlags returned the wrong number of results";
@@ -42,10 +39,6 @@ TEST_F(LegacyAPIsStandardTest, TestLambdaSymbolResolver) {
       << "Incorrect lookupFlags result for Foo";
   EXPECT_EQ(SymbolFlags[Bar], BarSym.getFlags())
       << "Incorrect lookupFlags result for Bar";
-  EXPECT_EQ(SymbolsNotFound.size(), 1U)
-      << "Expected one symbol not found in lookupFlags";
-  EXPECT_EQ(SymbolsNotFound.count(Baz), 1U)
-      << "Expected baz not to be found in lookupFlags";
 
   bool OnResolvedRun = false;
 
@@ -86,9 +79,8 @@ TEST(LegacyAPIInteropTest, QueryAgainstVSO) {
   JITEvaluatedSymbol FooSym(0xdeadbeef, JITSymbolFlags::Exported);
   cantFail(V.define(absoluteSymbols({{Foo, FooSym}})));
 
-  auto LookupFlags = [&](SymbolFlagsMap &SymbolFlags,
+  auto LookupFlags = [&](const SymbolNameSet &Names) {
-                         const SymbolNameSet &Names) {
+    return V.lookupFlags(Names);
-    return V.lookupFlags(SymbolFlags, Names);
   };
 
   auto Lookup = [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -153,19 +145,14 @@ TEST(LegacyAPIInteropTset, LegacyLookupHelpersFn) {
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
+  auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
-  auto SymbolsNotFound =
-      lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
 
-  EXPECT_TRUE(!!SymbolsNotFound) << "lookupFlagsWithLegacy failed unexpectedly";
+  EXPECT_TRUE(!!SymbolFlags) << "Expected lookupFlagsWithLegacyFn to succeed";
-  EXPECT_EQ(SymbolFlags.size(), 2U) << "Wrong number of flags returned";
+  EXPECT_EQ(SymbolFlags->size(), 2U) << "Wrong number of flags returned";
-  EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Flags for foo missing";
+  EXPECT_EQ(SymbolFlags->count(Foo), 1U) << "Flags for foo missing";
-  EXPECT_EQ(SymbolFlags.count(Bar), 1U) << "Flags for foo missing";
+  EXPECT_EQ(SymbolFlags->count(Bar), 1U) << "Flags for foo missing";
-  EXPECT_EQ(SymbolFlags[Foo], FooFlags) << "Wrong flags for foo";
+  EXPECT_EQ((*SymbolFlags)[Foo], FooFlags) << "Wrong flags for foo";
-  EXPECT_EQ(SymbolFlags[Bar], BarFlags) << "Wrong flags for foo";
+  EXPECT_EQ((*SymbolFlags)[Bar], BarFlags) << "Wrong flags for foo";
-  EXPECT_EQ(SymbolsNotFound->size(), 1U) << "Expected one symbol not found";
-  EXPECT_EQ(SymbolsNotFound->count(Baz), 1U)
-      << "Expected symbol baz to be not found";
   EXPECT_FALSE(BarMaterialized)
       << "lookupFlags should not have materialized bar";
 

llvm/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp

@@ -184,9 +184,8 @@ TEST_F(RTDyldObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {
   };
 
   Resolvers[K2] = createSymbolResolver(
-      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
+      [&](const SymbolNameSet &Symbols) {
-        return cantFail(
+        return cantFail(lookupFlagsWithLegacyFn(Symbols, LegacyLookup));
-            lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup));
       },
       [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
           const SymbolNameSet &Symbols) {