Skip to content

[LTO][NFC] Free the GlobalResolutions map after final use #76780

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.

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
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion llvm/include/llvm/LTO/LTO.h
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ class LTO {
};

// Global mapping from mangled symbol names to resolutions.
StringMap<GlobalResolution> GlobalResolutions;
// Make this an optional to guard against accessing after it has been reset
// (to reduce memory after we're done with it).
std::optional<StringMap<GlobalResolution>> GlobalResolutions;

void addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
ArrayRef<SymbolResolution> Res, unsigned Partition,
Expand Down
36 changes: 23 additions & 13 deletions llvm/lib/LTO/LTO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,9 @@ LTO::LTO(Config Conf, ThinBackend Backend,
unsigned ParallelCodeGenParallelismLevel, LTOKind LTOMode)
: Conf(std::move(Conf)),
RegularLTO(ParallelCodeGenParallelismLevel, this->Conf),
ThinLTO(std::move(Backend)), LTOMode(LTOMode) {}
ThinLTO(std::move(Backend)),
GlobalResolutions(std::make_optional<StringMap<GlobalResolution>>()),
LTOMode(LTOMode) {}

// Requires a destructor for MapVector<BitcodeModule>.
LTO::~LTO() = default;
Expand All @@ -610,7 +612,7 @@ void LTO::addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
assert(ResI != ResE);
SymbolResolution Res = *ResI++;

auto &GlobalRes = GlobalResolutions[Sym.getName()];
auto &GlobalRes = (*GlobalResolutions)[Sym.getName()];
GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr();
if (Res.Prevailing) {
assert(!GlobalRes.Prevailing &&
Expand Down Expand Up @@ -1125,7 +1127,7 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {
// Compute "dead" symbols, we don't want to import/export these!
DenseSet<GlobalValue::GUID> GUIDPreservedSymbols;
DenseMap<GlobalValue::GUID, PrevailingType> GUIDPrevailingResolutions;
for (auto &Res : GlobalResolutions) {
for (auto &Res : *GlobalResolutions) {
// Normally resolution have IR name of symbol. We can do nothing here
// otherwise. See comments in GlobalResolution struct for more details.
if (Res.second.IRName.empty())
Expand Down Expand Up @@ -1169,6 +1171,8 @@ Error LTO::run(AddStreamFn AddStream, FileCache Cache) {

Error Result = runRegularLTO(AddStream);
if (!Result)
// This will reset the GlobalResolutions optional once done with it to
// reduce peak memory before importing.
Result = runThinLTO(AddStream, Cache, GUIDPreservedSymbols);

if (StatsFile)
Expand Down Expand Up @@ -1273,8 +1277,8 @@ Error LTO::runRegularLTO(AddStreamFn AddStream) {
// This returns true when the name is local or not defined. Locals are
// expected to be handled separately.
auto IsVisibleToRegularObj = [&](StringRef name) {
auto It = GlobalResolutions.find(name);
return (It == GlobalResolutions.end() || It->second.VisibleOutsideSummary);
auto It = GlobalResolutions->find(name);
return (It == GlobalResolutions->end() || It->second.VisibleOutsideSummary);
};

// If allowed, upgrade public vcall visibility metadata to linkage unit
Expand All @@ -1291,7 +1295,7 @@ Error LTO::runRegularLTO(AddStreamFn AddStream) {
return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));

if (!Conf.CodeGenOnly) {
for (const auto &R : GlobalResolutions) {
for (const auto &R : *GlobalResolutions) {
GlobalValue *GV =
RegularLTO.CombinedModule->getNamedValue(R.second.IRName);
if (!R.second.isPrevailingIRSymbol())
Expand Down Expand Up @@ -1708,8 +1712,8 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
// This returns true when the name is local or not defined. Locals are
// expected to be handled separately.
auto IsVisibleToRegularObj = [&](StringRef name) {
auto It = GlobalResolutions.find(name);
return (It == GlobalResolutions.end() ||
auto It = GlobalResolutions->find(name);
return (It == GlobalResolutions->end() ||
It->second.VisibleOutsideSummary);
};

Expand Down Expand Up @@ -1739,15 +1743,11 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
ContextDisambiguation.run(ThinLTO.CombinedIndex, isPrevailing);
}

if (Conf.OptLevel > 0)
ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
isPrevailing, ImportLists, ExportLists);

// Figure out which symbols need to be internalized. This also needs to happen
// at -O0 because summary-based DCE is implemented using internalization, and
// we must apply DCE consistently with the full LTO module in order to avoid
// undefined references during the final link.
for (auto &Res : GlobalResolutions) {
for (auto &Res : *GlobalResolutions) {
// If the symbol does not have external references or it is not prevailing,
// then not need to mark it as exported from a ThinLTO partition.
if (Res.second.Partition != GlobalResolution::External ||
Expand All @@ -1760,6 +1760,16 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache,
ExportedGUIDs.insert(GUID);
}

// Reset the GlobalResolutions to deallocate the associated memory, as there
// are no further accesses. We specifically want to do this before computing
// cross module importing, which adds to peak memory via the computed import
// and export lists.
GlobalResolutions.reset();

if (Conf.OptLevel > 0)
ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
isPrevailing, ImportLists, ExportLists);

// Any functions referenced by the jump table in the regular LTO object must
// be exported.
for (auto &Def : ThinLTO.CombinedIndex.cfiFunctionDefs())
Expand Down