Skip to content

Revert "Reland "[ThinLTO] Populate declaration import status except for distributed ThinLTO under a default-off new option" (#92718) #94503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Revert "Reland "[ThinLTO] Populate declaration import status except for distributed ThinLTO under a default-off new option" (#92718) #94503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions llvm/include/llvm/IR/ModuleSummaryIndex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,10 +587,6 @@ class GlobalValueSummary {

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

GlobalValueSummary::ImportKind importType() const {
return static_cast<ImportKind>(Flags.ImportType);
}

GlobalValue::VisibilityTypes getVisibility() const {
return (GlobalValue::VisibilityTypes)Flags.Visibility;
}
Expand Down Expand Up @@ -1276,9 +1272,6 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
/// a particular module, and provide efficient access to their summary.
using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;

/// A set of global value summary pointers.
using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;

/// Map of a type GUID to type id string and summary (multimap used
/// in case of GUID conflicts).
using TypeIdSummaryMapTy =
Expand Down
15 changes: 5 additions & 10 deletions llvm/include/llvm/Transforms/IPO/FunctionImport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class Module;
/// based on the provided summary informations.
class FunctionImporter {
public:
/// The functions to import from a source module and their import type.
using FunctionsToImportTy =
DenseMap<GlobalValue::GUID, GlobalValueSummary::ImportKind>;
/// Set of functions to import from a source module. Each entry is a set
/// containing all the GUIDs of all functions to import for a source module.
using FunctionsToImportTy = std::unordered_set<GlobalValue::GUID>;

/// The different reasons selectCallee will chose not to import a
/// candidate.
Expand Down Expand Up @@ -99,13 +99,8 @@ 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.
using ExportSetTy = DenseSet<ValueInfo>;

/// A function of this type is used to load modules referenced by the index.
using ModuleLoaderTy =
Expand Down
32 changes: 12 additions & 20 deletions llvm/lib/LTO/LTO.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ void llvm::computeLTOCacheKey(
support::endian::write64le(Data, I);
Hasher.update(Data);
};
auto AddUint8 = [&](const uint8_t I) {
Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&I, 1));
};
AddString(Conf.CPU);
// FIXME: Hash more of Options. For now all clients initialize Options from
// command-line flags (which is unsupported in production), but may set
Expand Down Expand Up @@ -159,18 +156,18 @@ 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) {
auto GUID = VI.getGUID();
ExportsGUID.push_back(GUID);
}

// Sort the export list elements GUIDs.
llvm::sort(ExportsGUID);
for (auto [GUID, ExportType] : ExportsGUID) {
for (uint64_t GUID : ExportsGUID) {
// The export list can impact the internalization, be conservative here
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
Expand Down Expand Up @@ -202,21 +199,19 @@ void llvm::computeLTOCacheKey(
[](const ImportModule &Lhs, const ImportModule &Rhs) -> bool {
return Lhs.getHash() < Rhs.getHash();
});
std::vector<std::pair<uint64_t, uint8_t>> ImportedGUIDs;
std::vector<uint64_t> ImportedGUIDs;
for (const ImportModule &Entry : ImportModulesVector) {
auto ModHash = Entry.getHash();
Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));

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

ImportedGUIDs.clear();
for (auto &[Fn, ImportType] : Entry.getFunctions())
ImportedGUIDs.push_back(std::make_pair(Fn, ImportType));
for (auto &Fn : Entry.getFunctions())
ImportedGUIDs.push_back(Fn);
llvm::sort(ImportedGUIDs);
for (auto &[GUID, Type] : ImportedGUIDs) {
for (auto &GUID : ImportedGUIDs)
AddUint64(GUID);
AddUint8(Type);
}
}

// Include the hash for the resolved ODR.
Expand Down Expand Up @@ -286,9 +281,9 @@ void llvm::computeLTOCacheKey(
// Imported functions may introduce new uses of type identifier resolutions,
// so we need to collect their used resolutions as well.
for (const ImportModule &ImpM : ImportModulesVector)
for (auto &[GUID, UnusedImportType] : ImpM.getFunctions()) {
for (auto &ImpF : ImpM.getFunctions()) {
GlobalValueSummary *S =
Index.findSummaryInModule(GUID, ImpM.getIdentifier());
Index.findSummaryInModule(ImpF, ImpM.getIdentifier());
AddUsedThings(S);
// If this is an alias, we also care about any types/etc. that the aliasee
// may reference.
Expand Down Expand Up @@ -1400,7 +1395,6 @@ class lto::ThinBackendProc {
llvm::StringRef ModulePath,
const std::string &NewModulePath) {
std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;

std::error_code EC;
gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
ImportList, ModuleToSummariesForIndex);
Expand All @@ -1409,8 +1403,6 @@ class lto::ThinBackendProc {
sys::fs::OpenFlags::OF_None);
if (EC)
return errorCodeToError(EC);

// TODO: Serialize declaration bits to bitcode.
writeIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);

if (ShouldEmitImportsFiles) {
Expand Down
9 changes: 1 addition & 8 deletions llvm/lib/LTO/LTOBackend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,14 +720,7 @@ bool lto::initImportList(const Module &M,
if (Summary->modulePath() == M.getModuleIdentifier())
continue;
// Add an entry to provoke importing by thinBackend.
// Try emplace the entry first. If an entry with the same key already
// exists, set the value to 'std::min(existing-value, new-value)' to make
// sure a definition takes precedence over a declaration.
auto [Iter, Inserted] = ImportList[Summary->modulePath()].try_emplace(
GUID, Summary->importType());

if (!Inserted)
Iter->second = std::min(Iter->second, Summary->importType());
ImportList[Summary->modulePath()].insert(GUID);
}
}
return true;
Expand Down
Loading
Loading