Skip to content

Commit

Permalink
Revert "[ORC] Track all dependencies on symbols that aren't Ready yet."
Browse files Browse the repository at this point in the history
This reverts commit 427fb5c while I investigate
the bot failure in https://lab.llvm.org/buildbot/#/builders/95/builds/6835.
  • Loading branch information
lhames committed Dec 2, 2024
1 parent dd0d956 commit 08c1a6b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 73 deletions.
13 changes: 5 additions & 8 deletions llvm/lib/ExecutionEngine/Orc/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ 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));
}
Expand Down Expand Up @@ -1206,8 +1207,9 @@ void JITDylib::MaterializingInfo::removeQuery(
PendingQueries, [&Q](const std::shared_ptr<AsynchronousSymbolQuery> &V) {
return V.get() == &Q;
});
if (I != PendingQueries.end())
PendingQueries.erase(I);
assert(I != PendingQueries.end() &&
"Query is not attached to this MaterializingInfo");
PendingQueries.erase(I);
}

JITDylib::AsynchronousSymbolQueryList
Expand Down Expand Up @@ -2613,12 +2615,6 @@ 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;
}

Expand Down Expand Up @@ -3169,6 +3165,7 @@ void ExecutionSession::IL_makeEDUEmitted(
Q->notifySymbolMetRequiredState(SymbolStringPtr(Sym), Entry.getSymbol());
if (Q->isComplete())
Queries.insert(Q);
Q->removeQueryDependence(JD, SymbolStringPtr(Sym));
}
}

Expand Down
65 changes: 0 additions & 65 deletions llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,71 +518,6 @@ 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
Expand Down

0 comments on commit 08c1a6b

Please sign in to comment.