https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/88024
>From cfb63d775d43a28b560d938346f1dd4b2dddc765 Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Thu, 4 Apr 2024 11:54:17 -0700 Subject: [PATCH 1/2] function import changes --- llvm/include/llvm/IR/ModuleSummaryIndex.h | 24 ++++ .../llvm/Transforms/IPO/FunctionImport.h | 18 ++- llvm/lib/LTO/LTO.cpp | 13 +- llvm/lib/LTO/LTOBackend.cpp | 5 +- llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 9 +- llvm/lib/Transforms/IPO/FunctionImport.cpp | 130 ++++++++++++------ llvm/tools/llvm-link/llvm-link.cpp | 2 +- 7 files changed, 146 insertions(+), 55 deletions(-) diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h index 286b51bda0e2c1..259fe56ce5f63e 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -296,6 +296,30 @@ template <> struct DenseMapInfo<ValueInfo> { static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.getRef(); } }; +struct SummaryImportInfo { + enum class ImportType : uint8_t { + NotImported = 0, + Declaration = 1, + Definition = 2, + }; + unsigned Type : 3; + SummaryImportInfo() : Type(static_cast<unsigned>(ImportType::NotImported)) {} + SummaryImportInfo(ImportType Type) : Type(static_cast<unsigned>(Type)) {} + + // FIXME: delete the first two set* helper function. + void updateType(ImportType InputType) { + Type = std::max(Type, static_cast<unsigned>(InputType)); + } + + bool isDefinition() const { + return static_cast<ImportType>(Type) == ImportType::Definition; + } + + bool isDeclaration() const { + return static_cast<ImportType>(Type) == ImportType::Declaration; + } +}; + /// Summary of memprof callsite metadata. struct CallsiteInfo { // Actual callee function. diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h index c4d19e8641eca2..9adc0c31eed439 100644 --- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h +++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h @@ -33,7 +33,14 @@ class FunctionImporter { public: /// Set of functions to import from a source module. Each entry is a set /// containing all the GUIDs of all functions to import for a source module. - using FunctionsToImportTy = std::unordered_set<GlobalValue::GUID>; + using FunctionsToImportTy = DenseMap<GlobalValue::GUID, SummaryImportInfo>; + + // FIXME: Remove this. + enum ImportStatus { + NotImported, + ImportDeclaration, + ImportDefinition, + }; /// The different reasons selectCallee will chose not to import a /// candidate. @@ -99,8 +106,10 @@ class FunctionImporter { /// index's module path string table). using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>; - /// The set contains an entry for every global value the module exports. - using ExportSetTy = DenseSet<ValueInfo>; + /// The map contains an entry for every global value the module exports, the + /// key being the value info, and the value is the summary-based import info. + /// FIXME: Does this set need to be a map? + using ExportSetTy = DenseMap<ValueInfo, SummaryImportInfo>; /// A function of this type is used to load modules referenced by the index. using ModuleLoaderTy = @@ -211,7 +220,8 @@ void gatherImportedSummariesForModule( StringRef ModulePath, const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries, const FunctionImporter::ImportMapTy &ImportList, - std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex); + std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex, + ModuleToGVSummaryPtrSet &ModuleToDecSummaries); /// Emit into \p OutputFilename the files module \p ModulePath will import from. std::error_code EmitImportsFiles( diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 53060df7f503e0..ace533fe28c92f 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -159,7 +159,7 @@ void llvm::computeLTOCacheKey( std::vector<uint64_t> ExportsGUID; ExportsGUID.reserve(ExportList.size()); for (const auto &VI : ExportList) { - auto GUID = VI.getGUID(); + auto GUID = VI.first.getGUID(); ExportsGUID.push_back(GUID); } @@ -205,7 +205,7 @@ void llvm::computeLTOCacheKey( AddUint64(Entry.getFunctions().size()); for (auto &Fn : Entry.getFunctions()) - AddUint64(Fn); + AddUint64(Fn.first); } // Include the hash for the resolved ODR. @@ -277,7 +277,7 @@ void llvm::computeLTOCacheKey( for (const ImportModule &ImpM : ImportModulesVector) for (auto &ImpF : ImpM.getFunctions()) { GlobalValueSummary *S = - Index.findSummaryInModule(ImpF, ImpM.getIdentifier()); + Index.findSummaryInModule(ImpF.first, ImpM.getIdentifier()); AddUsedThings(S); // If this is an alias, we also care about any types/etc. that the aliasee // may reference. @@ -1389,15 +1389,18 @@ class lto::ThinBackendProc { llvm::StringRef ModulePath, const std::string &NewModulePath) { std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex; + ModuleToGVSummaryPtrSet ModuleToDeclarationSummaries; std::error_code EC; gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries, - ImportList, ModuleToSummariesForIndex); + ImportList, ModuleToSummariesForIndex, + ModuleToDeclarationSummaries); raw_fd_ostream OS(NewModulePath + ".thinlto.bc", EC, sys::fs::OpenFlags::OF_None); if (EC) return errorCodeToError(EC); - writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex); + writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex, + &ModuleToDeclarationSummaries); if (ShouldEmitImportsFiles) { EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports", diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index 71e8849dc3cc91..6bb514d3d24e70 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -716,7 +716,10 @@ bool lto::initImportList(const Module &M, if (Summary->modulePath() == M.getModuleIdentifier()) continue; // Add an entry to provoke importing by thinBackend. - ImportList[Summary->modulePath()].insert(GUID); + ImportList[Summary->modulePath()][GUID].updateType( + Summary->flags().ImportAsDec + ? SummaryImportInfo::ImportType::Declaration + : SummaryImportInfo::ImportType::Definition); } } return true; diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp index 8f517eb50dc76f..75aecc95e789fc 100644 --- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp @@ -794,9 +794,11 @@ void ThinLTOCodeGenerator::gatherImportedSummariesForModule( IsPrevailing(PrevailingCopy), ImportLists, ExportLists); + ModuleToGVSummaryPtrSet ModuleToDecSummaries; llvm::gatherImportedSummariesForModule( ModuleIdentifier, ModuleToDefinedGVSummaries, - ImportLists[ModuleIdentifier], ModuleToSummariesForIndex); + ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, + ModuleToDecSummaries); } /** @@ -833,9 +835,12 @@ void ThinLTOCodeGenerator::emitImports(Module &TheModule, StringRef OutputName, ExportLists); std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex; + // FIXME: Pass on `ModuleToDecSummaries` to `EmitImportFiles` below. + ModuleToGVSummaryPtrSet ModuleToDecSummaries; llvm::gatherImportedSummariesForModule( ModuleIdentifier, ModuleToDefinedGVSummaries, - ImportLists[ModuleIdentifier], ModuleToSummariesForIndex); + ImportLists[ModuleIdentifier], ModuleToSummariesForIndex, + ModuleToDecSummaries); std::error_code EC; if ((EC = EmitImportsFiles(ModuleIdentifier, OutputName, diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 68f9799616ae6d..042fc285128b14 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -358,17 +358,23 @@ class GlobalsImporter final { if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) || LocalNotInModule(GVS)) continue; - auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID()); + auto [Iter, Inserted] = + ImportList[RefSummary->modulePath()].try_emplace( + VI.getGUID(), + SummaryImportInfo(SummaryImportInfo::ImportType::Definition)); // Only update stat and exports if we haven't already imported this // variable. - if (!ILI.second) + if (!Inserted) { + Iter->second.updateType(SummaryImportInfo::ImportType::Definition); break; + } NumImportedGlobalVarsThinLink++; // Any references made by this variable will be marked exported // later, in ComputeCrossModuleImport, after import decisions are // complete, which is more efficient than adding them here. if (ExportLists) - (*ExportLists)[RefSummary->modulePath()].insert(VI); + (*ExportLists)[RefSummary->modulePath()][VI].updateType( + SummaryImportInfo::ImportType::Definition); // If variable is not writeonly we attempt to recursively analyze // its references in order to import referenced constants. @@ -545,10 +551,12 @@ class WorkloadImportsManager : public ModuleImportsManager { LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from " << ExportingModule << " : " << Function::getGUID(VI.name()) << "\n"); - ImportList[ExportingModule].insert(VI.getGUID()); + ImportList[ExportingModule][VI.getGUID()].updateType( + SummaryImportInfo::ImportType::Definition); GVI.onImportingSummary(*GVS); if (ExportLists) - (*ExportLists)[ExportingModule].insert(VI); + (*ExportLists)[ExportingModule][VI].updateType( + SummaryImportInfo::ImportType::Definition); } LLVM_DEBUG(dbgs() << "[Workload] Done\n"); } @@ -816,23 +824,27 @@ static void computeImportForFunction( "selectCallee() didn't honor the threshold"); auto ExportModulePath = ResolvedCalleeSummary->modulePath(); - auto ILI = ImportList[ExportModulePath].insert(VI.getGUID()); + auto [Iter, Inserted] = + ImportList[ExportModulePath].insert(std::make_pair( + VI.getGUID(), + SummaryImportInfo(SummaryImportInfo::ImportType::Definition))); // We previously decided to import this GUID definition if it was already // inserted in the set of imports from the exporting module. - bool PreviouslyImported = !ILI.second; - if (!PreviouslyImported) { + if (Inserted) { NumImportedFunctionsThinLink++; if (IsHotCallsite) NumImportedHotFunctionsThinLink++; if (IsCriticalCallsite) NumImportedCriticalFunctionsThinLink++; - } + } else + Iter->second.updateType(SummaryImportInfo::ImportType::Definition); // Any calls/references made by this function will be marked exported // later, in ComputeCrossModuleImport, after import decisions are // complete, which is more efficient than adding them here. if (ExportLists) - (*ExportLists)[ExportModulePath].insert(VI); + (*ExportLists)[ExportModulePath][VI].updateType( + SummaryImportInfo::ImportType::Definition); } auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) { @@ -942,9 +954,11 @@ template <class T> static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont) { unsigned NumGVS = 0; - for (auto &V : Cont) - if (isGlobalVarSummary(Index, V)) + for (auto &[GUID, Type] : Cont) { + assert(Type.isDefinition() && "Declaration type doesn't exist yet"); + if (isGlobalVarSummary(Index, GUID)) ++NumGVS; + } return NumGVS; } #endif @@ -959,8 +973,8 @@ static bool checkVariableImport( for (auto &ImportPerModule : ImportLists) for (auto &ExportPerModule : ImportPerModule.second) - FlattenedImports.insert(ExportPerModule.second.begin(), - ExportPerModule.second.end()); + for (auto &[GUID, Type] : ExportPerModule.second) + FlattenedImports.insert(GUID); // Checks that all GUIDs of read/writeonly vars we see in export lists // are also in the import lists. Otherwise we my face linker undefs, @@ -979,7 +993,7 @@ static bool checkVariableImport( }; for (auto &ExportPerModule : ExportLists) - for (auto &VI : ExportPerModule.second) + for (auto &[VI, Unused] : ExportPerModule.second) if (!FlattenedImports.count(VI.getGUID()) && IsReadOrWriteOnlyVarNeedingImporting(ExportPerModule.first, VI)) return false; @@ -1015,7 +1029,8 @@ void llvm::ComputeCrossModuleImport( FunctionImporter::ExportSetTy NewExports; const auto &DefinedGVSummaries = ModuleToDefinedGVSummaries.lookup(ELI.first); - for (auto &EI : ELI.second) { + for (auto &[EI, Type] : ELI.second) { + assert(Type.isDefinition() && "Declaration type doesn't exist yet"); // Find the copy defined in the exporting module so that we can mark the // values it references in that specific definition as exported. // Below we will add all references and called values, without regard to @@ -1035,13 +1050,16 @@ void llvm::ComputeCrossModuleImport( // See processGlobalForThinLTO. if (!Index.isWriteOnly(GVS)) for (const auto &VI : GVS->refs()) - NewExports.insert(VI); + NewExports[VI].updateType( + SummaryImportInfo::ImportType::Declaration); } else { auto *FS = cast<FunctionSummary>(S); for (const auto &Edge : FS->calls()) - NewExports.insert(Edge.first); + NewExports[Edge.first].updateType( + SummaryImportInfo::ImportType::Declaration); for (const auto &Ref : FS->refs()) - NewExports.insert(Ref); + NewExports[Ref].updateType( + SummaryImportInfo::ImportType::Declaration); } } // Prune list computed above to only include values defined in the exporting @@ -1049,7 +1067,7 @@ void llvm::ComputeCrossModuleImport( // ref/call target multiple times in above loop, and it is more efficient to // avoid a set lookup each time. for (auto EI = NewExports.begin(); EI != NewExports.end();) { - if (!DefinedGVSummaries.count(EI->getGUID())) + if (!DefinedGVSummaries.count(EI->first.getGUID())) NewExports.erase(EI++); else ++EI; @@ -1149,7 +1167,10 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest( if (Summary->modulePath() == ModulePath) continue; // Add an entry to provoke importing by thinBackend. - ImportList[Summary->modulePath()].insert(GUID); + ImportList[Summary->modulePath()][GUID].updateType( + Summary->flags().ImportAsDec + ? SummaryImportInfo::ImportType::Declaration + : SummaryImportInfo::ImportType::Definition); } #ifndef NDEBUG dumpImportListForModule(Index, ModulePath, ImportList); @@ -1332,20 +1353,25 @@ void llvm::gatherImportedSummariesForModule( StringRef ModulePath, const DenseMap<StringRef, GVSummaryMapTy> &ModuleToDefinedGVSummaries, const FunctionImporter::ImportMapTy &ImportList, - std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) { + std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex, + ModuleToGVSummaryPtrSet &ModuleToDecSummaries) { // Include all summaries from the importing module. ModuleToSummariesForIndex[std::string(ModulePath)] = ModuleToDefinedGVSummaries.lookup(ModulePath); // Include summaries for imports. for (const auto &ILI : ImportList) { - auto &SummariesForIndex = ModuleToSummariesForIndex[std::string(ILI.first)]; + std::string ModulePath(ILI.first); + auto &SummariesForIndex = ModuleToSummariesForIndex[ModulePath]; + auto &DecSummaries = ModuleToDecSummaries[ModulePath]; const auto &DefinedGVSummaries = ModuleToDefinedGVSummaries.lookup(ILI.first); - for (const auto &GI : ILI.second) { - const auto &DS = DefinedGVSummaries.find(GI); + for (const auto &[GUID, Type] : ILI.second) { + const auto &DS = DefinedGVSummaries.find(GUID); assert(DS != DefinedGVSummaries.end() && "Expected a defined summary for imported global value"); - SummariesForIndex[GI] = DS->second; + SummariesForIndex[GUID] = DS->second; + if (Type.isDeclaration()) + DecSummaries.insert(DS->second); } } } @@ -1617,6 +1643,16 @@ Expected<bool> FunctionImporter::importFunctions( for (const auto &FunctionsToImportPerModule : ImportList) { ModuleNameOrderedList.insert(FunctionsToImportPerModule.first); } + + auto getImportStatus = [&](const FunctionsToImportTy &GUIDToImportType, + GlobalValue::GUID GUID) -> ImportStatus { + auto Iter = GUIDToImportType.find(GUID); + if (Iter == GUIDToImportType.end()) + return ImportStatus::NotImported; + return Iter->second.isDefinition() ? ImportStatus::ImportDefinition + : ImportStatus::ImportDeclaration; + }; + for (const auto &Name : ModuleNameOrderedList) { // Get the module for the import const auto &FunctionsToImportPerModule = ImportList.find(Name); @@ -1634,17 +1670,21 @@ Expected<bool> FunctionImporter::importFunctions( return std::move(Err); auto &ImportGUIDs = FunctionsToImportPerModule->second; + // Find the globals to import SetVector<GlobalValue *> GlobalsToImport; + SetVector<GlobalValue *> GlobalDeclsToImport; for (Function &F : *SrcModule) { if (!F.hasName()) continue; auto GUID = F.getGUID(); - auto Import = ImportGUIDs.count(GUID); - LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing function " - << GUID << " " << F.getName() << " from " - << SrcModule->getSourceFileName() << "\n"); - if (Import) { + auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + const bool ImportDefinition = + ImportStatus == ImportStatus::ImportDefinition; + LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") + << " importing function " << GUID << " " << F.getName() + << " from " << SrcModule->getSourceFileName() << "\n"); + if (ImportDefinition) { if (Error Err = F.materialize()) return std::move(Err); // MemProf should match function's definition and summary, @@ -1664,17 +1704,20 @@ Expected<bool> FunctionImporter::importFunctions( SrcModule->getSourceFileName())})); } GlobalsToImport.insert(&F); - } + } else if (ImportStatus == ImportDeclaration) + GlobalDeclsToImport.insert(&F); } for (GlobalVariable &GV : SrcModule->globals()) { if (!GV.hasName()) continue; auto GUID = GV.getGUID(); - auto Import = ImportGUIDs.count(GUID); - LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global " - << GUID << " " << GV.getName() << " from " - << SrcModule->getSourceFileName() << "\n"); - if (Import) { + auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + const bool ImportDefinition = + (ImportStatus == ImportStatus::ImportDefinition); + LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") + << " importing global " << GUID << " " << GV.getName() + << " from " << SrcModule->getSourceFileName() << "\n"); + if (ImportDefinition) { if (Error Err = GV.materialize()) return std::move(Err); ImportedGVCount += GlobalsToImport.insert(&GV); @@ -1684,11 +1727,13 @@ Expected<bool> FunctionImporter::importFunctions( if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject())) continue; auto GUID = GA.getGUID(); - auto Import = ImportGUIDs.count(GUID); - LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing alias " - << GUID << " " << GA.getName() << " from " - << SrcModule->getSourceFileName() << "\n"); - if (Import) { + auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + const bool ImportDefinition = + (ImportStatus == ImportStatus::ImportDefinition); + LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") + << " importing alias " << GUID << " " << GA.getName() + << " from " << SrcModule->getSourceFileName() << "\n"); + if (ImportDefinition) { if (Error Err = GA.materialize()) return std::move(Err); // Import alias as a copy of its aliasee. @@ -1738,6 +1783,7 @@ Expected<bool> FunctionImporter::importFunctions( << " from " << SrcModule->getSourceFileName() << "\n"; } + // FIXME: A later change will pass on GlobalDeclsToImport to IRMover. if (Error Err = Mover.move(std::move(SrcModule), GlobalsToImport.getArrayRef(), nullptr, /*IsPerformingImport=*/true)) diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp index 9049cb5e858002..0a60b1e3d8f5d2 100644 --- a/llvm/tools/llvm-link/llvm-link.cpp +++ b/llvm/tools/llvm-link/llvm-link.cpp @@ -376,7 +376,7 @@ static bool importFunctions(const char *argv0, Module &DestModule) { auto &Entry = ImportList[FileNameStringCache.insert(FileName).first->getKey()]; - Entry.insert(F->getGUID()); + Entry[F->getGUID()].updateType(SummaryImportInfo::ImportType::Definition); } auto CachedModuleLoader = [&](StringRef Identifier) { return ModuleLoaderCache.takeModule(std::string(Identifier)); >From 23a3fa4d3d124a2525f20e3a58da6d5e7740e7df Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Tue, 16 Apr 2024 21:12:34 -0700 Subject: [PATCH 2/2] simplify code --- llvm/include/llvm/IR/ModuleSummaryIndex.h | 30 ++----- .../llvm/Transforms/IPO/FunctionImport.h | 16 ++-- llvm/lib/LTO/LTOBackend.cpp | 10 ++- llvm/lib/Transforms/IPO/FunctionImport.cpp | 90 +++++++++---------- llvm/tools/llvm-link/llvm-link.cpp | 2 +- 5 files changed, 62 insertions(+), 86 deletions(-) diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h index 7f6f7446e09620..29d13cf4f4fae6 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -296,30 +296,6 @@ template <> struct DenseMapInfo<ValueInfo> { static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.getRef(); } }; -struct SummaryImportInfo { - enum class ImportType : uint8_t { - NotImported = 0, - Declaration = 1, - Definition = 2, - }; - unsigned Type : 3; - SummaryImportInfo() : Type(static_cast<unsigned>(ImportType::NotImported)) {} - SummaryImportInfo(ImportType Type) : Type(static_cast<unsigned>(Type)) {} - - // FIXME: delete the first two set* helper function. - void updateType(ImportType InputType) { - Type = std::max(Type, static_cast<unsigned>(InputType)); - } - - bool isDefinition() const { - return static_cast<ImportType>(Type) == ImportType::Definition; - } - - bool isDeclaration() const { - return static_cast<ImportType>(Type) == ImportType::Declaration; - } -}; - /// Summary of memprof callsite metadata. struct CallsiteInfo { // Actual callee function. @@ -609,7 +585,11 @@ class GlobalValueSummary { return Flags.ImportType == GlobalValueSummary::ImportKind::Declaration; } - void setImportKind(ImportKind IK) { Flags.ImportType = IK; } + GlobalValueSummary::ImportKind importType() const { + return static_cast<ImportKind>(Flags.ImportType); + } + + void setImportType(ImportKind IK) { Flags.ImportType = IK; } GlobalValue::VisibilityTypes getVisibility() const { return (GlobalValue::VisibilityTypes)Flags.Visibility; diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h index 9adc0c31eed439..6729fcc420a6ed 100644 --- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h +++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h @@ -33,14 +33,8 @@ class FunctionImporter { public: /// Set of functions to import from a source module. Each entry is a set /// containing all the GUIDs of all functions to import for a source module. - using FunctionsToImportTy = DenseMap<GlobalValue::GUID, SummaryImportInfo>; - - // FIXME: Remove this. - enum ImportStatus { - NotImported, - ImportDeclaration, - ImportDefinition, - }; + using FunctionsToImportTy = + DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>; /// The different reasons selectCallee will chose not to import a /// candidate. @@ -107,9 +101,9 @@ class FunctionImporter { using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>; /// The map contains an entry for every global value the module exports, the - /// key being the value info, and the value is the summary-based import info. - /// FIXME: Does this set need to be a map? - using ExportSetTy = DenseMap<ValueInfo, SummaryImportInfo>; + /// key being the value info, and the value indicates whether the definition + /// or declaration is visible to another module. + using ExportSetTy = DenseMap<ValueInfo, GlobalValueSummary::ImportKind>; /// A function of this type is used to load modules referenced by the index. using ModuleLoaderTy = diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index 6bb514d3d24e70..23d75c238fc296 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -715,11 +715,13 @@ bool lto::initImportList(const Module &M, // e.g. record required linkage changes. if (Summary->modulePath() == M.getModuleIdentifier()) continue; + // Add an entry to provoke importing by thinBackend. - ImportList[Summary->modulePath()][GUID].updateType( - Summary->flags().ImportAsDec - ? SummaryImportInfo::ImportType::Declaration - : SummaryImportInfo::ImportType::Definition); + auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace( + GUID, Summary->importType()); + + if (!Inserted) + Iter->second = std::min(Iter->second, Summary->importType()); } } return true; diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 042fc285128b14..1485259e642a04 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -358,14 +358,14 @@ class GlobalsImporter final { if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) || LocalNotInModule(GVS)) continue; + auto [Iter, Inserted] = ImportList[RefSummary->modulePath()].try_emplace( - VI.getGUID(), - SummaryImportInfo(SummaryImportInfo::ImportType::Definition)); + VI.getGUID(), GlobalValueSummary::Definition); // Only update stat and exports if we haven't already imported this // variable. if (!Inserted) { - Iter->second.updateType(SummaryImportInfo::ImportType::Definition); + Iter->second = std::min(GlobalValueSummary::Definition, Iter->second); break; } NumImportedGlobalVarsThinLink++; @@ -373,8 +373,8 @@ class GlobalsImporter final { // later, in ComputeCrossModuleImport, after import decisions are // complete, which is more efficient than adding them here. if (ExportLists) - (*ExportLists)[RefSummary->modulePath()][VI].updateType( - SummaryImportInfo::ImportType::Definition); + (*ExportLists)[RefSummary->modulePath()][VI] = + GlobalValueSummary::Definition; // If variable is not writeonly we attempt to recursively analyze // its references in order to import referenced constants. @@ -551,12 +551,11 @@ class WorkloadImportsManager : public ModuleImportsManager { LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from " << ExportingModule << " : " << Function::getGUID(VI.name()) << "\n"); - ImportList[ExportingModule][VI.getGUID()].updateType( - SummaryImportInfo::ImportType::Definition); + ImportList[ExportingModule][VI.getGUID()] = + GlobalValueSummary::Definition; GVI.onImportingSummary(*GVS); if (ExportLists) - (*ExportLists)[ExportingModule][VI].updateType( - SummaryImportInfo::ImportType::Definition); + (*ExportLists)[ExportingModule][VI] = GlobalValueSummary::Definition; } LLVM_DEBUG(dbgs() << "[Workload] Done\n"); } @@ -824,10 +823,8 @@ static void computeImportForFunction( "selectCallee() didn't honor the threshold"); auto ExportModulePath = ResolvedCalleeSummary->modulePath(); - auto [Iter, Inserted] = - ImportList[ExportModulePath].insert(std::make_pair( - VI.getGUID(), - SummaryImportInfo(SummaryImportInfo::ImportType::Definition))); + auto [Iter, Inserted] = ImportList[ExportModulePath].insert( + std::make_pair(VI.getGUID(), GlobalValueSummary::Definition)); // We previously decided to import this GUID definition if it was already // inserted in the set of imports from the exporting module. if (Inserted) { @@ -837,14 +834,13 @@ static void computeImportForFunction( if (IsCriticalCallsite) NumImportedCriticalFunctionsThinLink++; } else - Iter->second.updateType(SummaryImportInfo::ImportType::Definition); + Iter->second = GlobalValueSummary::Definition; // Any calls/references made by this function will be marked exported // later, in ComputeCrossModuleImport, after import decisions are // complete, which is more efficient than adding them here. if (ExportLists) - (*ExportLists)[ExportModulePath][VI].updateType( - SummaryImportInfo::ImportType::Definition); + (*ExportLists)[ExportModulePath][VI] = GlobalValueSummary::Definition; } auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) { @@ -955,7 +951,8 @@ static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont) { unsigned NumGVS = 0; for (auto &[GUID, Type] : Cont) { - assert(Type.isDefinition() && "Declaration type doesn't exist yet"); + assert(Type == GlobalValueSummary::Definition && + "Declaration type doesn't exist yet"); if (isGlobalVarSummary(Index, GUID)) ++NumGVS; } @@ -1030,7 +1027,8 @@ void llvm::ComputeCrossModuleImport( const auto &DefinedGVSummaries = ModuleToDefinedGVSummaries.lookup(ELI.first); for (auto &[EI, Type] : ELI.second) { - assert(Type.isDefinition() && "Declaration type doesn't exist yet"); + assert(Type == GlobalValueSummary::Definition && + "Declaration type doesn't exist yet"); // Find the copy defined in the exporting module so that we can mark the // values it references in that specific definition as exported. // Below we will add all references and called values, without regard to @@ -1049,17 +1047,15 @@ void llvm::ComputeCrossModuleImport( // we convert such variables initializers to "zeroinitializer". // See processGlobalForThinLTO. if (!Index.isWriteOnly(GVS)) - for (const auto &VI : GVS->refs()) - NewExports[VI].updateType( - SummaryImportInfo::ImportType::Declaration); + for (const auto &VI : GVS->refs()) { + NewExports.try_emplace(VI, GlobalValueSummary::Declaration); + } } else { auto *FS = cast<FunctionSummary>(S); for (const auto &Edge : FS->calls()) - NewExports[Edge.first].updateType( - SummaryImportInfo::ImportType::Declaration); + NewExports.try_emplace(Edge.first, GlobalValueSummary::Declaration); for (const auto &Ref : FS->refs()) - NewExports[Ref].updateType( - SummaryImportInfo::ImportType::Declaration); + NewExports.try_emplace(Ref, GlobalValueSummary::Declaration); } } // Prune list computed above to only include values defined in the exporting @@ -1167,10 +1163,10 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest( if (Summary->modulePath() == ModulePath) continue; // Add an entry to provoke importing by thinBackend. - ImportList[Summary->modulePath()][GUID].updateType( - Summary->flags().ImportAsDec - ? SummaryImportInfo::ImportType::Declaration - : SummaryImportInfo::ImportType::Definition); + auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace( + GUID, Summary->importType()); + if (!Inserted) + Iter->second = std::min(Iter->second, Summary->importType()); } #ifndef NDEBUG dumpImportListForModule(Index, ModulePath, ImportList); @@ -1370,7 +1366,7 @@ void llvm::gatherImportedSummariesForModule( assert(DS != DefinedGVSummaries.end() && "Expected a defined summary for imported global value"); SummariesForIndex[GUID] = DS->second; - if (Type.isDeclaration()) + if (Type == GlobalValueSummary::Declaration) DecSummaries.insert(DS->second); } } @@ -1644,13 +1640,13 @@ Expected<bool> FunctionImporter::importFunctions( ModuleNameOrderedList.insert(FunctionsToImportPerModule.first); } - auto getImportStatus = [&](const FunctionsToImportTy &GUIDToImportType, - GlobalValue::GUID GUID) -> ImportStatus { + auto maybeGetImportType = [&](const FunctionsToImportTy &GUIDToImportType, + GlobalValue::GUID GUID) + -> std::optional<GlobalValueSummary::ImportKind> { auto Iter = GUIDToImportType.find(GUID); if (Iter == GUIDToImportType.end()) - return ImportStatus::NotImported; - return Iter->second.isDefinition() ? ImportStatus::ImportDefinition - : ImportStatus::ImportDeclaration; + return std::nullopt; + return Iter->second; }; for (const auto &Name : ModuleNameOrderedList) { @@ -1673,14 +1669,15 @@ Expected<bool> FunctionImporter::importFunctions( // Find the globals to import SetVector<GlobalValue *> GlobalsToImport; - SetVector<GlobalValue *> GlobalDeclsToImport; for (Function &F : *SrcModule) { if (!F.hasName()) continue; auto GUID = F.getGUID(); - auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + auto ImportType = maybeGetImportType(ImportGUIDs, GUID); + if (!ImportType) + continue; const bool ImportDefinition = - ImportStatus == ImportStatus::ImportDefinition; + *ImportType == GlobalValueSummary::Definition; LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") << " importing function " << GUID << " " << F.getName() << " from " << SrcModule->getSourceFileName() << "\n"); @@ -1704,16 +1701,17 @@ Expected<bool> FunctionImporter::importFunctions( SrcModule->getSourceFileName())})); } GlobalsToImport.insert(&F); - } else if (ImportStatus == ImportDeclaration) - GlobalDeclsToImport.insert(&F); + } } for (GlobalVariable &GV : SrcModule->globals()) { if (!GV.hasName()) continue; auto GUID = GV.getGUID(); - auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + auto ImportType = maybeGetImportType(ImportGUIDs, GUID); + if (!ImportType) + continue; const bool ImportDefinition = - (ImportStatus == ImportStatus::ImportDefinition); + (*ImportType == GlobalValueSummary::Definition); LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") << " importing global " << GUID << " " << GV.getName() << " from " << SrcModule->getSourceFileName() << "\n"); @@ -1727,9 +1725,12 @@ Expected<bool> FunctionImporter::importFunctions( if (!GA.hasName() || isa<GlobalIFunc>(GA.getAliaseeObject())) continue; auto GUID = GA.getGUID(); - auto ImportStatus = getImportStatus(ImportGUIDs, GUID); + auto ImportType = maybeGetImportType(ImportGUIDs, GUID); + if (!ImportType) + continue; + const bool ImportDefinition = - (ImportStatus == ImportStatus::ImportDefinition); + (*ImportType == GlobalValueSummary::Definition); LLVM_DEBUG(dbgs() << (ImportDefinition ? "Is" : "Not") << " importing alias " << GUID << " " << GA.getName() << " from " << SrcModule->getSourceFileName() << "\n"); @@ -1783,7 +1784,6 @@ Expected<bool> FunctionImporter::importFunctions( << " from " << SrcModule->getSourceFileName() << "\n"; } - // FIXME: A later change will pass on GlobalDeclsToImport to IRMover. if (Error Err = Mover.move(std::move(SrcModule), GlobalsToImport.getArrayRef(), nullptr, /*IsPerformingImport=*/true)) diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp index 9e28b3a217f8b0..a6bee02b1302d3 100644 --- a/llvm/tools/llvm-link/llvm-link.cpp +++ b/llvm/tools/llvm-link/llvm-link.cpp @@ -377,7 +377,7 @@ static bool importFunctions(const char *argv0, Module &DestModule) { auto &Entry = ImportList[FileNameStringCache.insert(FileName).first->getKey()]; - Entry[F->getGUID()].updateType(SummaryImportInfo::ImportType::Definition); + Entry[F->getGUID()] = GlobalValueSummary::Definition; } auto CachedModuleLoader = [&](StringRef Identifier) { return ModuleLoaderCache.takeModule(std::string(Identifier)); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits