Skip to content

Commit 8de7890

Browse files
[ThinLTO] Populate declaration import status except for distributed ThinLTO under a default-off new option (#88024)
The goal is to populate `declaration` import status if a new flag`-import-declaration` is on. * For in-process ThinLTO, the `declaration` status is visible to backend `function-import` pass, so `FunctionImporter::importFunctions` should read the import status and be no-op for declaration summaries. Basically, the postlink pipeline is updated to keep its current behavior (import definitions), but not updated to handle `declaration` summaries. Two use cases (better call-graph sort and cross-module auto-init) would use this bit differently. * For distributed ThinLTO, the `declaration` status is not serialized to bitcode. As discussed, #87600 will do this. [1] https://discourse.llvm.org/t/rfc-for-better-call-graph-sort-build-a-more-complete-call-graph-by-adding-more-indirect-call-edges/74029#support-cross-module-function-declaration-import-5 [2] #87597 (comment)
1 parent f652777 commit 8de7890

File tree

9 files changed

+443
-85
lines changed

9 files changed

+443
-85
lines changed

llvm/include/llvm/IR/ModuleSummaryIndex.h

+7
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,10 @@ 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+
590594
GlobalValue::VisibilityTypes getVisibility() const {
591595
return (GlobalValue::VisibilityTypes)Flags.Visibility;
592596
}
@@ -1272,6 +1276,9 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
12721276
/// a particular module, and provide efficient access to their summary.
12731277
using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
12741278

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

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

+10-5
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-
/// 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>;
34+
/// The functions to import from a source module and their import type.
35+
using FunctionsToImportTy =
36+
DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
3737

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

102-
/// The set contains an entry for every global value the module exports.
103-
using ExportSetTy = DenseSet<ValueInfo>;
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>;
104109

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

llvm/lib/LTO/LTO.cpp

+20-12
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ 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+
};
124127
AddString(Conf.CPU);
125128
// FIXME: Hash more of Options. For now all clients initialize Options from
126129
// command-line flags (which is unsupported in production), but may set
@@ -156,18 +159,18 @@ void llvm::computeLTOCacheKey(
156159
auto ModHash = Index.getModuleHash(ModuleID);
157160
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
158161

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

166168
// Sort the export list elements GUIDs.
167169
llvm::sort(ExportsGUID);
168-
for (uint64_t GUID : ExportsGUID) {
170+
for (auto [GUID, ExportType] : ExportsGUID) {
169171
// The export list can impact the internalization, be conservative here
170172
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&GUID, sizeof(GUID)));
173+
AddUint8(ExportType);
171174
}
172175

173176
// Include the hash for every module we import functions from. The set of
@@ -199,19 +202,21 @@ void llvm::computeLTOCacheKey(
199202
[](const ImportModule &Lhs, const ImportModule &Rhs) -> bool {
200203
return Lhs.getHash() < Rhs.getHash();
201204
});
202-
std::vector<uint64_t> ImportedGUIDs;
205+
std::vector<std::pair<uint64_t, uint8_t>> ImportedGUIDs;
203206
for (const ImportModule &Entry : ImportModulesVector) {
204207
auto ModHash = Entry.getHash();
205208
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
206209

207210
AddUint64(Entry.getFunctions().size());
208211

209212
ImportedGUIDs.clear();
210-
for (auto &Fn : Entry.getFunctions())
211-
ImportedGUIDs.push_back(Fn);
213+
for (auto &[Fn, ImportType] : Entry.getFunctions())
214+
ImportedGUIDs.push_back(std::make_pair(Fn, ImportType));
212215
llvm::sort(ImportedGUIDs);
213-
for (auto &GUID : ImportedGUIDs)
216+
for (auto &[GUID, Type] : ImportedGUIDs) {
214217
AddUint64(GUID);
218+
AddUint8(Type);
219+
}
215220
}
216221

217222
// Include the hash for the resolved ODR.
@@ -281,9 +286,9 @@ void llvm::computeLTOCacheKey(
281286
// Imported functions may introduce new uses of type identifier resolutions,
282287
// so we need to collect their used resolutions as well.
283288
for (const ImportModule &ImpM : ImportModulesVector)
284-
for (auto &ImpF : ImpM.getFunctions()) {
289+
for (auto &[GUID, UnusedImportType] : ImpM.getFunctions()) {
285290
GlobalValueSummary *S =
286-
Index.findSummaryInModule(ImpF, ImpM.getIdentifier());
291+
Index.findSummaryInModule(GUID, ImpM.getIdentifier());
287292
AddUsedThings(S);
288293
// If this is an alias, we also care about any types/etc. that the aliasee
289294
// may reference.
@@ -1395,6 +1400,7 @@ class lto::ThinBackendProc {
13951400
llvm::StringRef ModulePath,
13961401
const std::string &NewModulePath) {
13971402
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
1403+
13981404
std::error_code EC;
13991405
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
14001406
ImportList, ModuleToSummariesForIndex);
@@ -1403,6 +1409,8 @@ class lto::ThinBackendProc {
14031409
sys::fs::OpenFlags::OF_None);
14041410
if (EC)
14051411
return errorCodeToError(EC);
1412+
1413+
// TODO: Serialize declaration bits to bitcode.
14061414
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
14071415

14081416
if (ShouldEmitImportsFiles) {

llvm/lib/LTO/LTOBackend.cpp

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

0 commit comments

Comments
 (0)