shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[ThinLTO] Make SummaryList private (NFC) (#164355)

Browse Source 
In preparation for a follow on change that will require checking every
time a new summary is added to the SummaryList for a GUID, make the
SummaryList private and require all accesses to go through one of two
new interfaces. Most changes are to access the list via the read only
getSummaryList() method, and the few that add new summaries (e.g. while
building the combined summary) use the new addSummary() method.
This commit is contained in:
Teresa Johnson 2025-10-21 06:53:40 -07:00 committed by GitHub
parent 83927a6913
commit 683e2bf059
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 65 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
llvm/include/llvm/IR/ModuleSummaryIndex.h
View File

@ -164,9 +164,24 @@ struct alignas(8) GlobalValueSummaryInfo {
inline GlobalValueSummaryInfo(bool HaveGVs);
/// Access a read-only list of global value summary structures for a
/// particular value held in the GlobalValueMap.
ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
return SummaryList;
}
/// Add a summary corresponding to a global value definition in a module with
/// the corresponding GUID.
void addSummary(std::unique_ptr<GlobalValueSummary> Summary) {
return SummaryList.push_back(std::move(Summary));
}
private:
/// List of global value summary structures for a particular value held
/// in the GlobalValueMap. Requires a vector in the case of multiple
/// COMDAT values of the same name.
/// COMDAT values of the same name, weak symbols, locals of the same name when
/// compiling without sufficient distinguishing path, or (theoretically) hash
/// collisions. Each summary is from a different module.
GlobalValueSummaryList SummaryList;
};
@ -201,7 +216,7 @@ struct ValueInfo {
}
ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
return getRef()->second.SummaryList;
return getRef()->second.getSummaryList();
}
// Even if the index is built with GVs available, we may not have one for
@ -1607,8 +1622,8 @@ public:
for (auto &S : *this) {
// Skip external functions
if (!S.second.SummaryList.size() ||
!isa<FunctionSummary>(S.second.SummaryList.front().get()))
if (!S.second.getSummaryList().size() ||
!isa<FunctionSummary>(S.second.getSummaryList().front().get()))
continue;
discoverNodes(ValueInfo(HaveGVs, &S), FunctionHasParent);
}
@ -1748,7 +1763,7 @@ public:
// Here we have a notionally const VI, but the value it points to is owned
// by the non-const *this.
const_cast<GlobalValueSummaryMapTy::value_type *>(VI.getRef())
->second.SummaryList.push_back(std::move(Summary));
->second.addSummary(std::move(Summary));
}
/// Add an original name for the value of the given GUID.
@ -1937,7 +1952,7 @@ public:
collectDefinedGVSummariesPerModule(Map &ModuleToDefinedGVSummaries) const {
for (const auto &GlobalList : *this) {
auto GUID = GlobalList.first;
for (const auto &Summary : GlobalList.second.SummaryList) {
for (const auto &Summary : GlobalList.second.getSummaryList()) {
ModuleToDefinedGVSummaries[Summary->modulePath()][GUID] = Summary.get();
}
}
@ -2035,7 +2050,7 @@ struct GraphTraits<ModuleSummaryIndex *> : public GraphTraits<ValueInfo> {
std::unique_ptr<GlobalValueSummary> Root =
std::make_unique<FunctionSummary>(I->calculateCallGraphRoot());
GlobalValueSummaryInfo G(I->haveGVs());
G.SummaryList.push_back(std::move(Root));
G.addSummary(std::move(Root));
static auto P =
GlobalValueSummaryMapTy::value_type(GlobalValue::GUID(0), std::move(G));
return ValueInfo(I->haveGVs(), &P);

8
llvm/include/llvm/IR/ModuleSummaryIndexYAML.h
View File

@ -237,7 +237,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
// This is done in fixAliaseeLinks() which is called in
// MappingTraits<ModuleSummaryIndex>::mapping().
ASum->setAliasee(AliaseeVI, /*Aliasee=*/nullptr);
Elem.SummaryList.push_back(std::move(ASum));
Elem.addSummary(std::move(ASum));
continue;
}
SmallVector<ValueInfo, 0> Refs;
@ -246,7 +246,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
auto It = V.try_emplace(RefGUID, /*IsAnalysis=*/false).first;
Refs.push_back(ValueInfo(/*IsAnalysis=*/false, &*It));
}
Elem.SummaryList.push_back(std::make_unique<FunctionSummary>(
Elem.addSummary(std::make_unique<FunctionSummary>(
GVFlags, /*NumInsts=*/0, FunctionSummary::FFlags{}, std::move(Refs),
SmallVector<FunctionSummary::EdgeTy, 0>{}, std::move(GVSum.TypeTests),
std::move(GVSum.TypeTestAssumeVCalls),
@ -260,7 +260,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
static void output(IO &io, GlobalValueSummaryMapTy &V) {
for (auto &P : V) {
std::vector<GlobalValueSummaryYaml> GVSums;
for (auto &Sum : P.second.SummaryList) {
for (auto &Sum : P.second.getSummaryList()) {
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
std::vector<uint64_t> Refs;
Refs.reserve(FSum->refs().size());
@ -295,7 +295,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
}
static void fixAliaseeLinks(GlobalValueSummaryMapTy &V) {
for (auto &P : V) {
for (auto &Sum : P.second.SummaryList) {
for (auto &Sum : P.second.getSummaryList()) {
if (auto *Alias = dyn_cast<AliasSummary>(Sum.get())) {
ValueInfo AliaseeVI = Alias->getAliaseeVI();
auto AliaseeSL = AliaseeVI.getSummaryList();

6
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
View File

@ -1100,12 +1100,12 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
for (auto &GlobalList : Index) {
// Ignore entries for references that are undefined in the current module.
if (GlobalList.second.SummaryList.empty())
if (GlobalList.second.getSummaryList().empty())
continue;
assert(GlobalList.second.SummaryList.size() == 1 &&
assert(GlobalList.second.getSummaryList().size() == 1 &&
"Expected module's index to have one summary per GUID");
auto &Summary = GlobalList.second.SummaryList[0];
auto &Summary = GlobalList.second.getSummaryList()[0];
if (!IsThinLTO) {
Summary->setNotEligibleToImport();
continue;

4
llvm/lib/Analysis/StackSafetyAnalysis.cpp
View File

@ -1136,7 +1136,7 @@ void llvm::generateParamAccessSummary(ModuleSummaryIndex &Index) {
if (!AreStatisticsEnabled())
return;
for (auto &GVS : Index)
for (auto &GV : GVS.second.SummaryList)
for (auto &GV : GVS.second.getSummaryList())
if (FunctionSummary *FS = dyn_cast<FunctionSummary>(GV.get()))
Stat += FS->paramAccesses().size();
};
@ -1147,7 +1147,7 @@ void llvm::generateParamAccessSummary(ModuleSummaryIndex &Index) {
// Convert the ModuleSummaryIndex to a FunctionMap
for (auto &GVS : Index) {
for (auto &GV : GVS.second.SummaryList) {
for (auto &GV : GVS.second.getSummaryList()) {
FunctionSummary *FS = dyn_cast<FunctionSummary>(GV.get());
if (!FS || FS->paramAccesses().empty())
continue;

4
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
View File

@ -235,7 +235,7 @@ public:
return;
for (const auto &GUIDSummaryLists : *Index)
// Examine all summaries for this GUID.
for (auto &Summary : GUIDSummaryLists.second.SummaryList)
for (auto &Summary : GUIDSummaryLists.second.getSummaryList())
if (auto FS = dyn_cast<FunctionSummary>(Summary.get())) {
// For each call in the function summary, see if the call
// is to a GUID (which means it is for an indirect call,
@ -587,7 +587,7 @@ public:
}
} else {
for (auto &Summaries : Index)
for (auto &Summary : Summaries.second.SummaryList)
for (auto &Summary : Summaries.second.getSummaryList())
Callback({Summaries.first, Summary.get()}, false);
}
}

2
llvm/lib/IR/AsmWriter.cpp
View File

@ -3196,7 +3196,7 @@ void AssemblyWriter::printModuleSummaryIndex() {
// for aliasee (then update BitcodeWriter.cpp and remove get/setAliaseeGUID).
for (auto &GlobalList : *TheIndex) {
auto GUID = GlobalList.first;
for (auto &Summary : GlobalList.second.SummaryList)
for (auto &Summary : GlobalList.second.getSummaryList())
SummaryToGUIDMap[Summary.get()] = GUID;
}

12
llvm/lib/IR/ModuleSummaryIndex.cpp
View File

@ -162,7 +162,7 @@ void ModuleSummaryIndex::collectDefinedFunctionsForModule(
StringRef ModulePath, GVSummaryMapTy &GVSummaryMap) const {
for (auto &GlobalList : *this) {
auto GUID = GlobalList.first;
for (auto &GlobSummary : GlobalList.second.SummaryList) {
for (auto &GlobSummary : GlobalList.second.getSummaryList()) {
auto *Summary = dyn_cast_or_null<FunctionSummary>(GlobSummary.get());
if (!Summary)
// Ignore global variable, focus on functions
@ -263,7 +263,7 @@ void ModuleSummaryIndex::propagateAttributes(
DenseSet<ValueInfo> MarkedNonReadWriteOnly;
for (auto &P : *this) {
bool IsDSOLocal = true;
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
if (!isGlobalValueLive(S.get())) {
// computeDeadSymbolsAndUpdateIndirectCalls should have marked all
// copies live. Note that it is possible that there is a GUID collision
@ -273,7 +273,7 @@ void ModuleSummaryIndex::propagateAttributes(
// all copies live we can assert here that all are dead if any copy is
// dead.
assert(llvm::none_of(
P.second.SummaryList,
P.second.getSummaryList(),
[&](const std::unique_ptr<GlobalValueSummary> &Summary) {
return isGlobalValueLive(Summary.get());
}));
@ -308,16 +308,16 @@ void ModuleSummaryIndex::propagateAttributes(
// Mark the flag in all summaries false so that we can do quick check
// without going through the whole list.
for (const std::unique_ptr<GlobalValueSummary> &Summary :
P.second.SummaryList)
P.second.getSummaryList())
Summary->setDSOLocal(false);
}
setWithAttributePropagation();
setWithDSOLocalPropagation();
if (llvm::AreStatisticsEnabled())
for (auto &P : *this)
if (P.second.SummaryList.size())
if (P.second.getSummaryList().size())
if (auto *GVS = dyn_cast<GlobalVarSummary>(
P.second.SummaryList[0]->getBaseObject()))
P.second.getSummaryList()[0]->getBaseObject()))
if (isGlobalValueLive(GVS)) {
if (GVS->maybeReadOnly())
ReadOnlyLiveGVars++;

4
llvm/lib/LTO/LTO.cpp
View File

@ -457,7 +457,7 @@ void llvm::thinLTOResolvePrevailingInIndex(
// when needed.
DenseSet<GlobalValueSummary *> GlobalInvolvedWithAlias;
for (auto &I : Index)
for (auto &S : I.second.SummaryList)
for (auto &S : I.second.getSummaryList())
if (auto AS = dyn_cast<AliasSummary>(S.get()))
GlobalInvolvedWithAlias.insert(&AS->getAliasee());
@ -1182,7 +1182,7 @@ Error LTO::checkPartiallySplit() {
// Otherwise check if there are any recorded in the combined summary from the
// ThinLTO modules.
for (auto &P : ThinLTO.CombinedIndex) {
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
auto *FS = dyn_cast<FunctionSummary>(S.get());
if (!FS)
continue;

4
llvm/lib/LTO/LTOBackend.cpp
View File

@ -770,11 +770,11 @@ bool lto::initImportList(const Module &M,
// via a WriteIndexesThinBackend.
for (const auto &GlobalList : CombinedIndex) {
// Ignore entries for undefined references.
if (GlobalList.second.SummaryList.empty())
if (GlobalList.second.getSummaryList().empty())
continue;
auto GUID = GlobalList.first;
for (const auto &Summary : GlobalList.second.SummaryList) {
for (const auto &Summary : GlobalList.second.getSummaryList()) {
// Skip the summaries for the importing module. These are included to
// e.g. record required linkage changes.
if (Summary->modulePath() == M.getModuleIdentifier())

15
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
View File

@ -101,8 +101,8 @@ static void saveTempBitcode(const Module &TheModule, StringRef TempDir,
WriteBitcodeToFile(TheModule, OS, /* ShouldPreserveUseListOrder */ true);
}
static const GlobalValueSummary *
getFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList) {
static const GlobalValueSummary *getFirstDefinitionForLinker(
ArrayRef<std::unique_ptr<GlobalValueSummary>> GVSummaryList) {
// If there is any strong definition anywhere, get it.
auto StrongDefForLinker = llvm::find_if(
GVSummaryList, [](const std::unique_ptr<GlobalValueSummary> &Summary) {
@ -131,14 +131,15 @@ getFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList) {
static void computePrevailingCopies(
const ModuleSummaryIndex &Index,
DenseMap<GlobalValue::GUID, const GlobalValueSummary *> &PrevailingCopy) {
auto HasMultipleCopies = [&](const GlobalValueSummaryList &GVSummaryList) {
return GVSummaryList.size() > 1;
};
auto HasMultipleCopies =
[&](ArrayRef<std::unique_ptr<GlobalValueSummary>> GVSummaryList) {
return GVSummaryList.size() > 1;
};
for (auto &I : Index) {
if (HasMultipleCopies(I.second.SummaryList))
if (HasMultipleCopies(I.second.getSummaryList()))
PrevailingCopy[I.first] =
getFirstDefinitionForLinker(I.second.SummaryList);
getFirstDefinitionForLinker(I.second.getSummaryList());
}
}

12
llvm/lib/Transforms/IPO/FunctionImport.cpp
View File

@ -1368,13 +1368,13 @@ static void ComputeCrossModuleImportForModuleFromIndexForTest(
FunctionImporter::ImportMapTy &ImportList) {
for (const auto &GlobalList : Index) {
// Ignore entries for undefined references.
if (GlobalList.second.SummaryList.empty())
if (GlobalList.second.getSummaryList().empty())
continue;
auto GUID = GlobalList.first;
assert(GlobalList.second.SummaryList.size() == 1 &&
assert(GlobalList.second.getSummaryList().size() == 1 &&
"Expected individual combined index to have one summary per GUID");
auto &Summary = GlobalList.second.SummaryList[0];
auto &Summary = GlobalList.second.getSummaryList()[0];
// Skip the summaries for the importing module. These are included to
// e.g. record required linkage changes.
if (Summary->modulePath() == ModulePath)
@ -1423,7 +1423,7 @@ void updateValueInfoForIndirectCalls(ModuleSummaryIndex &Index,
void llvm::updateIndirectCalls(ModuleSummaryIndex &Index) {
for (const auto &Entry : Index) {
for (const auto &S : Entry.second.SummaryList) {
for (const auto &S : Entry.second.getSummaryList()) {
if (auto *FS = dyn_cast<FunctionSummary>(S.get()))
updateValueInfoForIndirectCalls(Index, FS);
}
@ -1456,7 +1456,7 @@ void llvm::computeDeadSymbolsAndUpdateIndirectCalls(
// Add values flagged in the index as live roots to the worklist.
for (const auto &Entry : Index) {
auto VI = Index.getValueInfo(Entry);
for (const auto &S : Entry.second.SummaryList) {
for (const auto &S : Entry.second.getSummaryList()) {
if (auto *FS = dyn_cast<FunctionSummary>(S.get()))
updateValueInfoForIndirectCalls(Index, FS);
if (S->isLive()) {
@ -2094,7 +2094,7 @@ static bool doImportingForModuleForTest(
// is only enabled when testing importing via the 'opt' tool, which does
// not do the ThinLink that would normally determine what values to promote.
for (auto &I : *Index) {
for (auto &S : I.second.SummaryList) {
for (auto &S : I.second.getSummaryList()) {
if (GlobalValue::isLocalLinkage(S->linkage()))
S->setLinkage(GlobalValue::ExternalLinkage);
}

4
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
View File

@ -2130,7 +2130,7 @@ bool LowerTypeTestsModule::lower() {
// A set of all functions that are address taken by a live global object.
DenseSet<GlobalValue::GUID> AddressTaken;
for (auto &I : *ExportSummary)
for (auto &GVS : I.second.SummaryList)
for (auto &GVS : I.second.getSummaryList())
if (GVS->isLive())
for (const auto &Ref : GVS->refs()) {
AddressTaken.insert(Ref.getGUID());
@ -2409,7 +2409,7 @@ bool LowerTypeTestsModule::lower() {
}
for (auto &P : *ExportSummary) {
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
if (!ExportSummary->isGlobalValueLive(S.get()))
continue;
if (auto *FS = dyn_cast<FunctionSummary>(S->getBaseObject()))

6
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
View File

@ -928,7 +928,7 @@ void llvm::updateVCallVisibilityInIndex(
// linker, as we have no information on their eventual use.
if (DynamicExportSymbols.count(P.first))
continue;
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
auto *GVar = dyn_cast<GlobalVarSummary>(S.get());
if (!GVar ||
GVar->getVCallVisibility() != GlobalObject::VCallVisibilityPublic)
@ -2413,7 +2413,7 @@ bool DevirtModule::run() {
}
for (auto &P : *ExportSummary) {
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
auto *FS = dyn_cast<FunctionSummary>(S.get());
if (!FS)
continue;
@ -2564,7 +2564,7 @@ void DevirtIndex::run() {
// Collect information from summary about which calls to try to devirtualize.
for (auto &P : ExportSummary) {
for (auto &S : P.second.SummaryList) {
for (auto &S : P.second.getSummaryList()) {
auto *FS = dyn_cast<FunctionSummary>(S.get());
if (!FS)
continue;

2
llvm/tools/llvm-link/llvm-link.cpp
View File

@ -420,7 +420,7 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
// does not do the ThinLink that would normally determine what values to
// promote.
for (auto &I : *Index) {
for (auto &S : I.second.SummaryList) {
for (auto &S : I.second.getSummaryList()) {
if (GlobalValue::isLocalLinkage(S->linkage()))
S->setLinkage(GlobalValue::ExternalLinkage);
}

2
llvm/tools/llvm-lto/llvm-lto.cpp
View File

@ -379,7 +379,7 @@ static void printIndexStats() {
unsigned Calls = 0, Refs = 0, Functions = 0, Alias = 0, Globals = 0;
for (auto &Summaries : *Index) {
for (auto &Summary : Summaries.second.SummaryList) {
for (auto &Summary : Summaries.second.getSummaryList()) {
Refs += Summary->refs().size();
if (auto *FuncSummary = dyn_cast<FunctionSummary>(Summary.get())) {
Functions++;