diff --git a/llvm/examples/ThinLtoJIT/OnDemandLayer.cpp b/llvm/examples/ThinLtoJIT/OnDemandLayer.cpp --- a/llvm/examples/ThinLtoJIT/OnDemandLayer.cpp +++ b/llvm/examples/ThinLtoJIT/OnDemandLayer.cpp @@ -159,9 +159,24 @@ << R.getSymbols() << "\n"); #endif - // TODO: Take responsibility for all symbols in the module. They should - // replace all synthetic weak definitions inherited from the - // EmitCallThroughStubs unit. + // Take responsibility for all symbols in the module. In the JITDylib, this + // will replace the synthetic weak definitions inherited from the + // EmitCallThroughStubs unit with the definitions parsed from the module. + // + // Note: The MaterializationResponsibility will keep the old entries, because + // it implicitly drops duplicates upon insert(). + // + // TODO: What happens if the module naturally contains weak definitions? + // So far defineMaterializing() would reject the incoming definition + // and incorrectly keep the existing one. It seems acceptable for now as + // we only replace their flags values under the hood. If we started + // recording other properties for symbols, this would probably break. + // Introducing a 'synthetic' flag may help to fix the ambiguity. + // + if (Error Err = R.defineMaterializing(std::move(ModuleSymbols))) { + Parent.getExecutionSession().reportError(std::move(Err)); + return; + } static constexpr bool SubmittedAsynchronously = false; Parent.emitWholeModule(std::move(R), std::move(TSM), SubmittedAsynchronously); @@ -296,15 +311,23 @@ Callables[Name] = SymbolAliasMapEntry(Name, Flags); } - // TODO: Create a materialization unit that will load and emit the module. The - // reexports that we generate in the end will issue a query on call-through, - // which should trigger the actual materialization. + // Create a materialization unit that will load and emit the whole module and + // lodge it with the shadow dylib. The reexports that we generate in the end + // will issue a query into the shadow dylib on call-through. This will trigger + // the actual materialization. + JITDylibResources &JDR = getJITDylibResources(R.getTargetJITDylib()); + if (auto Err = JDR.ShadowJD->define( + std::make_unique( + R.getSymbols(), ModulePath, R.getVModuleKey(), *this))) { + getExecutionSession().reportError(std::move(Err)); + R.failMaterialization(); + return; + } // No need to track function aliasees. constexpr ImplSymbolMap *NoAliaseeImpls = nullptr; // Emit call-through symbols to main dylib. - JITDylibResources &JDR = getJITDylibResources(R.getTargetJITDylib()); R.replace(lazyReexports(*CallThroughManager, *JDR.ISManager, *JDR.ShadowJD, std::move(Callables), NoAliaseeImpls)); @@ -339,15 +362,24 @@ NonCallables[Name] = SymbolAliasMapEntry(Name, Flags); } - // TODO: Create a materialization unit that will emit the whole module. The - // reexports that we generate in the end will issue a query on call-through, - // which should trigger the actual materialization. + // Create a materialization unit that will emit the whole module and lodge it + // with the shadow dylib. The reexports that we generate in the end will issue + // a query into the shadow dylib on call-through. This will trigger the actual + // materialization. + JITDylib &JD = R.getTargetJITDylib(); + static constexpr bool SubmittedFromDiscovery = false; + if (auto Err = addWholeModule(JD, std::move(TSM), R.getVModuleKey(), + SubmittedFromDiscovery)) { + getExecutionSession().reportError(std::move(Err)); + R.failMaterialization(); + return; + } PSTATS("[", ModulePath , "] Start emitting reexports"); // No need to track function aliasees. constexpr ImplSymbolMap *NoAliaseeImpls = nullptr; - JITDylibResources &JDR = getJITDylibResources(R.getTargetJITDylib()); + JITDylibResources &JDR = getJITDylibResources(JD); R.replace(reexports(*JDR.ShadowJD, std::move(NonCallables), JITDylibLookupFlags::MatchAllSymbols)); diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp --- a/llvm/lib/ExecutionEngine/Orc/Core.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp @@ -832,23 +832,33 @@ // If the entry already exists... if (EntryItr != Symbols.end()) { + if (Flags.isWeak()) { + // The incoming symbol definition is weak. + RejectedWeakDefs.push_back(SFItr); + continue; + } - // If this is a strong definition then error out. - if (!Flags.isWeak()) { - // Remove any symbols already added. + if (EntryItr->second.getFlags().isWeak()) { + // The existing symbol definition is weak. + if (!EntryItr->second.isInMaterializationPhase()) { + auto UMII = UnmaterializedInfos.find(Name); + assert(UMII != UnmaterializedInfos.end() && + "Overridden existing def should have an UnmaterializedInfo"); + UMII->second->MU->doDiscard(*this, Name); + } + Symbols.erase(EntryItr); + } else { + // Both definitions are strong. Remove any symbols already added. for (auto &SI : AddedSyms) Symbols.erase(SI); // FIXME: Return all duplicates. return make_error(std::string(*Name)); } + } - // Otherwise just make a note to discard this symbol after the loop. - RejectedWeakDefs.push_back(SFItr); - continue; - } else - EntryItr = - Symbols.insert(std::make_pair(Name, SymbolTableEntry(Flags))).first; + EntryItr = + Symbols.insert(std::make_pair(Name, SymbolTableEntry(Flags))).first; AddedSyms.push_back(EntryItr); EntryItr->second.setState(SymbolState::Materializing); @@ -1837,24 +1847,24 @@ Error JITDylib::defineImpl(MaterializationUnit &MU) { SymbolNameSet Duplicates; std::vector ExistingDefsOverridden; - std::vector MUDefsOverridden; + std::vector RejectedWeakDefs; for (const auto &KV : MU.getSymbols()) { auto I = Symbols.find(KV.first); if (I != Symbols.end()) { - if (KV.second.isStrong()) { - if (I->second.getFlags().isStrong() || - I->second.getState() > SymbolState::NeverSearched) - Duplicates.insert(KV.first); - else { - assert(I->second.getState() == SymbolState::NeverSearched && - "Overridden existing def should be in the never-searched " - "state"); - ExistingDefsOverridden.push_back(KV.first); - } - } else - MUDefsOverridden.push_back(KV.first); + if (KV.second.isWeak()) { + // The incoming symbol definition is weak. + RejectedWeakDefs.push_back(KV.first); + continue; + } else if (I->second.getFlags().isWeak() && + I->second.getState() == SymbolState::NeverSearched) { + // The existing symbol definition is weak and can be overridden. + ExistingDefsOverridden.push_back(KV.first); + } else { + // Both definitions are strong. + Duplicates.insert(KV.first); + } } } @@ -1863,7 +1873,7 @@ return make_error(std::string(**Duplicates.begin())); // Discard any overridden defs in this MU. - for (auto &S : MUDefsOverridden) + for (auto &S : RejectedWeakDefs) MU.doDiscard(*this, S); // Discard existing overridden defs.