[llvm] r323398 - [ORC] Refactor the various lookupFlags methods to return the flags map via the
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 24 17:43:00 PST 2018
Author: lhames
Date: Wed Jan 24 17:43:00 2018
New Revision: 323398
URL: http://llvm.org/viewvc/llvm-project?rev=323398&view=rev
Log:
[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).
Modified:
llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h
llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h Wed Jan 24 17:43:00 2018
@@ -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.
///
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h Wed Jan 24 17:43:00 2018
@@ -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 SymbolName
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 SymbolName
/// 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>
Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Wed Jan 24 17:43:00 2018
@@ -288,8 +288,7 @@ void VSO::finalize(SymbolNameSet Symbols
}
}
-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(Symbo
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,
Modified: llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp Wed Jan 24 17:43:00 2018
@@ -62,8 +62,10 @@ JITSymbolResolverAdapter::lookupFlags(co
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;
}
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Wed Jan 24 17:43:00 2018
@@ -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, TestLambdaSymbolResol
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, TestLambdaSymbolResol
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;
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp?rev=323398&r1=323397&r2=323398&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp Wed Jan 24 17:43:00 2018
@@ -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, QueryAgainstV
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, LegacyLookupH
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";
More information about the llvm-commits
mailing list