llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lto Author: Andrew Ng (nga888) <details> <summary>Changes</summary> Backport the following commits to `release/21.x`: 0341fb63: [ThinLTO] Avoid creating map entries on lookup (NFCI) (#<!-- -->164873) 564c3de6: [ThinLTO][NFC] Improve performance of `addThinLTO` (#<!-- -->166067) --- Full diff: https://github.com/llvm/llvm-project/pull/166409.diff 2 Files Affected: - (modified) llvm/include/llvm/LTO/LTO.h (+13) - (modified) llvm/lib/LTO/LTO.cpp (+19-23) ``````````diff diff --git a/llvm/include/llvm/LTO/LTO.h b/llvm/include/llvm/LTO/LTO.h index d8e632b5a49d5..d5e7d2ede7e9b 100644 --- a/llvm/include/llvm/LTO/LTO.h +++ b/llvm/include/llvm/LTO/LTO.h @@ -464,6 +464,19 @@ class LTO { ModuleMapType ModuleMap; // The bitcode modules to compile, if specified by the LTO Config. std::optional<ModuleMapType> ModulesToCompile; + + void setPrevailingModuleForGUID(GlobalValue::GUID GUID, StringRef Module) { + PrevailingModuleForGUID[GUID] = Module; + } + bool isPrevailingModuleForGUID(GlobalValue::GUID GUID, + StringRef Module) const { + auto It = PrevailingModuleForGUID.find(GUID); + return It != PrevailingModuleForGUID.end() && It->second == Module; + } + + private: + // Make this private so all accesses must go through above accessor methods + // to avoid inadvertently creating new entries on lookups. DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID; } ThinLTO; diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 73e79c08a56ca..3c6951d8ec5fe 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -1035,63 +1035,59 @@ Error LTO::linkRegularLTO(RegularLTOState::AddedModule Mod, Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms, const SymbolResolution *&ResI, const SymbolResolution *ResE) { + const auto BMID = BM.getModuleIdentifier(); const SymbolResolution *ResITmp = ResI; for (const InputFile::Symbol &Sym : Syms) { assert(ResITmp != ResE); SymbolResolution Res = *ResITmp++; - if (!Sym.getIRName().empty()) { + if (!Sym.getIRName().empty() && Res.Prevailing) { auto GUID = GlobalValue::getGUIDAssumingExternalLinkage( GlobalValue::getGlobalIdentifier(Sym.getIRName(), GlobalValue::ExternalLinkage, "")); - if (Res.Prevailing) - ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier(); + ThinLTO.setPrevailingModuleForGUID(GUID, BMID); } } - if (Error Err = - BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(), - [&](GlobalValue::GUID GUID) { - return ThinLTO.PrevailingModuleForGUID[GUID] == - BM.getModuleIdentifier(); - })) + if (Error Err = BM.readSummary( + ThinLTO.CombinedIndex, BMID, [&](GlobalValue::GUID GUID) { + return ThinLTO.isPrevailingModuleForGUID(GUID, BMID); + })) return Err; - LLVM_DEBUG(dbgs() << "Module " << BM.getModuleIdentifier() << "\n"); + LLVM_DEBUG(dbgs() << "Module " << BMID << "\n"); for (const InputFile::Symbol &Sym : Syms) { assert(ResI != ResE); SymbolResolution Res = *ResI++; - if (!Sym.getIRName().empty()) { + if (!Sym.getIRName().empty() && + (Res.Prevailing || Res.FinalDefinitionInLinkageUnit)) { auto GUID = GlobalValue::getGUIDAssumingExternalLinkage( GlobalValue::getGlobalIdentifier(Sym.getIRName(), GlobalValue::ExternalLinkage, "")); if (Res.Prevailing) { - assert(ThinLTO.PrevailingModuleForGUID[GUID] == - BM.getModuleIdentifier()); + assert(ThinLTO.isPrevailingModuleForGUID(GUID, BMID)); // For linker redefined symbols (via --wrap or --defsym) we want to // switch the linkage to `weak` to prevent IPOs from happening. // Find the summary in the module for this very GV and record the new // linkage so that we can switch it when we import the GV. if (Res.LinkerRedefined) - if (auto S = ThinLTO.CombinedIndex.findSummaryInModule( - GUID, BM.getModuleIdentifier())) + if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(GUID, BMID)) S->setLinkage(GlobalValue::WeakAnyLinkage); } // If the linker resolved the symbol to a local definition then mark it // as local in the summary for the module we are adding. if (Res.FinalDefinitionInLinkageUnit) { - if (auto S = ThinLTO.CombinedIndex.findSummaryInModule( - GUID, BM.getModuleIdentifier())) { + if (auto S = ThinLTO.CombinedIndex.findSummaryInModule(GUID, BMID)) { S->setDSOLocal(true); } } } } - if (!ThinLTO.ModuleMap.insert({BM.getModuleIdentifier(), BM}).second) + if (!ThinLTO.ModuleMap.insert({BMID, BM}).second) return make_error<StringError>( "Expected at most one ThinLTO module per bitcode file", inconvertibleErrorCode()); @@ -1102,10 +1098,10 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms, // This is a fuzzy name matching where only modules with name containing the // specified switch values are going to be compiled. for (const std::string &Name : Conf.ThinLTOModulesToCompile) { - if (BM.getModuleIdentifier().contains(Name)) { - ThinLTO.ModulesToCompile->insert({BM.getModuleIdentifier(), BM}); - LLVM_DEBUG(dbgs() << "[ThinLTO] Selecting " << BM.getModuleIdentifier() - << " to compile\n"); + if (BMID.contains(Name)) { + ThinLTO.ModulesToCompile->insert({BMID, BM}); + LLVM_DEBUG(dbgs() << "[ThinLTO] Selecting " << BMID << " to compile\n"); + break; } } } @@ -1974,7 +1970,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache, LocalWPDTargetsMap); auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) { - return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath(); + return ThinLTO.isPrevailingModuleForGUID(GUID, S->modulePath()); }; if (EnableMemProfContextDisambiguation) { MemProfContextDisambiguation ContextDisambiguation; `````````` </details> https://github.com/llvm/llvm-project/pull/166409 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
