[ORC] Refactor the various lookupFlags methods to return the flags map via the

first argument.

This makes lookupFlags more consistent with lookup (which takes the query as the
first argument) and composes better in practice, since lookups are usually
linearly chained: Each lookupFlags can populate the result map based on the
symbols not found in the previous lookup. (If the maps were returned rather than
passed by reference there would have to be a merge step at the end).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@323398 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ExecutionEngine/Orc/Core.h b/include/llvm/ExecutionEngine/Orc/Core.h
index be2492a..52ce042 100644 (file)
--- a/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/include/llvm/ExecutionEngine/Orc/Core.h
@@ -92,14 +92,6 @@ private:
   SymbolsReadyCallback NotifySymbolsReady;
 };
 
-/// @brief A SymbolFlagsMap containing flags of found symbols, plus a set of
-///        not-found symbols. Shared between SymbolResolver::lookupFlags and
-///        VSO::lookupFlags for convenience.
-struct LookupFlagsResult {
-  SymbolFlagsMap SymbolFlags;
-  SymbolNameSet SymbolsNotFound;
-};
-
 /// @brief SymbolResolver is a composable interface for looking up symbol flags
 ///        and addresses using the AsynchronousSymbolQuery type. It will
 ///        eventually replace the LegacyJITSymbolResolver interface as the
@@ -110,7 +102,8 @@ public:
 
   /// @brief Returns the flags for each symbol in Symbols that can be found,
   ///        along with the set of symbol that could not be found.
-  virtual LookupFlagsResult lookupFlags(const SymbolNameSet &Symbols) = 0;
+  virtual SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+                                    const SymbolNameSet &Symbols) = 0;
 
   /// @brief 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.
@@ -131,8 +124,9 @@ public:
       : LookupFlags(std::forward<LookupFlagsFnRef>(LookupFlags)),
         Lookup(std::forward<LookupFnRef>(Lookup)) {}
 
-  LookupFlagsResult lookupFlags(const SymbolNameSet &Symbols) final {
-    return LookupFlags(Symbols);
+  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
+                            const SymbolNameSet &Symbols) final {
+    return LookupFlags(Flags, Symbols);
   }
 
   SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
@@ -250,7 +244,7 @@ public:
   ///
   /// Returns the flags for the give symbols, together with the set of symbols
   /// not found.
-  LookupFlagsResult lookupFlags(SymbolNameSet Symbols);
+  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags, SymbolNameSet Symbols);
 
   /// @brief Apply the given query to the given symbols in this VSO.
   ///

