From 7a276ba66769f515268c6b8071ef2c9aac39b23d Mon Sep 17 00:00:00 2001 From: mingmingl Date: Mon, 1 Jul 2024 14:14:18 -0700 Subject: [PATCH] [ThinLTO] Track definitions only in export-set --- .../llvm/Transforms/IPO/FunctionImport.h | 10 ++-- llvm/lib/LTO/LTO.cpp | 12 ++--- llvm/lib/Transforms/IPO/FunctionImport.cpp | 53 +++++++++++-------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h index d8c142ec89d821..6cb9bf19faf613 100644 --- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h +++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h @@ -104,13 +104,9 @@ class FunctionImporter { /// index's module path string table). using ImportMapTy = DenseMap; - /// The map contains an entry for every global value the module exports. - /// The key is ValueInfo, and the value indicates whether the definition - /// or declaration is visible to another module. If a function's definition is - /// visible to other modules, the global values this function referenced are - /// visible and shouldn't be internalized. - /// TODO: Rename to `ExportMapTy`. - using ExportSetTy = DenseMap; + /// The set contains an entry for every global value the module exports as + // a definition. + using ExportSetTy = DenseSet; /// A function of this type is used to load modules referenced by the index. using ModuleLoaderTy = diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 6bbec535d8e987..fe44721161f263 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -161,19 +161,15 @@ void llvm::computeLTOCacheKey( auto ModHash = Index.getModuleHash(ModuleID); Hasher.update(ArrayRef((uint8_t *)&ModHash[0], sizeof(ModHash))); - std::vector> ExportsGUID; + std::vector ExportsGUID; ExportsGUID.reserve(ExportList.size()); - for (const auto &[VI, ExportType] : ExportList) - ExportsGUID.push_back( - std::make_pair(VI.getGUID(), static_cast(ExportType))); + for (const auto &VI : ExportList) + ExportsGUID.push_back(VI.getGUID()); // Sort the export list elements GUIDs. llvm::sort(ExportsGUID); - for (auto [GUID, ExportType] : ExportsGUID) { - // The export list can impact the internalization, be conservative here + for (auto GUID : ExportsGUID) Hasher.update(ArrayRef((uint8_t *)&GUID, sizeof(GUID))); - AddUint8(ExportType); - } // Include the hash for every module we import functions from. The set of // imported symbols for each module may affect code generation and is diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index ec5294b9512cf3..3b92ae0e46876c 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -400,8 +400,7 @@ 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] = - GlobalValueSummary::Definition; + (*ExportLists)[RefSummary->modulePath()].insert(VI); // If variable is not writeonly we attempt to recursively analyze // its references in order to import referenced constants. @@ -582,7 +581,7 @@ class WorkloadImportsManager : public ModuleImportsManager { GlobalValueSummary::Definition; GVI.onImportingSummary(*GVS); if (ExportLists) - (*ExportLists)[ExportingModule][VI] = GlobalValueSummary::Definition; + (*ExportLists)[ExportingModule].insert(VI); } LLVM_DEBUG(dbgs() << "[Workload] Done\n"); } @@ -819,9 +818,6 @@ static void computeImportForFunction( // try emplace pair without checking insert result. // If insert doesn't happen, there must be an existing entry keyed by // VI. - if (ExportLists) - (*ExportLists)[DeclSourceModule].try_emplace( - VI, GlobalValueSummary::Declaration); ImportList[DeclSourceModule].try_emplace( VI.getGUID(), GlobalValueSummary::Declaration); } @@ -892,7 +888,7 @@ static void computeImportForFunction( // later, in ComputeCrossModuleImport, after import decisions are // complete, which is more efficient than adding them here. if (ExportLists) - (*ExportLists)[ExportModulePath][VI] = GlobalValueSummary::Definition; + (*ExportLists)[ExportModulePath].insert(VI); } auto GetAdjustedThreshold = [](unsigned Threshold, bool IsHotCallsite) { @@ -998,10 +994,27 @@ static bool isGlobalVarSummary(const ModuleSummaryIndex &Index, return false; } -template -static unsigned numGlobalVarSummaries(const ModuleSummaryIndex &Index, T &Cont, - unsigned &DefinedGVS, - unsigned &DefinedFS) { +static unsigned +numGlobalVarSummariesInSet(const ModuleSummaryIndex &Index, + FunctionImporter::ExportSetTy &ExportSet, + unsigned &DefinedGVS, unsigned &DefinedFS) { + unsigned NumGVS = 0; + DefinedGVS = 0; + DefinedFS = 0; + for (auto &VI : ExportSet) { + if (isGlobalVarSummary(Index, VI.getGUID())) { + ++DefinedGVS; + ++NumGVS; + } else + ++DefinedFS; + } + return NumGVS; +} + +static unsigned +numGlobalVarSummaries(const ModuleSummaryIndex &Index, + FunctionImporter::FunctionsToImportTy &Cont, + unsigned &DefinedGVS, unsigned &DefinedFS) { unsigned NumGVS = 0; DefinedGVS = 0; DefinedFS = 0; @@ -1046,7 +1059,7 @@ static bool checkVariableImport( }; for (auto &ExportPerModule : ExportLists) - for (auto &[VI, Unused] : ExportPerModule.second) + for (auto &VI : ExportPerModule.second) if (!FlattenedImports.count(VI.getGUID()) && IsReadOrWriteOnlyVarNeedingImporting(ExportPerModule.first, VI)) return false; @@ -1082,11 +1095,7 @@ void llvm::ComputeCrossModuleImport( FunctionImporter::ExportSetTy NewExports; const auto &DefinedGVSummaries = ModuleToDefinedGVSummaries.lookup(ELI.first); - for (auto &[EI, Type] : ELI.second) { - // If a variable is exported as a declaration, its 'refs' and 'calls' are - // not further exported. - if (Type == GlobalValueSummary::Declaration) - continue; + for (auto &EI : ELI.second) { // 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 @@ -1108,19 +1117,19 @@ void llvm::ComputeCrossModuleImport( for (const auto &VI : GVS->refs()) { // Try to emplace the declaration entry. If a definition entry // already exists for key `VI`, this is a no-op. - NewExports.try_emplace(VI, GlobalValueSummary::Declaration); + NewExports.insert(VI); } } else { auto *FS = cast(S); for (const auto &Edge : FS->calls()) { // Try to emplace the declaration entry. If a definition entry // already exists for key `VI`, this is a no-op. - NewExports.try_emplace(Edge.first, GlobalValueSummary::Declaration); + NewExports.insert(Edge.first); } for (const auto &Ref : FS->refs()) { // Try to emplace the declaration entry. If a definition entry // already exists for key `VI`, this is a no-op. - NewExports.try_emplace(Ref, GlobalValueSummary::Declaration); + NewExports.insert(Ref); } } } @@ -1129,7 +1138,7 @@ void llvm::ComputeCrossModuleImport( // the same 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->first.getGUID())) + if (!DefinedGVSummaries.count(EI->getGUID())) NewExports.erase(EI++); else ++EI; @@ -1146,7 +1155,7 @@ void llvm::ComputeCrossModuleImport( auto &Exports = ExportLists[ModName]; unsigned DefinedGVS = 0, DefinedFS = 0; unsigned NumGVS = - numGlobalVarSummaries(Index, Exports, DefinedGVS, DefinedFS); + numGlobalVarSummariesInSet(Index, Exports, DefinedGVS, DefinedFS); LLVM_DEBUG(dbgs() << "* Module " << ModName << " exports " << DefinedFS << " function as definitions, " << Exports.size() - NumGVS - DefinedFS