diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h index 5ab8c6d130b60a..0c8380db74314f 100644 --- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h +++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h @@ -96,13 +96,54 @@ class FunctionImporter { std::tuple>>; - /// The map contains an entry for every module to import from, the key being - /// the module identifier to pass to the ModuleLoader. The value is the set of - /// functions to import. The module identifier strings must be owned - /// elsewhere, typically by the in-memory ModuleSummaryIndex the importing - /// decisions are made from (the module path for each summary is owned by the - /// index's module path string table). - using ImportMapTy = DenseMap; + /// The map maintains the list of imports. Conceptually, it is a collection + /// of tuples of the form: + /// + /// (The name of the source module, GUID, Definition/Declaration) + /// + /// The name of the source module is the module identifier to pass to the + /// ModuleLoader. The module identifier strings must be owned elsewhere, + /// typically by the in-memory ModuleSummaryIndex the importing decisions are + /// made from (the module path for each summary is owned by the index's module + /// path string table). + class ImportMapTy { + public: + using ImportMapTyImpl = DenseMap; + + enum class AddDefinitionStatus { + // No change was made to the list of imports or whether each import should + // be imported as a declaration or definition. + NoChange, + // Successfully added the given GUID to be imported as a definition. There + // was no existing entry with the same GUID as a declaration. + Inserted, + // An existing with the given GUID was changed to a definition. + ChangedToDefinition, + }; + + // Add the given GUID to ImportList as a definition. If the same GUID has + // been added as a declaration previously, that entry is overridden. + AddDefinitionStatus addDefinition(StringRef FromModule, + GlobalValue::GUID GUID); + + // Add the given GUID to ImportList as a declaration. If the same GUID has + // been added as a definition previously, that entry takes precedence, and + // no change is made. + void maybeAddDeclaration(StringRef FromModule, GlobalValue::GUID GUID); + + void addGUID(StringRef FromModule, GlobalValue::GUID GUID, + GlobalValueSummary::ImportKind ImportKind) { + if (ImportKind == GlobalValueSummary::Definition) + addDefinition(FromModule, GUID); + else + maybeAddDeclaration(FromModule, GUID); + } + + const ImportMapTyImpl &getImportMap() const { return ImportMap; } + + private: + ImportMapTyImpl ImportMap; + }; /// The set contains an entry for every global value that the module exports. /// Depending on the user context, this container is allowed to contain @@ -122,33 +163,6 @@ class FunctionImporter { /// Import functions in Module \p M based on the supplied import list. Expected importFunctions(Module &M, const ImportMapTy &ImportList); - enum class AddDefinitionStatus { - NoChange, - Inserted, - ChangedToDefinition, - }; - - // Add the given GUID to ImportList as a definition. If the same GUID has - // been added as a declaration previously, that entry is overridden. - static AddDefinitionStatus addDefinition(ImportMapTy &ImportList, - StringRef FromModule, - GlobalValue::GUID GUID); - - // Add the given GUID to ImportList as a declaration. If the same GUID has - // been added as a definition previously, that entry takes precedence, and no - // change is made. - static void maybeAddDeclaration(ImportMapTy &ImportList, StringRef FromModule, - GlobalValue::GUID GUID); - - static void addGUID(ImportMapTy &ImportList, StringRef FromModule, - GlobalValue::GUID GUID, - GlobalValueSummary::ImportKind ImportKind) { - if (ImportKind == GlobalValueSummary::Definition) - addDefinition(ImportList, FromModule, GUID); - else - maybeAddDeclaration(ImportList, FromModule, GUID); - } - private: /// The summaries index used to trigger importing. const ModuleSummaryIndex &Index; diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index e5545860c329d4..ee0193344d5ac9 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -177,7 +177,8 @@ std::string llvm::computeLTOCacheKey( // Include the hash for every module we import functions from. The set of // imported symbols for each module may affect code generation and is // sensitive to link order, so include that as well. - using ImportMapIteratorTy = FunctionImporter::ImportMapTy::const_iterator; + using ImportMapIteratorTy = + FunctionImporter::ImportMapTy::ImportMapTyImpl::const_iterator; struct ImportModule { ImportMapIteratorTy ModIt; const ModuleSummaryIndex::ModuleInfo *ModInfo; @@ -191,10 +192,10 @@ std::string llvm::computeLTOCacheKey( }; std::vector ImportModulesVector; - ImportModulesVector.reserve(ImportList.size()); + ImportModulesVector.reserve(ImportList.getImportMap().size()); - for (ImportMapIteratorTy It = ImportList.begin(); It != ImportList.end(); - ++It) { + for (ImportMapIteratorTy It = ImportList.getImportMap().begin(); + It != ImportList.getImportMap().end(); ++It) { ImportModulesVector.push_back({It, Index.getModule(It->getFirst())}); } // Order using module hash, to be both independent of module name and diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index ae46d946ae06a6..4e58cd369c3ac9 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -726,8 +726,7 @@ bool lto::initImportList(const Module &M, if (Summary->modulePath() == M.getModuleIdentifier()) continue; // Add an entry to provoke importing by thinBackend. - FunctionImporter::addGUID(ImportList, Summary->modulePath(), GUID, - Summary->importType()); + ImportList.addGUID(Summary->modulePath(), GUID, Summary->importType()); } } return true; diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 55803670071d16..b26c15b665b590 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -334,11 +334,11 @@ using EdgeInfo = std::tuple; } // anonymous namespace -FunctionImporter::AddDefinitionStatus -FunctionImporter::addDefinition(ImportMapTy &ImportList, StringRef FromModule, - GlobalValue::GUID GUID) { +FunctionImporter::ImportMapTy::AddDefinitionStatus +FunctionImporter::ImportMapTy::addDefinition(StringRef FromModule, + GlobalValue::GUID GUID) { auto [It, Inserted] = - ImportList[FromModule].try_emplace(GUID, GlobalValueSummary::Definition); + ImportMap[FromModule].try_emplace(GUID, GlobalValueSummary::Definition); if (Inserted) return AddDefinitionStatus::Inserted; if (It->second == GlobalValueSummary::Definition) @@ -347,10 +347,9 @@ FunctionImporter::addDefinition(ImportMapTy &ImportList, StringRef FromModule, return AddDefinitionStatus::ChangedToDefinition; } -void FunctionImporter::maybeAddDeclaration(ImportMapTy &ImportList, - StringRef FromModule, - GlobalValue::GUID GUID) { - ImportList[FromModule].try_emplace(GUID, GlobalValueSummary::Declaration); +void FunctionImporter::ImportMapTy::maybeAddDeclaration( + StringRef FromModule, GlobalValue::GUID GUID) { + ImportMap[FromModule].try_emplace(GUID, GlobalValueSummary::Declaration); } /// Import globals referenced by a function or other globals that are being @@ -411,9 +410,8 @@ class GlobalsImporter final { // If there isn't an entry for GUID, insert pair. // Otherwise, definition should take precedence over declaration. - if (FunctionImporter::addDefinition( - ImportList, RefSummary->modulePath(), VI.getGUID()) != - FunctionImporter::AddDefinitionStatus::Inserted) + if (ImportList.addDefinition(RefSummary->modulePath(), VI.getGUID()) != + FunctionImporter::ImportMapTy::AddDefinitionStatus::Inserted) break; // Only update stat and exports if we haven't already imported this @@ -600,8 +598,7 @@ class WorkloadImportsManager : public ModuleImportsManager { LLVM_DEBUG(dbgs() << "[Workload][Including]" << VI.name() << " from " << ExportingModule << " : " << Function::getGUID(VI.name()) << "\n"); - FunctionImporter::addDefinition(ImportList, ExportingModule, - VI.getGUID()); + ImportList.addDefinition(ExportingModule, VI.getGUID()); GVI.onImportingSummary(*GVS); if (ExportLists) (*ExportLists)[ExportingModule].insert(VI); @@ -899,8 +896,7 @@ static void computeImportForFunction( // Note `ExportLists` only keeps track of exports due to imported // definitions. - FunctionImporter::maybeAddDeclaration(ImportList, DeclSourceModule, - VI.getGUID()); + ImportList.maybeAddDeclaration(DeclSourceModule, VI.getGUID()); } // Update with new larger threshold if this was a retry (otherwise // we would have already inserted with NewThreshold above). Also @@ -949,9 +945,8 @@ static void computeImportForFunction( // Try emplace the definition entry, and update stats based on insertion // status. - if (FunctionImporter::addDefinition(ImportList, ExportModulePath, - VI.getGUID()) != - FunctionImporter::AddDefinitionStatus::NoChange) { + if (ImportList.addDefinition(ExportModulePath, VI.getGUID()) != + FunctionImporter::ImportMapTy::AddDefinitionStatus::NoChange) { NumImportedFunctionsThinLink++; if (IsHotCallsite) NumImportedHotFunctionsThinLink++; @@ -1084,7 +1079,7 @@ numGlobalVarSummaries(const ModuleSummaryIndex &Index, // the number of defined function summaries as output parameter. static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, - FunctionImporter::FunctionsToImportTy &ImportMap, + const FunctionImporter::FunctionsToImportTy &ImportMap, unsigned &DefinedFS) { unsigned NumGVS = 0; DefinedFS = 0; @@ -1105,9 +1100,9 @@ static bool checkVariableImport( DenseMap &ExportLists) { DenseSet FlattenedImports; - for (auto &ImportPerModule : ImportLists) - for (auto &ExportPerModule : ImportPerModule.second) - for (auto &[GUID, Type] : ExportPerModule.second) + for (const auto &ImportPerModule : ImportLists) + for (const auto &ExportPerModule : ImportPerModule.second.getImportMap()) + for (const auto &[GUID, Type] : ExportPerModule.second) FlattenedImports.insert(GUID); // Checks that all GUIDs of read/writeonly vars we see in export lists @@ -1217,9 +1212,10 @@ void llvm::ComputeCrossModuleImport( unsigned NumGVS = numGlobalVarSummaries(Index, Exports); LLVM_DEBUG(dbgs() << "* Module " << ModName << " exports " << Exports.size() - NumGVS << " functions and " << NumGVS - << " vars. Imports from " << ModuleImports.second.size() + << " vars. Imports from " + << ModuleImports.second.getImportMap().size() << " modules.\n"); - for (auto &Src : ModuleImports.second) { + for (const auto &Src : ModuleImports.second.getImportMap()) { auto SrcModName = Src.first; unsigned DefinedFS = 0; unsigned NumGVSPerMod = @@ -1240,8 +1236,8 @@ static void dumpImportListForModule(const ModuleSummaryIndex &Index, StringRef ModulePath, FunctionImporter::ImportMapTy &ImportList) { LLVM_DEBUG(dbgs() << "* Module " << ModulePath << " imports from " - << ImportList.size() << " modules.\n"); - for (auto &Src : ImportList) { + << ImportList.getImportMap().size() << " modules.\n"); + for (const auto &Src : ImportList.getImportMap()) { auto SrcModName = Src.first; unsigned DefinedFS = 0; unsigned NumGVSPerMod = numGlobalVarSummaries(Index, Src.second, DefinedFS); @@ -1306,8 +1302,7 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest( if (Summary->modulePath() == ModulePath) continue; // Add an entry to provoke importing by thinBackend. - FunctionImporter::addGUID(ImportList, Summary->modulePath(), GUID, - Summary->importType()); + ImportList.addGUID(Summary->modulePath(), GUID, Summary->importType()); } #ifndef NDEBUG dumpImportListForModule(Index, ModulePath, ImportList); @@ -1496,7 +1491,7 @@ void llvm::gatherImportedSummariesForModule( ModuleToSummariesForIndex[std::string(ModulePath)] = ModuleToDefinedGVSummaries.lookup(ModulePath); // Include summaries for imports. - for (const auto &ILI : ImportList) { + for (const auto &ILI : ImportList.getImportMap()) { auto &SummariesForIndex = ModuleToSummariesForIndex[std::string(ILI.first)]; const auto &DefinedGVSummaries = @@ -1777,7 +1772,7 @@ Expected FunctionImporter::importFunctions( IRMover Mover(DestModule); // Do the actual import of functions now, one Module at a time std::set ModuleNameOrderedList; - for (const auto &FunctionsToImportPerModule : ImportList) { + for (const auto &FunctionsToImportPerModule : ImportList.getImportMap()) { ModuleNameOrderedList.insert(FunctionsToImportPerModule.first); } @@ -1792,8 +1787,9 @@ Expected FunctionImporter::importFunctions( for (const auto &Name : ModuleNameOrderedList) { // Get the module for the import - const auto &FunctionsToImportPerModule = ImportList.find(Name); - assert(FunctionsToImportPerModule != ImportList.end()); + const auto &FunctionsToImportPerModule = + ImportList.getImportMap().find(Name); + assert(FunctionsToImportPerModule != ImportList.getImportMap().end()); Expected> SrcModuleOrErr = ModuleLoader(Name); if (!SrcModuleOrErr) return SrcModuleOrErr.takeError(); diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp index ef6f85d38fede6..54d38a6ddd6f10 100644 --- a/llvm/tools/llvm-link/llvm-link.cpp +++ b/llvm/tools/llvm-link/llvm-link.cpp @@ -381,9 +381,8 @@ static bool importFunctions(const char *argv0, Module &DestModule) { // definition, so make the import type definition directly. // FIXME: A follow-up patch should add test coverage for import declaration // in `llvm-link` CLI (e.g., by introducing a new command line option). - FunctionImporter::addDefinition( - ImportList, FileNameStringCache.insert(FileName).first->getKey(), - F->getGUID()); + ImportList.addDefinition( + FileNameStringCache.insert(FileName).first->getKey(), F->getGUID()); } auto CachedModuleLoader = [&](StringRef Identifier) { return ModuleLoaderCache.takeModule(std::string(Identifier));