Re-apply "[ORC] Track all dependencies on symbols that aren't..." with fixes.
This reapplies427fb5cc5a, which was reverted in08c1a6b3e1due to bot failures. The fix was to remove an incorrect assertion: In IL_emit, during the initial worklist loop, an EDU can have all of its dependencies removed without becoming ready (because it may still have implicit dependencies that will be added back during the subsequent propagateExtraEmitDeps operation). The EDU will be marked Ready at the end of IL_emit if its Dependencies set is empty at that point. Prior to that we can only assert that it's either Emitted or Ready (which is already covered by other assertions).
This commit is contained in:
@@ -938,7 +938,6 @@ Error JITDylib::resolve(MaterializationResponsibility &MR,
|
||||
auto &MI = MII->second;
|
||||
for (auto &Q : MI.takeQueriesMeeting(SymbolState::Resolved)) {
|
||||
Q->notifySymbolMetRequiredState(Name, ResolvedSym);
|
||||
Q->removeQueryDependence(*this, Name);
|
||||
if (Q->isComplete())
|
||||
CompletedQueries.insert(std::move(Q));
|
||||
}
|
||||
@@ -1207,9 +1206,8 @@ void JITDylib::MaterializingInfo::removeQuery(
|
||||
PendingQueries, [&Q](const std::shared_ptr<AsynchronousSymbolQuery> &V) {
|
||||
return V.get() == &Q;
|
||||
});
|
||||
assert(I != PendingQueries.end() &&
|
||||
"Query is not attached to this MaterializingInfo");
|
||||
PendingQueries.erase(I);
|
||||
if (I != PendingQueries.end())
|
||||
PendingQueries.erase(I);
|
||||
}
|
||||
|
||||
JITDylib::AsynchronousSymbolQueryList
|
||||
@@ -2615,6 +2613,12 @@ void ExecutionSession::OL_completeLookup(
|
||||
LLVM_DEBUG(dbgs()
|
||||
<< "matched, symbol already in required state\n");
|
||||
Q->notifySymbolMetRequiredState(Name, SymI->second.getSymbol());
|
||||
|
||||
// If this symbol is in anything other than the Ready state then
|
||||
// we need to track the dependence.
|
||||
if (SymI->second.getState() != SymbolState::Ready)
|
||||
Q->addQueryDependence(JD, Name);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -3165,7 +3169,6 @@ void ExecutionSession::IL_makeEDUEmitted(
|
||||
Q->notifySymbolMetRequiredState(SymbolStringPtr(Sym), Entry.getSymbol());
|
||||
if (Q->isComplete())
|
||||
Queries.insert(Q);
|
||||
Q->removeQueryDependence(JD, SymbolStringPtr(Sym));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3317,8 +3320,6 @@ ExecutionSession::IL_emit(MaterializationResponsibility &MR,
|
||||
auto &DepMI = DepJD->MaterializingInfos[SymbolStringPtr(Dep)];
|
||||
assert(DepMI.DefiningEDU &&
|
||||
"Emitted symbol does not have a defining EDU");
|
||||
assert(!DepMI.DefiningEDU->Dependencies.empty() &&
|
||||
"Emitted symbol has empty dependencies (should be ready)");
|
||||
assert(DepMI.DependantEDUs.empty() &&
|
||||
"Already-emitted symbol has dependant EDUs?");
|
||||
auto &DepEDUInfo = EDUInfos[DepMI.DefiningEDU.get()];
|
||||
|
||||
@@ -518,6 +518,71 @@ TEST_F(CoreAPIsStandardTest, TestTrivialCircularDependency) {
|
||||
<< "Self-dependency prevented symbol from being marked ready";
|
||||
}
|
||||
|
||||
TEST_F(CoreAPIsStandardTest, TestBasicQueryDependenciesReporting) {
|
||||
// Test that dependencies are reported as expected.
|
||||
|
||||
bool DependenciesCallbackRan = false;
|
||||
|
||||
std::unique_ptr<MaterializationResponsibility> FooR;
|
||||
std::unique_ptr<MaterializationResponsibility> BarR;
|
||||
|
||||
cantFail(JD.define(std::make_unique<SimpleMaterializationUnit>(
|
||||
SymbolFlagsMap({{Foo, FooSym.getFlags()}}),
|
||||
[&](std::unique_ptr<MaterializationResponsibility> R) {
|
||||
FooR = std::move(R);
|
||||
})));
|
||||
|
||||
cantFail(JD.define(std::make_unique<SimpleMaterializationUnit>(
|
||||
SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
|
||||
[&](std::unique_ptr<MaterializationResponsibility> R) {
|
||||
BarR = std::move(R);
|
||||
})));
|
||||
|
||||
cantFail(JD.define(std::make_unique<SimpleMaterializationUnit>(
|
||||
SymbolFlagsMap({{Baz, BazSym.getFlags()}}),
|
||||
[&](std::unique_ptr<MaterializationResponsibility> R) {
|
||||
cantFail(R->notifyResolved({{Baz, BazSym}}));
|
||||
cantFail(R->notifyEmitted({}));
|
||||
})));
|
||||
|
||||
// First issue a lookup for Foo and Bar so that we can put them
|
||||
// into the required states for the test lookup below.
|
||||
ES.lookup(
|
||||
LookupKind::Static, makeJITDylibSearchOrder(&JD),
|
||||
SymbolLookupSet({Foo, Bar}), SymbolState::Resolved,
|
||||
[](Expected<SymbolMap> Result) {
|
||||
EXPECT_THAT_EXPECTED(std::move(Result), Succeeded());
|
||||
},
|
||||
NoDependenciesToRegister);
|
||||
|
||||
cantFail(FooR->notifyResolved({{Foo, FooSym}}));
|
||||
cantFail(FooR->notifyEmitted({}));
|
||||
|
||||
cantFail(BarR->notifyResolved({{Bar, BarSym}}));
|
||||
|
||||
ES.lookup(
|
||||
LookupKind::Static, makeJITDylibSearchOrder(&JD),
|
||||
SymbolLookupSet({Foo, Bar, Baz}), SymbolState::Resolved,
|
||||
[](Expected<SymbolMap> Result) {
|
||||
EXPECT_THAT_EXPECTED(std::move(Result), Succeeded());
|
||||
},
|
||||
[&](const SymbolDependenceMap &Dependencies) {
|
||||
EXPECT_EQ(Dependencies.size(), 1U)
|
||||
<< "Expect dependencies on only one JITDylib";
|
||||
EXPECT_TRUE(Dependencies.count(&JD))
|
||||
<< "Expect dependencies on JD only";
|
||||
auto &Deps = Dependencies.begin()->second;
|
||||
EXPECT_EQ(Deps.size(), 2U);
|
||||
EXPECT_TRUE(Deps.count(Bar));
|
||||
EXPECT_TRUE(Deps.count(Baz));
|
||||
DependenciesCallbackRan = true;
|
||||
});
|
||||
|
||||
cantFail(BarR->notifyEmitted({}));
|
||||
|
||||
EXPECT_TRUE(DependenciesCallbackRan);
|
||||
}
|
||||
|
||||
TEST_F(CoreAPIsStandardTest, TestCircularDependenceInOneJITDylib) {
|
||||
// Test that a circular symbol dependency between three symbols in a JITDylib
|
||||
// does not prevent any symbol from becoming 'ready' once all symbols are
|
||||
|
||||
Reference in New Issue
Block a user