diff --git a/include/llvm/ExecutionEngine/Orc/Legacy.h b/include/llvm/ExecutionEngine/Orc/Legacy.h
index 9947e78..e04ef93 100644 (file)
--- a/include/llvm/ExecutionEngine/Orc/Legacy.h
+++ b/include/llvm/ExecutionEngine/Orc/Legacy.h
@@ -23,8 +23,8 @@ namespace orc {
 class JITSymbolResolverAdapter : public JITSymbolResolver {
 public:
   JITSymbolResolverAdapter(ExecutionSession &ES, SymbolResolver &R);
-  Expected<LookupResult> lookup(const LookupSet &Symbols) override;
   Expected<LookupFlagsResult> lookupFlags(const LookupSet &Symbols) override;
+  Expected<LookupResult> lookup(const LookupSet &Symbols) override;
 
 private:
   ExecutionSession &ES;
@@ -35,15 +35,15 @@ private:
 /// @brief Use the given legacy-style FindSymbol function (i.e. a function that
 ///        takes a const std::string& or StringRef and returns a JITSymbol) to
 ///        find the flags for each symbol in Symbols and store their flags in
-///        FlagsMap. If any JITSymbol returned by FindSymbol is in an error
+///        SymbolFlags. If any JITSymbol returned by FindSymbol is in an error
 ///        state the function returns immediately with that error, otherwise it
 ///        returns the set of symbols not found.
 ///
 /// Useful for implementing lookupFlags bodies that query legacy resolvers.
 template <typename FindSymbolFn>
-Expected<LookupFlagsResult>
-lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols, FindSymbolFn FindSymbol) {
-  SymbolFlagsMap SymbolFlags;
+Expected<SymbolNameSet> lookupFlagsWithLegacyFn(SymbolFlagsMap &SymbolFlags,
+                                                const SymbolNameSet &Symbols,
+                                                FindSymbolFn FindSymbol) {
   SymbolNameSet SymbolsNotFound;
 
   for (auto &S : Symbols) {
@@ -55,7 +55,7 @@ lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols, FindSymbolFn FindSymbol) {
       SymbolsNotFound.insert(S);
   }
 
-  return LookupFlagsResult{std::move(SymbolFlags), std::move(SymbolsNotFound)};
+  return SymbolsNotFound;
 }
 
 /// @brief Use the given legacy-style FindSymbol function (i.e. a function that
@@ -64,7 +64,7 @@ lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols, FindSymbolFn FindSymbol) {
 ///        result in Query. If any JITSymbol returned by FindSymbol is in an
 ///        error then Query.setFailed(...) is called with that error and the
 ///        function returns immediately. On success, returns the set of symbols
-///         not found.
+///        not found.
 ///
 /// Useful for implementing lookup bodies that query legacy resolvers.
 template <typename FindSymbolFn>

diff --git a/lib/ExecutionEngine/Orc/Core.cpp b/lib/ExecutionEngine/Orc/Core.cpp
index ff78ba1..886bd7e 100644 (file)
--- a/lib/ExecutionEngine/Orc/Core.cpp
+++ b/lib/ExecutionEngine/Orc/Core.cpp
@@ -288,8 +288,7 @@ void VSO::finalize(SymbolNameSet SymbolsToFinalize) {
   }
 }
 
-LookupFlagsResult VSO::lookupFlags(SymbolNameSet Names) {
-  SymbolFlagsMap FlagsFound;
+SymbolNameSet VSO::lookupFlags(SymbolFlagsMap &Flags, SymbolNameSet Names) {
 
   for (SymbolNameSet::iterator I = Names.begin(), E = Names.end(); I != E;) {
     auto Tmp = I++;
@@ -301,11 +300,11 @@ LookupFlagsResult VSO::lookupFlags(SymbolNameSet Names) {
 
     Names.erase(Tmp);
 
-    FlagsFound[SymI->first] =
+    Flags[SymI->first] =
         JITSymbolFlags::stripTransientFlags(SymI->second.getFlags());
   }
 
-  return {std::move(FlagsFound), std::move(Names)};
+  return Names;
 }
 
 VSO::LookupResult VSO::lookup(AsynchronousSymbolQuery &Query,

diff --git a/lib/ExecutionEngine/Orc/Legacy.cpp b/lib/ExecutionEngine/Orc/Legacy.cpp
index e4eba8b..7a11b94 100644 (file)
--- a/lib/ExecutionEngine/Orc/Legacy.cpp
+++ b/lib/ExecutionEngine/Orc/Legacy.cpp
@@ -62,8 +62,10 @@ JITSymbolResolverAdapter::lookupFlags(const LookupSet &Symbols) {
   for (auto &S : Symbols)
     InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
 
+  SymbolFlagsMap SymbolFlags;
+  R.lookupFlags(SymbolFlags, InternedSymbols);
   LookupFlagsResult Result;
-  for (auto &KV : R.lookupFlags(InternedSymbols).SymbolFlags) {
+  for (auto &KV : SymbolFlags) {
     ResolvedStrings.insert(KV.first);
     Result[*KV.first] = KV.second;
   }

diff --git a/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
index 3454ee2..1f1eb51 100644 (file)
--- a/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
+++ b/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
@@ -163,21 +163,18 @@ TEST(CoreAPIsTest, LookupFlagsTest) {
 
   SymbolNameSet Names({Foo, Bar, Baz});
 
-  auto LFR = V.lookupFlags(Names);
+  SymbolFlagsMap SymbolFlags;
+  auto SymbolsNotFound = V.lookupFlags(SymbolFlags, Names);
 
-  EXPECT_EQ(LFR.SymbolsNotFound.size(), 1U) << "Expected one not-found symbol";
-  EXPECT_EQ(*LFR.SymbolsNotFound.begin(), Baz)
-      << "Expected Baz to be not-found";
-  EXPECT_EQ(LFR.SymbolFlags.size(), 2U)
+  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(LFR.SymbolFlags.count(Foo), 1U)
-      << "Missing lookupFlags result for Foo";
-  EXPECT_EQ(LFR.SymbolFlags[Foo], FooFlags)
-      << "Incorrect flags returned for Foo";
-  EXPECT_EQ(LFR.SymbolFlags.count(Bar), 1U)
+  EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Missing lookupFlags result for Foo";
+  EXPECT_EQ(SymbolFlags[Foo], FooFlags) << "Incorrect flags returned for Foo";
+  EXPECT_EQ(SymbolFlags.count(Bar), 1U)
       << "Missing  lookupFlags result for Bar";
-  EXPECT_EQ(LFR.SymbolFlags[Bar], BarFlags)
-      << "Incorrect flags returned for Bar";
+  EXPECT_EQ(SymbolFlags[Bar], BarFlags) << "Incorrect flags returned for Bar";
 }
 
 TEST(CoreAPIsTest, AddAndMaterializeLazySymbol) {
@@ -271,7 +268,9 @@ TEST(CoreAPIsTest, TestLambdaSymbolResolver) {
   cantFail(V.define({{Foo, FooSym}, {Bar, BarSym}}));
 
   auto Resolver = createSymbolResolver(
-      [&](const SymbolNameSet &Symbols) { return V.lookupFlags(Symbols); },
+      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
+        return V.lookupFlags(SymbolFlags, Symbols);
+      },
       [&](AsynchronousSymbolQuery &Q, SymbolNameSet Symbols) {
         auto LR = V.lookup(Q, Symbols);
         assert(LR.MaterializationWork.empty() &&
@@ -282,21 +281,20 @@ TEST(CoreAPIsTest, TestLambdaSymbolResolver) {
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  LookupFlagsResult LFR = Resolver->lookupFlags(Symbols);
+  SymbolFlagsMap SymbolFlags;
+  SymbolNameSet SymbolsNotFound = Resolver->lookupFlags(SymbolFlags, Symbols);
 
-  EXPECT_EQ(LFR.SymbolFlags.size(), 2U)
+  EXPECT_EQ(SymbolFlags.size(), 2U)
       << "lookupFlags returned the wrong number of results";
-  EXPECT_EQ(LFR.SymbolFlags.count(Foo), 1U)
-      << "Missing lookupFlags result for foo";
-  EXPECT_EQ(LFR.SymbolFlags.count(Bar), 1U)
-      << "Missing lookupFlags result for bar";
-  EXPECT_EQ(LFR.SymbolFlags[Foo], FooSym.getFlags())
+  EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Missing lookupFlags result for foo";
+  EXPECT_EQ(SymbolFlags.count(Bar), 1U) << "Missing lookupFlags result for bar";
+  EXPECT_EQ(SymbolFlags[Foo], FooSym.getFlags())
       << "Incorrect lookupFlags result for Foo";
-  EXPECT_EQ(LFR.SymbolFlags[Bar], BarSym.getFlags())
+  EXPECT_EQ(SymbolFlags[Bar], BarSym.getFlags())
       << "Incorrect lookupFlags result for Bar";
-  EXPECT_EQ(LFR.SymbolsNotFound.size(), 1U)
+  EXPECT_EQ(SymbolsNotFound.size(), 1U)
       << "Expected one symbol not found in lookupFlags";
-  EXPECT_EQ(LFR.SymbolsNotFound.count(Baz), 1U)
+  EXPECT_EQ(SymbolsNotFound.count(Baz), 1U)
       << "Expected baz not to be found in lookupFlags";
 
   bool OnResolvedRun = false;

diff --git a/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp b/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
index 963ce9f..19c194b 100644 (file)
--- a/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
+++ b/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
@@ -16,15 +16,17 @@ using namespace llvm::orc;
 
 class SimpleORCResolver : public SymbolResolver {
 public:
-  using LookupFlagsFn = std::function<LookupFlagsResult(const SymbolNameSet &)>;
+  using LookupFlagsFn = std::function<SymbolNameSet(SymbolFlagsMap &SymbolFlags,
+                                                    const SymbolNameSet &)>;
   using LookupFn = std::function<SymbolNameSet(AsynchronousSymbolQuery &Q,
                                                SymbolNameSet Symbols)>;
 
   SimpleORCResolver(LookupFlagsFn LookupFlags, LookupFn Lookup)
       : LookupFlags(std::move(LookupFlags)), Lookup(std::move(Lookup)) {}
 
-  LookupFlagsResult lookupFlags(const SymbolNameSet &Symbols) override {
-    return LookupFlags(Symbols);
+  SymbolNameSet lookupFlags(SymbolFlagsMap &SymbolFlags,
+                            const SymbolNameSet &Symbols) override {
+    return LookupFlags(SymbolFlags, Symbols);
   }
 
   SymbolNameSet lookup(AsynchronousSymbolQuery &Query,
@@ -51,8 +53,9 @@ TEST(LegacyAPIInteropTest, QueryAgainstVSO) {
   Defs[Foo] = FooSym;
   cantFail(V.define(std::move(Defs)));
 
-  auto LookupFlags = [&](const SymbolNameSet &Names) {
-    return V.lookupFlags(Names);
+  auto LookupFlags = [&](SymbolFlagsMap &SymbolFlags,
+                         const SymbolNameSet &Names) {
+    return V.lookupFlags(SymbolFlags, Names);
   };
 
   auto Lookup = [&](AsynchronousSymbolQuery &Query, SymbolNameSet Symbols) {
@@ -119,16 +122,18 @@ TEST(LegacyAPIInteropTset, LegacyLookupHelpersFn) {
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  auto LFR = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
-
-  EXPECT_TRUE(!!LFR) << "lookupFlagsWithLegacy failed unexpectedly";
-  EXPECT_EQ(LFR->SymbolFlags.size(), 2U) << "Wrong number of flags returned";
-  EXPECT_EQ(LFR->SymbolFlags.count(Foo), 1U) << "Flags for foo missing";
-  EXPECT_EQ(LFR->SymbolFlags.count(Bar), 1U) << "Flags for foo missing";
-  EXPECT_EQ(LFR->SymbolFlags[Foo], FooFlags) << "Wrong flags for foo";
-  EXPECT_EQ(LFR->SymbolFlags[Bar], BarFlags) << "Wrong flags for foo";
-  EXPECT_EQ(LFR->SymbolsNotFound.size(), 1U) << "Expected one symbol not found";
-  EXPECT_EQ(LFR->SymbolsNotFound.count(Baz), 1U)
+  SymbolFlagsMap SymbolFlags;
+  auto SymbolsNotFound =
+      lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
+
+  EXPECT_TRUE(!!SymbolsNotFound) << "lookupFlagsWithLegacy failed unexpectedly";
+  EXPECT_EQ(SymbolFlags.size(), 2U) << "Wrong number of flags returned";
+  EXPECT_EQ(SymbolFlags.count(Foo), 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[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";