[llvm] r340874 - [ORC] Replace lookupFlags in JITSymbolResolver with getResponsibilitySet.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 28 14:18:05 PDT 2018
Author: lhames
Date: Tue Aug 28 14:18:05 2018
New Revision: 340874
URL: http://llvm.org/viewvc/llvm-project?rev=340874&view=rev
Log:
[ORC] Replace lookupFlags in JITSymbolResolver with getResponsibilitySet.
The new method name/behavior more closely models the way it was being used.
It also fixes an assertion that can occur when using the new ORC Core APIs,
where flags alone don't necessarily provide enough context to decide whether
the caller is responsible for materializing a given symbol (which was always
the reason this API existed).
The default implementation of getResponsibilitySet uses lookupFlags to determine
responsibility as before, so existing JITSymbolResolvers should continue to
work.
Added:
llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-1.ll
llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-2.ll
llvm/trunk/test/ExecutionEngine/OrcLazy/weak-non-materialization.ll
Modified:
llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h
llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h
llvm/trunk/include/llvm/ExecutionEngine/Orc/NullResolver.h
llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
llvm/trunk/lib/ExecutionEngine/Orc/NullResolver.cpp
llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
llvm/trunk/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
Modified: llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/JITSymbol.h Tue Aug 28 14:18:05 2018
@@ -298,7 +298,6 @@ class JITSymbolResolver {
public:
using LookupSet = std::set<StringRef>;
using LookupResult = std::map<StringRef, JITEvaluatedSymbol>;
- using LookupFlagsResult = std::map<StringRef, JITSymbolFlags>;
virtual ~JITSymbolResolver() = default;
@@ -309,11 +308,11 @@ public:
/// resolved, or if the resolution process itself triggers an error.
virtual Expected<LookupResult> lookup(const LookupSet &Symbols) = 0;
- /// Returns the symbol flags for each of the given symbols.
- ///
- /// This method does NOT return an error if any of the given symbols is
- /// missing. Instead, that symbol will be left out of the result map.
- virtual Expected<LookupFlagsResult> lookupFlags(const LookupSet &Symbols) = 0;
+ /// Returns the subset of the given symbols that should be materialized by
+ /// the caller. Only weak/common symbols should be looked up, as strong
+ /// definitions are implicitly always part of the caller's responsibility.
+ virtual Expected<LookupSet>
+ getResponsibilitySet(const LookupSet &Symbols) = 0;
private:
virtual void anchor();
@@ -329,7 +328,7 @@ public:
/// Performs flags lookup by calling findSymbolInLogicalDylib and
/// returning the flags value for that symbol.
- Expected<LookupFlagsResult> lookupFlags(const LookupSet &Symbols) final;
+ Expected<LookupSet> getResponsibilitySet(const LookupSet &Symbols) final;
/// This method returns the address of the specified symbol if it exists
/// within the logical dynamic library represented by this JITSymbolResolver.
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h Tue Aug 28 14:18:05 2018
@@ -500,28 +500,29 @@ private:
auto GVsResolver = createSymbolResolver(
[&LD, LegacyLookup](const SymbolNameSet &Symbols) {
- auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
+ auto RS = getResponsibilitySetWithLegacyFn(Symbols, LegacyLookup);
- if (!SymbolFlags) {
- logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
- "CODLayer/GVsResolver flags lookup failed: ");
- return SymbolFlagsMap();
+ if (!RS) {
+ logAllUnhandledErrors(
+ RS.takeError(), errs(),
+ "CODLayer/GVsResolver responsibility set lookup failed: ");
+ return SymbolNameSet();
}
- if (SymbolFlags->size() == Symbols.size())
- return *SymbolFlags;
+ if (RS->size() == Symbols.size())
+ return *RS;
SymbolNameSet NotFoundViaLegacyLookup;
for (auto &S : Symbols)
- if (!SymbolFlags->count(S))
+ if (!RS->count(S))
NotFoundViaLegacyLookup.insert(S);
- auto SymbolFlags2 =
- LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+ auto RS2 =
+ LD.BackingResolver->getResponsibilitySet(NotFoundViaLegacyLookup);
- for (auto &KV : SymbolFlags2)
- (*SymbolFlags)[KV.first] = std::move(KV.second);
+ for (auto &S : RS2)
+ (*RS).insert(S);
- return *SymbolFlags;
+ return *RS;
},
[this, &LD,
LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -669,28 +670,29 @@ private:
// Create memory manager and symbol resolver.
auto Resolver = createSymbolResolver(
[&LD, LegacyLookup](const SymbolNameSet &Symbols) {
- auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
- if (!SymbolFlags) {
- logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
- "CODLayer/SubResolver flags lookup failed: ");
- return SymbolFlagsMap();
+ auto RS = getResponsibilitySetWithLegacyFn(Symbols, LegacyLookup);
+ if (!RS) {
+ logAllUnhandledErrors(
+ RS.takeError(), errs(),
+ "CODLayer/SubResolver responsibility set lookup failed: ");
+ return SymbolNameSet();
}
- if (SymbolFlags->size() == Symbols.size())
- return *SymbolFlags;
+ if (RS->size() == Symbols.size())
+ return *RS;
SymbolNameSet NotFoundViaLegacyLookup;
for (auto &S : Symbols)
- if (!SymbolFlags->count(S))
+ if (!RS->count(S))
NotFoundViaLegacyLookup.insert(S);
- auto SymbolFlags2 =
- LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+ auto RS2 =
+ LD.BackingResolver->getResponsibilitySet(NotFoundViaLegacyLookup);
- for (auto &KV : SymbolFlags2)
- (*SymbolFlags)[KV.first] = std::move(KV.second);
+ for (auto &S : RS2)
+ (*RS).insert(S);
- return *SymbolFlags;
+ return *RS;
},
[this, &LD, LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Q,
SymbolNameSet Symbols) {
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=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Legacy.h Tue Aug 28 14:18:05 2018
@@ -31,12 +31,12 @@ class SymbolResolver {
public:
virtual ~SymbolResolver() = default;
- /// Returns the flags for each symbol in Symbols that can be found,
- /// along with the set of symbol that could not be found.
- virtual SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) = 0;
+ /// Returns the subset of the given symbols that the caller is responsible for
+ /// materializing.
+ virtual SymbolNameSet getResponsibilitySet(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.
+ /// value in Query. Returns the set of symbols that could not be found.
virtual SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
SymbolNameSet Symbols) = 0;
@@ -46,16 +46,18 @@ private:
/// Implements SymbolResolver with a pair of supplied function objects
/// for convenience. See createSymbolResolver.
-template <typename LookupFlagsFn, typename LookupFn>
+template <typename GetResponsibilitySetFn, typename LookupFn>
class LambdaSymbolResolver final : public SymbolResolver {
public:
- template <typename LookupFlagsFnRef, typename LookupFnRef>
- LambdaSymbolResolver(LookupFlagsFnRef &&LookupFlags, LookupFnRef &&Lookup)
- : LookupFlags(std::forward<LookupFlagsFnRef>(LookupFlags)),
+ template <typename GetResponsibilitySetFnRef, typename LookupFnRef>
+ LambdaSymbolResolver(GetResponsibilitySetFnRef &&GetResponsibilitySet,
+ LookupFnRef &&Lookup)
+ : GetResponsibilitySet(
+ std::forward<GetResponsibilitySetFnRef>(GetResponsibilitySet)),
Lookup(std::forward<LookupFnRef>(Lookup)) {}
- SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
- return LookupFlags(Symbols);
+ SymbolNameSet getResponsibilitySet(const SymbolNameSet &Symbols) final {
+ return GetResponsibilitySet(Symbols);
}
SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -64,33 +66,35 @@ public:
}
private:
- LookupFlagsFn LookupFlags;
+ GetResponsibilitySetFn GetResponsibilitySet;
LookupFn Lookup;
};
/// Creates a SymbolResolver implementation from the pair of supplied
/// function objects.
-template <typename LookupFlagsFn, typename LookupFn>
+template <typename GetResponsibilitySetFn, typename LookupFn>
std::unique_ptr<LambdaSymbolResolver<
typename std::remove_cv<
- typename std::remove_reference<LookupFlagsFn>::type>::type,
+ typename std::remove_reference<GetResponsibilitySetFn>::type>::type,
typename std::remove_cv<
typename std::remove_reference<LookupFn>::type>::type>>
-createSymbolResolver(LookupFlagsFn &&LookupFlags, LookupFn &&Lookup) {
+createSymbolResolver(GetResponsibilitySetFn &&GetResponsibilitySet,
+ LookupFn &&Lookup) {
using LambdaSymbolResolverImpl = LambdaSymbolResolver<
typename std::remove_cv<
- typename std::remove_reference<LookupFlagsFn>::type>::type,
+ typename std::remove_reference<GetResponsibilitySetFn>::type>::type,
typename std::remove_cv<
typename std::remove_reference<LookupFn>::type>::type>;
return llvm::make_unique<LambdaSymbolResolverImpl>(
- std::forward<LookupFlagsFn>(LookupFlags), std::forward<LookupFn>(Lookup));
+ std::forward<GetResponsibilitySetFn>(GetResponsibilitySet),
+ std::forward<LookupFn>(Lookup));
}
class JITSymbolResolverAdapter : public JITSymbolResolver {
public:
JITSymbolResolverAdapter(ExecutionSession &ES, SymbolResolver &R,
MaterializationResponsibility *MR);
- Expected<LookupFlagsResult> lookupFlags(const LookupSet &Symbols) override;
+ Expected<LookupSet> getResponsibilitySet(const LookupSet &Symbols) override;
Expected<LookupResult> lookup(const LookupSet &Symbols) override;
private:
@@ -100,27 +104,29 @@ private:
MaterializationResponsibility *MR;
};
-/// 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
-/// 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.
+/// Use the given legacy-style FindSymbol function (i.e. a function that takes
+/// a const std::string& or StringRef and returns a JITSymbol) to get the
+/// subset of symbols that the caller is responsible for materializing. If any
+/// JITSymbol returned by FindSymbol is in an error state the function returns
+/// immediately with that error.
///
-/// Useful for implementing lookupFlags bodies that query legacy resolvers.
+/// Useful for implementing getResponsibilitySet bodies that query legacy
+/// resolvers.
template <typename FindSymbolFn>
-Expected<SymbolFlagsMap> lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols,
- FindSymbolFn FindSymbol) {
- SymbolFlagsMap SymbolFlags;
+Expected<SymbolNameSet>
+getResponsibilitySetWithLegacyFn(const SymbolNameSet &Symbols,
+ FindSymbolFn FindSymbol) {
+ SymbolNameSet Result;
for (auto &S : Symbols) {
- if (JITSymbol Sym = FindSymbol(*S))
- SymbolFlags[S] = Sym.getFlags();
- else if (auto Err = Sym.takeError())
+ if (JITSymbol Sym = FindSymbol(*S)) {
+ if (!Sym.getFlags().isStrong())
+ Result.insert(S);
+ } else if (auto Err = Sym.takeError())
return std::move(Err);
}
- return SymbolFlags;
+ return Result;
}
/// Use the given legacy-style FindSymbol function (i.e. a function that
@@ -177,12 +183,13 @@ public:
: ES(ES), LegacyLookup(std::move(LegacyLookup)),
ReportError(std::move(ReportError)) {}
- SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
- if (auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup))
- return std::move(*SymbolFlags);
+ SymbolNameSet getResponsibilitySet(const SymbolNameSet &Symbols) final {
+ if (auto ResponsibilitySet =
+ getResponsibilitySetWithLegacyFn(Symbols, LegacyLookup))
+ return std::move(*ResponsibilitySet);
else {
- ReportError(SymbolFlags.takeError());
- return SymbolFlagsMap();
+ ReportError(ResponsibilitySet.takeError());
+ return SymbolNameSet();
}
}
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/NullResolver.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/NullResolver.h?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/NullResolver.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/NullResolver.h Tue Aug 28 14:18:05 2018
@@ -23,10 +23,10 @@ namespace orc {
class NullResolver : public SymbolResolver {
public:
- SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override;
+ SymbolNameSet getResponsibilitySet(const SymbolNameSet &Symbols) final;
SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
- SymbolNameSet Symbols) override;
+ SymbolNameSet Symbols) final;
};
/// SymbolResolver impliementation that rejects all resolution requests.
Modified: llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Legacy.cpp Tue Aug 28 14:18:05 2018
@@ -48,17 +48,17 @@ JITSymbolResolverAdapter::lookup(const L
return Result;
}
-Expected<JITSymbolResolverAdapter::LookupFlagsResult>
-JITSymbolResolverAdapter::lookupFlags(const LookupSet &Symbols) {
+Expected<JITSymbolResolverAdapter::LookupSet>
+JITSymbolResolverAdapter::getResponsibilitySet(const LookupSet &Symbols) {
SymbolNameSet InternedSymbols;
for (auto &S : Symbols)
InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
- SymbolFlagsMap SymbolFlags = R.lookupFlags(InternedSymbols);
- LookupFlagsResult Result;
- for (auto &KV : SymbolFlags) {
- ResolvedStrings.insert(KV.first);
- Result[*KV.first] = KV.second;
+ auto InternedResult = R.getResponsibilitySet(InternedSymbols);
+ LookupSet Result;
+ for (auto &S : InternedResult) {
+ ResolvedStrings.insert(S);
+ Result.insert(*S);
}
return Result;
Modified: llvm/trunk/lib/ExecutionEngine/Orc/NullResolver.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/NullResolver.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/NullResolver.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/NullResolver.cpp Tue Aug 28 14:18:05 2018
@@ -14,8 +14,8 @@
namespace llvm {
namespace orc {
-SymbolFlagsMap NullResolver::lookupFlags(const SymbolNameSet &Symbols) {
- return SymbolFlagsMap();
+SymbolNameSet NullResolver::getResponsibilitySet(const SymbolNameSet &Symbols) {
+ return Symbols;
}
SymbolNameSet
Modified: llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/OrcCBindingsStack.h Tue Aug 28 14:18:05 2018
@@ -129,20 +129,21 @@ private:
: Stack(Stack), ExternalResolver(std::move(ExternalResolver)),
ExternalResolverCtx(std::move(ExternalResolverCtx)) {}
- orc::SymbolFlagsMap
- lookupFlags(const orc::SymbolNameSet &Symbols) override {
- orc::SymbolFlagsMap SymbolFlags;
+ orc::SymbolNameSet
+ getResponsibilitySet(const orc::SymbolNameSet &Symbols) override {
+ orc::SymbolNameSet Result;
for (auto &S : Symbols) {
- if (auto Sym = findSymbol(*S))
- SymbolFlags[S] = Sym.getFlags();
- else if (auto Err = Sym.takeError()) {
+ if (auto Sym = findSymbol(*S)) {
+ if (!Sym.getFlags().isStrong())
+ Result.insert(S);
+ } else if (auto Err = Sym.takeError()) {
Stack.reportError(std::move(Err));
- return orc::SymbolFlagsMap();
+ return orc::SymbolNameSet();
}
}
- return SymbolFlags;
+ return Result;
}
orc::SymbolNameSet
Modified: llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/OrcMCJITReplacement.h Tue Aug 28 14:18:05 2018
@@ -144,26 +144,29 @@ class OrcMCJITReplacement : public Execu
public:
LinkingORCResolver(OrcMCJITReplacement &M) : M(M) {}
- SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override {
- SymbolFlagsMap SymbolFlags;
+ SymbolNameSet getResponsibilitySet(const SymbolNameSet &Symbols) override {
+ SymbolNameSet Result;
for (auto &S : Symbols) {
if (auto Sym = M.findMangledSymbol(*S)) {
- SymbolFlags[S] = Sym.getFlags();
+ if (!Sym.getFlags().isStrong())
+ Result.insert(S);
} else if (auto Err = Sym.takeError()) {
M.reportError(std::move(Err));
- return SymbolFlagsMap();
+ return SymbolNameSet();
} else {
if (auto Sym2 = M.ClientResolver->findSymbolInLogicalDylib(*S)) {
- SymbolFlags[S] = Sym2.getFlags();
+ if (!Sym2.getFlags().isStrong())
+ Result.insert(S);
} else if (auto Err = Sym2.takeError()) {
M.reportError(std::move(Err));
- return SymbolFlagsMap();
- }
+ return SymbolNameSet();
+ } else
+ Result.insert(S);
}
}
- return SymbolFlags;
+ return Result;
}
SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
Modified: llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp Tue Aug 28 14:18:05 2018
@@ -44,27 +44,13 @@ public:
return Result;
}
- Expected<LookupFlagsResult> lookupFlags(const LookupSet &Symbols) {
- auto &ES = MR.getTargetJITDylib().getExecutionSession();
+ Expected<LookupSet> getResponsibilitySet(const LookupSet &Symbols) {
+ LookupSet Result;
- SymbolNameSet InternedSymbols;
-
- for (auto &S : Symbols)
- InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
-
- SymbolFlagsMap InternedResult;
- MR.getTargetJITDylib().withSearchOrderDo([&](const JITDylibList &JDs) {
- // An empty search order is pathalogical, but allowed.
- if (JDs.empty())
- return;
-
- assert(JDs.front() && "VSOList entry can not be null");
- InternedResult = JDs.front()->lookupFlags(InternedSymbols);
- });
-
- LookupFlagsResult Result;
- for (auto &KV : InternedResult)
- Result[*KV.first] = std::move(KV.second);
+ for (auto &KV : MR.getSymbols()) {
+ if (Symbols.count(*KV.first))
+ Result.insert(*KV.first);
+ }
return Result;
}
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp Tue Aug 28 14:18:05 2018
@@ -94,16 +94,24 @@ LegacyJITSymbolResolver::lookup(const Lo
/// Performs flags lookup by calling findSymbolInLogicalDylib and
/// returning the flags value for that symbol.
-Expected<JITSymbolResolver::LookupFlagsResult>
-LegacyJITSymbolResolver::lookupFlags(const LookupSet &Symbols) {
- JITSymbolResolver::LookupFlagsResult Result;
+Expected<JITSymbolResolver::LookupSet>
+LegacyJITSymbolResolver::getResponsibilitySet(const LookupSet &Symbols) {
+ JITSymbolResolver::LookupSet Result;
for (auto &Symbol : Symbols) {
std::string SymName = Symbol.str();
- if (auto Sym = findSymbolInLogicalDylib(SymName))
- Result[Symbol] = Sym.getFlags();
- else if (auto Err = Sym.takeError())
+ if (auto Sym = findSymbolInLogicalDylib(SymName)) {
+ // If there's an existing def but it is not strong, then the caller is
+ // responsible for it.
+ if (!Sym.getFlags().isStrong())
+ Result.insert(Symbol);
+ } else if (auto Err = Sym.takeError())
return std::move(Err);
+ else {
+ // If there is no existing definition then the caller is responsible for
+ // it.
+ Result.insert(Symbol);
+ }
}
return std::move(Result);
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Tue Aug 28 14:18:05 2018
@@ -204,7 +204,7 @@ RuntimeDyldImpl::loadObjectImpl(const ob
// First, collect all weak and common symbols. We need to know if stronger
// definitions occur elsewhere.
- JITSymbolResolver::LookupFlagsResult SymbolFlags;
+ JITSymbolResolver::LookupSet ResponsibilitySet;
{
JITSymbolResolver::LookupSet Symbols;
for (auto &Sym : Obj.symbols()) {
@@ -218,10 +218,10 @@ RuntimeDyldImpl::loadObjectImpl(const ob
}
}
- if (auto FlagsResultOrErr = Resolver.lookupFlags(Symbols))
- SymbolFlags = std::move(*FlagsResultOrErr);
+ if (auto ResultOrErr = Resolver.getResponsibilitySet(Symbols))
+ ResponsibilitySet = std::move(*ResultOrErr);
else
- return FlagsResultOrErr.takeError();
+ return ResultOrErr.takeError();
}
// Parse symbols
@@ -259,29 +259,26 @@ RuntimeDyldImpl::loadObjectImpl(const ob
// strong.
if (JITSymFlags->isWeak() || JITSymFlags->isCommon()) {
// First check whether there's already a definition in this instance.
- // FIXME: Override existing weak definitions with strong ones.
if (GlobalSymbolTable.count(Name))
continue;
- // Then check whether we found flags for an existing symbol during the
- // flags lookup earlier.
- auto FlagsI = SymbolFlags.find(Name);
- if (FlagsI == SymbolFlags.end() ||
- (JITSymFlags->isWeak() && !FlagsI->second.isStrong()) ||
- (JITSymFlags->isCommon() && FlagsI->second.isCommon())) {
- if (JITSymFlags->isWeak())
- *JITSymFlags &= ~JITSymbolFlags::Weak;
- if (JITSymFlags->isCommon()) {
- *JITSymFlags &= ~JITSymbolFlags::Common;
- uint32_t Align = I->getAlignment();
- uint64_t Size = I->getCommonSize();
- if (!CommonAlign)
- CommonAlign = Align;
- CommonSize = alignTo(CommonSize, Align) + Size;
- CommonSymbolsToAllocate.push_back(*I);
- }
- } else
+ // If we're not responsible for this symbol, skip it.
+ if (!ResponsibilitySet.count(Name))
continue;
+
+ // Otherwise update the flags on the symbol to make this definition
+ // strong.
+ if (JITSymFlags->isWeak())
+ *JITSymFlags &= ~JITSymbolFlags::Weak;
+ if (JITSymFlags->isCommon()) {
+ *JITSymFlags &= ~JITSymbolFlags::Common;
+ uint32_t Align = I->getAlignment();
+ uint64_t Size = I->getCommonSize();
+ if (!CommonAlign)
+ CommonAlign = Align;
+ CommonSize = alignTo(CommonSize, Align) + Size;
+ CommonSymbolsToAllocate.push_back(*I);
+ }
}
if (Flags & SymbolRef::SF_Absolute &&
Added: llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-1.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-1.ll?rev=340874&view=auto
==============================================================================
--- llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-1.ll (added)
+++ llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-1.ll Tue Aug 28 14:18:05 2018
@@ -0,0 +1 @@
+ at X = weak global i32 0, align 4
Added: llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-2.ll?rev=340874&view=auto
==============================================================================
--- llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-2.ll (added)
+++ llvm/trunk/test/ExecutionEngine/OrcLazy/Inputs/obj-weak-non-materialization-2.ll Tue Aug 28 14:18:05 2018
@@ -0,0 +1,7 @@
+ at X = weak global i32 1, align 4
+
+define void @foo() {
+entry:
+ ret void
+}
+
Added: llvm/trunk/test/ExecutionEngine/OrcLazy/weak-non-materialization.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ExecutionEngine/OrcLazy/weak-non-materialization.ll?rev=340874&view=auto
==============================================================================
--- llvm/trunk/test/ExecutionEngine/OrcLazy/weak-non-materialization.ll (added)
+++ llvm/trunk/test/ExecutionEngine/OrcLazy/weak-non-materialization.ll Tue Aug 28 14:18:05 2018
@@ -0,0 +1,17 @@
+; RUN: llc -filetype=obj -o %t1.o %p/Inputs/obj-weak-non-materialization-1.ll
+; RUN: llc -filetype=obj -o %t2.o %p/Inputs/obj-weak-non-materialization-2.ll
+; RUN: lli -jit-kind=orc-lazy -extra-object %t1.o -extra-object %t2.o %s
+;
+; Check that %t1.o's version of the weak symbol X is used, even though %t2.o is
+; materialized first.
+
+ at X = external global i32
+
+declare void @foo()
+
+define i32 @main(i32 %argc, i8** %argv) {
+entry:
+ call void @foo()
+ %0 = load i32, i32* @X
+ ret i32 %0
+}
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Tue Aug 28 14:18:05 2018
@@ -531,6 +531,57 @@ TEST_F(CoreAPIsStandardTest, AddAndMater
EXPECT_TRUE(OnReadyRun) << "OnReady was not run";
}
+TEST_F(CoreAPIsStandardTest, TestBasicWeakSymbolMaterialization) {
+ // Test that weak symbols are materialized correctly when we look them up.
+ BarSym.setFlags(static_cast<JITSymbolFlags::FlagNames>(BarSym.getFlags() |
+ JITSymbolFlags::Weak));
+
+ bool BarMaterialized = false;
+ auto MU1 = llvm::make_unique<SimpleMaterializationUnit>(
+ SymbolFlagsMap({{Foo, FooSym.getFlags()}, {Bar, BarSym.getFlags()}}),
+ [&](MaterializationResponsibility R) {
+ R.resolve(SymbolMap({{Foo, FooSym}, {Bar, BarSym}})), R.emit();
+ BarMaterialized = true;
+ });
+
+ bool DuplicateBarDiscarded = false;
+ auto MU2 = llvm::make_unique<SimpleMaterializationUnit>(
+ SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
+ [&](MaterializationResponsibility R) {
+ ADD_FAILURE() << "Attempt to materialize Bar from the wrong unit";
+ R.failMaterialization();
+ },
+ [&](const JITDylib &JD, SymbolStringPtr Name) {
+ EXPECT_EQ(Name, Bar) << "Expected \"Bar\" to be discarded";
+ DuplicateBarDiscarded = true;
+ });
+
+ cantFail(JD.define(MU1));
+ cantFail(JD.define(MU2));
+
+ bool OnResolvedRun = false;
+ bool OnReadyRun = false;
+
+ auto OnResolution = [&](Expected<SymbolMap> Result) {
+ cantFail(std::move(Result));
+ OnResolvedRun = true;
+ };
+
+ auto OnReady = [&](Error Err) {
+ cantFail(std::move(Err));
+ OnReadyRun = true;
+ };
+
+ ES.lookup({&JD}, {Bar}, std::move(OnResolution), std::move(OnReady),
+ NoDependenciesToRegister);
+
+ EXPECT_TRUE(OnResolvedRun) << "OnResolved not run";
+ EXPECT_TRUE(OnReadyRun) << "OnReady not run";
+ EXPECT_TRUE(BarMaterialized) << "Bar was not materialized at all";
+ EXPECT_TRUE(DuplicateBarDiscarded)
+ << "Duplicate bar definition not discarded";
+}
+
TEST_F(CoreAPIsStandardTest, DefineMaterializingSymbol) {
bool ExpectNoMoreMaterialization = false;
ES.setDispatchMaterialization(
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp Tue Aug 28 14:18:05 2018
@@ -19,26 +19,31 @@ class LegacyAPIsStandardTest : public Co
namespace {
TEST_F(LegacyAPIsStandardTest, TestLambdaSymbolResolver) {
+ BarSym.setFlags(static_cast<JITSymbolFlags::FlagNames>(BarSym.getFlags() |
+ JITSymbolFlags::Weak));
+
cantFail(JD.define(absoluteSymbols({{Foo, FooSym}, {Bar, BarSym}})));
auto Resolver = createSymbolResolver(
- [&](const SymbolNameSet &Symbols) { return JD.lookupFlags(Symbols); },
+ [&](const SymbolNameSet &Symbols) {
+ auto FlagsMap = JD.lookupFlags(Symbols);
+ llvm::dbgs() << "FlagsMap is " << FlagsMap << "\n";
+ SymbolNameSet Result;
+ for (auto &KV : FlagsMap)
+ if (!KV.second.isStrong())
+ Result.insert(KV.first);
+ return Result;
+ },
[&](std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Symbols) {
return JD.legacyLookup(std::move(Q), Symbols);
});
- SymbolNameSet Symbols({Foo, Bar, Baz});
-
- SymbolFlagsMap SymbolFlags = Resolver->lookupFlags(Symbols);
+ auto RS = Resolver->getResponsibilitySet(SymbolNameSet({Bar, Baz}));
- EXPECT_EQ(SymbolFlags.size(), 2U)
- << "lookupFlags returned the wrong number of results";
- 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(SymbolFlags[Bar], BarSym.getFlags())
- << "Incorrect lookupFlags result for Bar";
+ EXPECT_EQ(RS.size(), 1U)
+ << "getResponsibilitySet returned the wrong number of results";
+ EXPECT_EQ(RS.count(Bar), 1U)
+ << "getResponsibilitySet result incorrect. Should be {'bar'}";
bool OnResolvedRun = false;
@@ -59,68 +64,22 @@ TEST_F(LegacyAPIsStandardTest, TestLambd
auto Q = std::make_shared<AsynchronousSymbolQuery>(SymbolNameSet({Foo, Bar}),
OnResolved, OnReady);
- auto Unresolved = Resolver->lookup(std::move(Q), Symbols);
+ auto Unresolved =
+ Resolver->lookup(std::move(Q), SymbolNameSet({Foo, Bar, Baz}));
EXPECT_EQ(Unresolved.size(), 1U) << "Expected one unresolved symbol";
EXPECT_EQ(Unresolved.count(Baz), 1U) << "Expected baz to not be resolved";
EXPECT_TRUE(OnResolvedRun) << "OnResolved was never run";
}
-TEST(LegacyAPIInteropTest, QueryAgainstJITDylib) {
-
- ExecutionSession ES(std::make_shared<SymbolStringPool>());
- auto Foo = ES.getSymbolStringPool().intern("foo");
-
- auto &JD = ES.createJITDylib("JD");
- JITEvaluatedSymbol FooSym(0xdeadbeef, JITSymbolFlags::Exported);
- cantFail(JD.define(absoluteSymbols({{Foo, FooSym}})));
-
- auto LookupFlags = [&](const SymbolNameSet &Names) {
- return JD.lookupFlags(Names);
- };
-
- auto Lookup = [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
- SymbolNameSet Symbols) {
- return JD.legacyLookup(std::move(Query), Symbols);
- };
-
- auto UnderlyingResolver =
- createSymbolResolver(std::move(LookupFlags), std::move(Lookup));
- JITSymbolResolverAdapter Resolver(ES, *UnderlyingResolver, nullptr);
-
- JITSymbolResolver::LookupSet Names{StringRef("foo")};
-
- auto LFR = Resolver.lookupFlags(Names);
- EXPECT_TRUE(!!LFR) << "lookupFlags failed";
- EXPECT_EQ(LFR->size(), 1U)
- << "lookupFlags returned the wrong number of results";
- EXPECT_EQ(LFR->count(*Foo), 1U)
- << "lookupFlags did not contain a result for 'foo'";
- EXPECT_EQ((*LFR)[*Foo], FooSym.getFlags())
- << "lookupFlags contained the wrong result for 'foo'";
-
- auto LR = Resolver.lookup(Names);
- EXPECT_TRUE(!!LR) << "lookup failed";
- EXPECT_EQ(LR->size(), 1U) << "lookup returned the wrong number of results";
- EXPECT_EQ(LR->count(*Foo), 1U) << "lookup did not contain a result for 'foo'";
- EXPECT_EQ((*LR)[*Foo].getFlags(), FooSym.getFlags())
- << "lookup returned the wrong result for flags of 'foo'";
- EXPECT_EQ((*LR)[*Foo].getAddress(), FooSym.getAddress())
- << "lookup returned the wrong result for address of 'foo'";
-}
-
-TEST(LegacyAPIInteropTset, LegacyLookupHelpersFn) {
- constexpr JITTargetAddress FooAddr = 0xdeadbeef;
- JITSymbolFlags FooFlags = JITSymbolFlags::Exported;
-
+TEST_F(LegacyAPIsStandardTest, LegacyLookupHelpersFn) {
bool BarMaterialized = false;
- constexpr JITTargetAddress BarAddr = 0xcafef00d;
- JITSymbolFlags BarFlags = static_cast<JITSymbolFlags::FlagNames>(
- JITSymbolFlags::Exported | JITSymbolFlags::Weak);
+ BarSym.setFlags(static_cast<JITSymbolFlags::FlagNames>(BarSym.getFlags() |
+ JITSymbolFlags::Weak));
auto LegacyLookup = [&](const std::string &Name) -> JITSymbol {
if (Name == "foo")
- return {FooAddr, FooFlags};
+ return FooSym;
if (Name == "bar") {
auto BarMaterializer = [&]() -> Expected<JITTargetAddress> {
@@ -128,27 +87,18 @@ TEST(LegacyAPIInteropTset, LegacyLookupH
return BarAddr;
};
- return {BarMaterializer, BarFlags};
+ return {BarMaterializer, BarSym.getFlags()};
}
return nullptr;
};
- ExecutionSession ES;
- auto Foo = ES.getSymbolStringPool().intern("foo");
- auto Bar = ES.getSymbolStringPool().intern("bar");
- auto Baz = ES.getSymbolStringPool().intern("baz");
-
- SymbolNameSet Symbols({Foo, Bar, Baz});
-
- auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
-
- EXPECT_TRUE(!!SymbolFlags) << "Expected lookupFlagsWithLegacyFn to succeed";
- 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";
+ auto RS =
+ getResponsibilitySetWithLegacyFn(SymbolNameSet({Bar, Baz}), LegacyLookup);
+
+ EXPECT_TRUE(!!RS) << "Expected getResponsibilitySetWithLegacyFn to succeed";
+ EXPECT_EQ(RS->size(), 1U) << "Wrong number of symbols returned";
+ EXPECT_EQ(RS->count(Bar), 1U) << "Incorrect responsibility set returned";
EXPECT_FALSE(BarMaterialized)
<< "lookupFlags should not have materialized bar";
@@ -162,9 +112,11 @@ TEST(LegacyAPIInteropTset, LegacyLookupH
EXPECT_EQ(Result->count(Foo), 1U) << "Result for foo missing";
EXPECT_EQ(Result->count(Bar), 1U) << "Result for bar missing";
EXPECT_EQ((*Result)[Foo].getAddress(), FooAddr) << "Wrong address for foo";
- EXPECT_EQ((*Result)[Foo].getFlags(), FooFlags) << "Wrong flags for foo";
+ EXPECT_EQ((*Result)[Foo].getFlags(), FooSym.getFlags())
+ << "Wrong flags for foo";
EXPECT_EQ((*Result)[Bar].getAddress(), BarAddr) << "Wrong address for bar";
- EXPECT_EQ((*Result)[Bar].getFlags(), BarFlags) << "Wrong flags for bar";
+ EXPECT_EQ((*Result)[Bar].getFlags(), BarSym.getFlags())
+ << "Wrong flags for bar";
};
auto OnReady = [&](Error Err) {
EXPECT_FALSE(!!Err) << "Finalization unexpectedly failed";
@@ -172,7 +124,8 @@ TEST(LegacyAPIInteropTset, LegacyLookupH
};
AsynchronousSymbolQuery Q({Foo, Bar}, OnResolved, OnReady);
- auto Unresolved = lookupWithLegacyFn(ES, Q, Symbols, LegacyLookup);
+ auto Unresolved =
+ lookupWithLegacyFn(ES, Q, SymbolNameSet({Foo, Bar, Baz}), LegacyLookup);
EXPECT_TRUE(OnResolvedRun) << "OnResolved was not run";
EXPECT_TRUE(OnReadyRun) << "OnReady was not run";
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp?rev=340874&r1=340873&r2=340874&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp Tue Aug 28 14:18:05 2018
@@ -185,7 +185,8 @@ TEST_F(RTDyldObjectLinkingLayerExecution
Resolvers[K2] = createSymbolResolver(
[&](const SymbolNameSet &Symbols) {
- return cantFail(lookupFlagsWithLegacyFn(Symbols, LegacyLookup));
+ return cantFail(
+ getResponsibilitySetWithLegacyFn(Symbols, LegacyLookup));
},
[&](std::shared_ptr<AsynchronousSymbolQuery> Query,
const SymbolNameSet &Symbols) {
More information about the llvm-commits
mailing list