[llvm] [ThinLTO] Track definitions only in export-set (PR #97360)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 1 15:41:46 PDT 2024
https://github.com/minglotus-6 created https://github.com/llvm/llvm-project/pull/97360
None
>From 7a276ba66769f515268c6b8071ef2c9aac39b23d Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
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 d8c142ec89d82..6cb9bf19faf61 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<StringRef, FunctionsToImportTy>;
- /// 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<ValueInfo, GlobalValueSummary::ImportKind>;
+ /// The set contains an entry for every global value the module exports as
+ // a definition.
+ using ExportSetTy = DenseSet<ValueInfo>;
/// 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 6bbec535d8e98..fe44721161f26 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>((uint8_t *)&ModHash[0], sizeof(ModHash)));
- std::vector<std::pair<uint64_t, uint8_t>> ExportsGUID;
+ std::vector<uint64_t> ExportsGUID;
ExportsGUID.reserve(ExportList.size());
- for (const auto &[VI, ExportType] : ExportList)
- ExportsGUID.push_back(
- std::make_pair(VI.getGUID(), static_cast<uint8_t>(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>((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 ec5294b9512cf..3b92ae0e46876 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 <VI, declaration> 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 <class T>
-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<FunctionSummary>(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
More information about the llvm-commits
mailing list