From d38d06e6493fa6bc77d54589960007d7a91c8096 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Thu, 26 Mar 2020 13:06:13 -0700 Subject: [PATCH] [ORC] Don't create MaterializingInfo entries unnecessarily. --- llvm/lib/ExecutionEngine/Orc/Core.cpp | 18 +++++++++++++----- .../ExecutionEngine/Orc/CoreAPIsTest.cpp | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp index 552a7f2ab4f6..9ba0d7d23167 100644 --- a/llvm/lib/ExecutionEngine/Orc/Core.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp @@ -759,8 +759,6 @@ void JITDylib::addDependencies(const SymbolStringPtr &Name, // If the dependency was not in the error state then add it to // our list of dependencies. - assert(OtherJITDylib.MaterializingInfos.count(OtherSymbol) && - "No MaterializingInfo for dependency"); auto &OtherMI = OtherJITDylib.MaterializingInfos[OtherSymbol]; if (OtherSymEntry.getState() == SymbolState::Emitted) @@ -841,7 +839,11 @@ Error JITDylib::resolve(const SymbolMap &Resolved) { SymI->second.setFlags(ResolvedFlags); SymI->second.setState(SymbolState::Resolved); - auto &MI = MaterializingInfos[Name]; + auto MII = MaterializingInfos.find(Name); + if (MII == MaterializingInfos.end()) + continue; + + auto &MI = MII->second; for (auto &Q : MI.takeQueriesMeeting(SymbolState::Resolved)) { Q->notifySymbolMetRequiredState(Name, ResolvedSym); Q->removeQueryDependence(*this, Name); @@ -909,8 +911,14 @@ Error JITDylib::emit(const SymbolFlagsMap &Emitted) { SymEntry.setState(SymbolState::Emitted); auto MII = MaterializingInfos.find(Name); - assert(MII != MaterializingInfos.end() && - "Missing MaterializingInfo entry"); + + // If this symbol has no MaterializingInfo then it's trivially ready. + // Update its state and continue. + if (MII == MaterializingInfos.end()) { + SymEntry.setState(SymbolState::Ready); + continue; + } + auto &MI = MII->second; // For each dependant, transfer this node's emitted dependencies to diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp index ef97f2cd40a4..65ab0cd7ba13 100644 --- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp +++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp @@ -91,6 +91,25 @@ TEST_F(CoreAPIsStandardTest, EmptyLookup) { EXPECT_TRUE(OnCompletionRun) << "OnCompletion was not run for empty query"; } +TEST_F(CoreAPIsStandardTest, ResolveUnrequestedSymbol) { + // Test that all symbols in a MaterializationUnit materialize corretly when + // only a subset of symbols is looked up. + // The aim here is to ensure that we're not relying on the query to set up + // state needed to materialize the unrequested symbols. + + cantFail(JD.define(std::make_unique( + SymbolFlagsMap({{Foo, FooSym.getFlags()}, {Bar, BarSym.getFlags()}}), + [this](MaterializationResponsibility R) { + cantFail(R.notifyResolved({{Foo, FooSym}, {Bar, BarSym}})); + cantFail(R.notifyEmitted()); + }))); + + auto Result = + cantFail(ES.lookup(makeJITDylibSearchOrder(&JD), SymbolLookupSet({Foo}))); + EXPECT_EQ(Result.size(), 1U) << "Unexpected number of results"; + EXPECT_TRUE(Result.count(Foo)) << "Expected result for \"Foo\""; +} + TEST_F(CoreAPIsStandardTest, RemoveSymbolsTest) { // Test that: // (1) Missing symbols generate a SymbolsNotFound error.