Skip to content

Commit 707f4de

Browse files
Revert "Reland "[ThinLTO] Populate declaration import status except for distributed ThinLTO under a default-off new option" (#92718) (#94503)
This reverts commit e33db24. The change from *set to *map increases memory usage, and caused indexing OOM in some applications. Need to profile offline to bring the memory usage down.
1 parent 53061ee commit 707f4de

File tree

9 files changed

+85
-444
lines changed

9 files changed

+85
-444
lines changed

llvm/include/llvm/IR/ModuleSummaryIndex.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -587,10 +587,6 @@ class GlobalValueSummary {
587587

588588
void setImportKind(ImportKind IK) { Flags.ImportType = IK; }
589589

590-
GlobalValueSummary::ImportKind importType() const {
591-
return static_cast<ImportKind>(Flags.ImportType);
592-
}
593-
594590
GlobalValue::VisibilityTypes getVisibility() const {
595591
return (GlobalValue::VisibilityTypes)Flags.Visibility;
596592
}
@@ -1276,9 +1272,6 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
12761272
/// a particular module, and provide efficient access to their summary.
12771273
using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
12781274

1279-
/// A set of global value summary pointers.
1280-
using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;
1281-
12821275
/// Map of a type GUID to type id string and summary (multimap used
12831276
/// in case of GUID conflicts).
12841277
using TypeIdSummaryMapTy =

llvm/include/llvm/Transforms/IPO/FunctionImport.h

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ class Module;
3131
/// based on the provided summary informations.
3232
class FunctionImporter {
3333
public:
34-
/// The functions to import from a source module and their import type.
35-
using FunctionsToImportTy =
36-
DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
34+
/// Set of functions to import from a source module. Each entry is a set
35+
/// containing all the GUIDs of all functions to import for a source module.
36+
using FunctionsToImportTy = std::unordered_set<GlobalValue::GUID>;
3737

3838
/// The different reasons selectCallee will chose not to import a
3939
/// candidate.
@@ -99,13 +99,8 @@ class FunctionImporter {
9999
/// index's module path string table).
100100
using ImportMapTy = DenseMap<StringRef, FunctionsToImportTy>;
101101

102-
/// The map contains an entry for every global value the module exports.
103-
/// The key is ValueInfo, and the value indicates whether the definition
104-
/// or declaration is visible to another module. If a function's definition is
105-
/// visible to other modules, the global values this function referenced are
106-
/// visible and shouldn't be internalized.
107-
/// TODO: Rename to `ExportMapTy`.
108-
using ExportSetTy = DenseMap<ValueInfo, GlobalValueSummary::ImportKind>;
102+
/// The set contains an entry for every global value the module exports.
103+
using ExportSetTy = DenseSet<ValueInfo>;
109104

110105
/// A function of this type is used to load modules referenced by the index.
111106
using ModuleLoaderTy =

llvm/lib/LTO/LTO.cpp

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,6 @@ void llvm::computeLTOCacheKey(
121121
support::endian::write64le(Data, I);
122122
Hasher.update(Data);
123123
};
124-
auto AddUint8 = [&](const uint8_t I) {
125-
Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&I, 1));
126-
};
127124
AddString(Conf.CPU);
128125
// FIXME: Hash more of Options. For now all clients initialize Options from
129126
// command-line flags (which is unsupported in production), but may set
@@ -159,18 +156,18 @@ void llvm::computeLTOCacheKey(
159156
auto ModHash = Index.getModuleHash(ModuleID);
160157
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
161158

162-
std::vector<std::pair<uint64_t, uint8_t>> ExportsGUID;
159+
std::vector<uint64_t> ExportsGUID;
163160
ExportsGUID.reserve(ExportList.size());
164-
for (const auto &[VI, ExportType] : ExportList)
165-
ExportsGUID.push_back(
166-
std::make_pair(VI.getGUID(), static_cast<uint8_t>(ExportType)));
161+
for (const auto &VI : ExportList) {
162+
auto GUID = VI.getGUID();
163+
ExportsGUID.push_back(GUID);
164+
}
167165

168166
// Sort the export list elements GUIDs.
169167
llvm::sort(ExportsGUID);
170-
for (auto [GUID, ExportType] : ExportsGUID) {
168+
for (uint64_t GUID : ExportsGUID) {
171169
// The export list can impact the internalization, be conservative here
172170
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&GUID, sizeof(GUID)));
173-
AddUint8(ExportType);
174171
}
175172

176173
// Include the hash for every module we import functions from. The set of
@@ -202,21 +199,19 @@ void llvm::computeLTOCacheKey(
202199
[](const ImportModule &Lhs, const ImportModule &Rhs) -> bool {
203200
return Lhs.getHash() < Rhs.getHash();
204201
});
205-
std::vector<std::pair<uint64_t, uint8_t>> ImportedGUIDs;
202+
std::vector<uint64_t> ImportedGUIDs;
206203
for (const ImportModule &Entry : ImportModulesVector) {
207204
auto ModHash = Entry.getHash();
208205
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
209206

210207
AddUint64(Entry.getFunctions().size());
211208

212209
ImportedGUIDs.clear();
213-
for (auto &[Fn, ImportType] : Entry.getFunctions())
214-
ImportedGUIDs.push_back(std::make_pair(Fn, ImportType));
210+
for (auto &Fn : Entry.getFunctions())
211+
ImportedGUIDs.push_back(Fn);
215212
llvm::sort(ImportedGUIDs);
216-
for (auto &[GUID, Type] : ImportedGUIDs) {
213+
for (auto &GUID : ImportedGUIDs)
217214
AddUint64(GUID);
218-
AddUint8(Type);
219-
}
220215
}
221216

222217
// Include the hash for the resolved ODR.
@@ -286,9 +281,9 @@ void llvm::computeLTOCacheKey(
286281
// Imported functions may introduce new uses of type identifier resolutions,
287282
// so we need to collect their used resolutions as well.
288283
for (const ImportModule &ImpM : ImportModulesVector)
289-
for (auto &[GUID, UnusedImportType] : ImpM.getFunctions()) {
284+
for (auto &ImpF : ImpM.getFunctions()) {
290285
GlobalValueSummary *S =
291-
Index.findSummaryInModule(GUID, ImpM.getIdentifier());
286+
Index.findSummaryInModule(ImpF, ImpM.getIdentifier());
292287
AddUsedThings(S);
293288
// If this is an alias, we also care about any types/etc. that the aliasee
294289
// may reference.
@@ -1400,7 +1395,6 @@ class lto::ThinBackendProc {
14001395
llvm::StringRef ModulePath,
14011396
const std::string &NewModulePath) {
14021397
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
1403-
14041398
std::error_code EC;
14051399
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
14061400
ImportList, ModuleToSummariesForIndex);
@@ -1409,8 +1403,6 @@ class lto::ThinBackendProc {
14091403
sys::fs::OpenFlags::OF_None);
14101404
if (EC)
14111405
return errorCodeToError(EC);
1412-
1413-
// TODO: Serialize declaration bits to bitcode.
14141406
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
14151407

14161408
if (ShouldEmitImportsFiles) {

llvm/lib/LTO/LTOBackend.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -720,14 +720,7 @@ bool lto::initImportList(const Module &M,
720720
if (Summary->modulePath() == M.getModuleIdentifier())
721721
continue;
722722
// Add an entry to provoke importing by thinBackend.
723-
// Try emplace the entry first. If an entry with the same key already
724-
// exists, set the value to 'std::min(existing-value, new-value)' to make
725-
// sure a definition takes precedence over a declaration.
726-
auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
727-
GUID, Summary->importType());
728-
729-
if (!Inserted)
730-
Iter->second = std::min(Iter->second, Summary->importType());
723+
ImportList[Summary->modulePath()].insert(GUID);
731724
}
732725
}
733726
return true;

0 commit comments

Comments
 (0)