Skip to content

Commit 5f85a9d

Browse files
committed
IR: Use pointers instead of GUIDs to represent edges in the module summary. NFCI.
When profiling a no-op incremental link of Chromium I found that the functions computeImportForFunction and computeDeadSymbols were consuming roughly 10% of the profile. The goal of this change is to improve the performance of those functions by changing the map lookups that they were previously doing into pointer dereferences. This is achieved by changing the ValueInfo data structure to be a pointer to an element of the global value map owned by ModuleSummaryIndex, and changing reference lists in the GlobalValueSummary to hold ValueInfos instead of GUIDs. This means that a ValueInfo will take a client directly to the summary list for a given GUID. Differential Revision: https://reviews.llvm.org/D32471 llvm-svn: 302108
1 parent 7c4eafa commit 5f85a9d

File tree

14 files changed

+210
-223
lines changed

14 files changed

+210
-223
lines changed

clang/lib/CodeGen/BackendUtil.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -975,9 +975,9 @@ static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,
975975
FunctionImporter::ImportMapTy ImportList;
976976
for (auto &GlobalList : *CombinedIndex) {
977977
auto GUID = GlobalList.first;
978-
assert(GlobalList.second.size() == 1 &&
978+
assert(GlobalList.second.SummaryList.size() == 1 &&
979979
"Expected individual combined index to have one summary per GUID");
980-
auto &Summary = GlobalList.second[0];
980+
auto &Summary = GlobalList.second.SummaryList[0];
981981
// Skip the summaries for the importing module. These are included to
982982
// e.g. record required linkage changes.
983983
if (Summary->modulePath() == M->getModuleIdentifier())

llvm/include/llvm/IR/ModuleSummaryIndex.h

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -45,58 +45,54 @@ struct CalleeInfo {
4545
}
4646
};
4747

48-
/// Struct to hold value either by GUID or GlobalValue*. Values in combined
49-
/// indexes as well as indirect calls are GUIDs, all others are GlobalValues.
50-
struct ValueInfo {
51-
/// The value representation used in this instance.
52-
enum ValueInfoKind {
53-
VI_GUID,
54-
VI_Value,
55-
};
48+
class GlobalValueSummary;
5649

57-
/// Union of the two possible value types.
58-
union ValueUnion {
59-
GlobalValue::GUID Id;
60-
const GlobalValue *GV;
61-
ValueUnion(GlobalValue::GUID Id) : Id(Id) {}
62-
ValueUnion(const GlobalValue *GV) : GV(GV) {}
63-
};
50+
typedef std::vector<std::unique_ptr<GlobalValueSummary>> GlobalValueSummaryList;
6451

65-
/// The value being represented.
66-
ValueUnion TheValue;
67-
/// The value representation.
68-
ValueInfoKind Kind;
69-
/// Constructor for a GUID value
70-
ValueInfo(GlobalValue::GUID Id = 0) : TheValue(Id), Kind(VI_GUID) {}
71-
/// Constructor for a GlobalValue* value
72-
ValueInfo(const GlobalValue *V) : TheValue(V), Kind(VI_Value) {}
73-
/// Accessor for GUID value
74-
GlobalValue::GUID getGUID() const {
75-
assert(Kind == VI_GUID && "Not a GUID type");
76-
return TheValue.Id;
77-
}
78-
/// Accessor for GlobalValue* value
79-
const GlobalValue *getValue() const {
80-
assert(Kind == VI_Value && "Not a Value type");
81-
return TheValue.GV;
82-
}
83-
bool isGUID() const { return Kind == VI_GUID; }
52+
struct GlobalValueSummaryInfo {
53+
/// The GlobalValue corresponding to this summary. This is only used in
54+
/// per-module summaries.
55+
const GlobalValue *GV = nullptr;
56+
57+
/// List of global value summary structures for a particular value held
58+
/// in the GlobalValueMap. Requires a vector in the case of multiple
59+
/// COMDAT values of the same name.
60+
GlobalValueSummaryList SummaryList;
8461
};
8562

86-
template <> struct DenseMapInfo<ValueInfo> {
87-
static inline ValueInfo getEmptyKey() { return ValueInfo((GlobalValue *)-1); }
88-
static inline ValueInfo getTombstoneKey() {
89-
return ValueInfo((GlobalValue *)-2);
63+
/// Map from global value GUID to corresponding summary structures. Use a
64+
/// std::map rather than a DenseMap so that pointers to the map's value_type
65+
/// (which are used by ValueInfo) are not invalidated by insertion. Also it will
66+
/// likely incur less overhead, as the value type is not very small and the size
67+
/// of the map is unknown, resulting in inefficiencies due to repeated
68+
/// insertions and resizing.
69+
typedef std::map<GlobalValue::GUID, GlobalValueSummaryInfo>
70+
GlobalValueSummaryMapTy;
71+
72+
/// Struct that holds a reference to a particular GUID in a global value
73+
/// summary.
74+
struct ValueInfo {
75+
const GlobalValueSummaryMapTy::value_type *Ref = nullptr;
76+
ValueInfo() = default;
77+
ValueInfo(const GlobalValueSummaryMapTy::value_type *Ref) : Ref(Ref) {}
78+
operator bool() const { return Ref; }
79+
80+
GlobalValue::GUID getGUID() const { return Ref->first; }
81+
const GlobalValue *getValue() const { return Ref->second.GV; }
82+
ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
83+
return Ref->second.SummaryList;
9084
}
91-
static bool isEqual(ValueInfo L, ValueInfo R) {
92-
if (L.isGUID() != R.isGUID())
93-
return false;
94-
return L.isGUID() ? (L.getGUID() == R.getGUID())
95-
: (L.getValue() == R.getValue());
85+
};
86+
87+
template <> struct DenseMapInfo<ValueInfo> {
88+
static inline ValueInfo getEmptyKey() {
89+
return ValueInfo((GlobalValueSummaryMapTy::value_type *)-1);
9690
}
97-
static unsigned getHashValue(ValueInfo I) {
98-
return I.isGUID() ? I.getGUID() : (uintptr_t)I.getValue();
91+
static inline ValueInfo getTombstoneKey() {
92+
return ValueInfo((GlobalValueSummaryMapTy::value_type *)-2);
9993
}
94+
static bool isEqual(ValueInfo L, ValueInfo R) { return L.Ref == R.Ref; }
95+
static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.Ref; }
10096
};
10197

10298
/// \brief Function and variable summary information to aid decisions and
@@ -483,19 +479,6 @@ struct TypeIdSummary {
483479
/// 160 bits SHA1
484480
typedef std::array<uint32_t, 5> ModuleHash;
485481

486-
/// List of global value summary structures for a particular value held
487-
/// in the GlobalValueMap. Requires a vector in the case of multiple
488-
/// COMDAT values of the same name.
489-
typedef std::vector<std::unique_ptr<GlobalValueSummary>> GlobalValueSummaryList;
490-
491-
/// Map from global value GUID to corresponding summary structures.
492-
/// Use a std::map rather than a DenseMap since it will likely incur
493-
/// less overhead, as the value type is not very small and the size
494-
/// of the map is unknown, resulting in inefficiencies due to repeated
495-
/// insertions and resizing.
496-
typedef std::map<GlobalValue::GUID, GlobalValueSummaryList>
497-
GlobalValueSummaryMapTy;
498-
499482
/// Type used for iterating through the global value summary map.
500483
typedef GlobalValueSummaryMapTy::const_iterator const_gvsummary_iterator;
501484
typedef GlobalValueSummaryMapTy::iterator gvsummary_iterator;
@@ -532,28 +515,34 @@ class ModuleSummaryIndex {
532515
// YAML I/O support.
533516
friend yaml::MappingTraits<ModuleSummaryIndex>;
534517

518+
GlobalValueSummaryMapTy::value_type *
519+
getOrInsertValuePtr(GlobalValue::GUID GUID) {
520+
return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo{}).first;
521+
}
522+
535523
public:
536524
gvsummary_iterator begin() { return GlobalValueMap.begin(); }
537525
const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
538526
gvsummary_iterator end() { return GlobalValueMap.end(); }
539527
const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
540528
size_t size() const { return GlobalValueMap.size(); }
541529

542-
/// Get the list of global value summary objects for a given value name.
543-
const GlobalValueSummaryList &getGlobalValueSummaryList(StringRef ValueName) {
544-
return GlobalValueMap[GlobalValue::getGUID(ValueName)];
530+
/// Return a ValueInfo for GUID if it exists, otherwise return ValueInfo().
531+
ValueInfo getValueInfo(GlobalValue::GUID GUID) const {
532+
auto I = GlobalValueMap.find(GUID);
533+
return ValueInfo(I == GlobalValueMap.end() ? nullptr : &*I);
545534
}
546535

547-
/// Get the list of global value summary objects for a given value name.
548-
const const_gvsummary_iterator
549-
findGlobalValueSummaryList(StringRef ValueName) const {
550-
return GlobalValueMap.find(GlobalValue::getGUID(ValueName));
536+
/// Return a ValueInfo for \p GUID.
537+
ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID) {
538+
return ValueInfo(getOrInsertValuePtr(GUID));
551539
}
552540

553-
/// Get the list of global value summary objects for a given value GUID.
554-
const const_gvsummary_iterator
555-
findGlobalValueSummaryList(GlobalValue::GUID ValueGUID) const {
556-
return GlobalValueMap.find(ValueGUID);
541+
/// Return a ValueInfo for \p GV and mark it as belonging to GV.
542+
ValueInfo getOrInsertValueInfo(const GlobalValue *GV) {
543+
auto VP = getOrInsertValuePtr(GV->getGUID());
544+
VP->second.GV = GV;
545+
return ValueInfo(VP);
557546
}
558547

559548
/// Return the GUID for \p OriginalId in the OidGuidMap.
@@ -565,17 +554,18 @@ class ModuleSummaryIndex {
565554
/// Add a global value summary for a value of the given name.
566555
void addGlobalValueSummary(StringRef ValueName,
567556
std::unique_ptr<GlobalValueSummary> Summary) {
568-
addOriginalName(GlobalValue::getGUID(ValueName),
569-
Summary->getOriginalName());
570-
GlobalValueMap[GlobalValue::getGUID(ValueName)].push_back(
571-
std::move(Summary));
557+
addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
558+
std::move(Summary));
572559
}
573560

574-
/// Add a global value summary for a value of the given GUID.
575-
void addGlobalValueSummary(GlobalValue::GUID ValueGUID,
561+
/// Add a global value summary for the given ValueInfo.
562+
void addGlobalValueSummary(ValueInfo VI,
576563
std::unique_ptr<GlobalValueSummary> Summary) {
577-
addOriginalName(ValueGUID, Summary->getOriginalName());
578-
GlobalValueMap[ValueGUID].push_back(std::move(Summary));
564+
addOriginalName(VI.getGUID(), Summary->getOriginalName());
565+
// Here we have a notionally const VI, but the value it points to is owned
566+
// by the non-const *this.
567+
const_cast<GlobalValueSummaryMapTy::value_type *>(VI.Ref)
568+
->second.SummaryList.push_back(std::move(Summary));
579569
}
580570

581571
/// Add an original name for the value of the given GUID.
@@ -593,16 +583,16 @@ class ModuleSummaryIndex {
593583
/// not found.
594584
GlobalValueSummary *findSummaryInModule(GlobalValue::GUID ValueGUID,
595585
StringRef ModuleId) const {
596-
auto CalleeInfoList = findGlobalValueSummaryList(ValueGUID);
597-
if (CalleeInfoList == end()) {
586+
auto CalleeInfo = getValueInfo(ValueGUID);
587+
if (!CalleeInfo) {
598588
return nullptr; // This function does not have a summary
599589
}
600590
auto Summary =
601-
llvm::find_if(CalleeInfoList->second,
591+
llvm::find_if(CalleeInfo.getSummaryList(),
602592
[&](const std::unique_ptr<GlobalValueSummary> &Summary) {
603593
return Summary->modulePath() == ModuleId;
604594
});
605-
if (Summary == CalleeInfoList->second.end())
595+
if (Summary == CalleeInfo.getSummaryList().end())
606596
return nullptr;
607597
return Summary->get();
608598
}

llvm/include/llvm/IR/ModuleSummaryIndexYAML.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
201201
for (auto &FSum : FSums) {
202202
GlobalValueSummary::GVFlags GVFlags(GlobalValue::ExternalLinkage, false,
203203
false);
204-
Elem.push_back(llvm::make_unique<FunctionSummary>(
204+
Elem.SummaryList.push_back(llvm::make_unique<FunctionSummary>(
205205
GVFlags, 0, ArrayRef<ValueInfo>{},
206206
ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
207207
std::move(FSum.TypeTestAssumeVCalls),
@@ -213,7 +213,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
213213
static void output(IO &io, GlobalValueSummaryMapTy &V) {
214214
for (auto &P : V) {
215215
std::vector<FunctionSummaryYaml> FSums;
216-
for (auto &Sum : P.second) {
216+
for (auto &Sum : P.second.SummaryList) {
217217
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get()))
218218
FSums.push_back(FunctionSummaryYaml{
219219
FSum->type_tests(), FSum->type_test_assume_vcalls(),

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ using namespace llvm;
3737
// Walk through the operands of a given User via worklist iteration and populate
3838
// the set of GlobalValue references encountered. Invoked either on an
3939
// Instruction or a GlobalVariable (which walks its initializer).
40-
static void findRefEdges(const User *CurUser, SetVector<ValueInfo> &RefEdges,
40+
static void findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
41+
SetVector<ValueInfo> &RefEdges,
4142
SmallPtrSet<const User *, 8> &Visited) {
4243
SmallVector<const User *, 32> Worklist;
4344
Worklist.push_back(CurUser);
@@ -61,7 +62,7 @@ static void findRefEdges(const User *CurUser, SetVector<ValueInfo> &RefEdges,
6162
// the reference set unless it is a callee. Callees are handled
6263
// specially by WriteFunction and are added to a separate list.
6364
if (!(CS && CS.isCallee(&OI)))
64-
RefEdges.insert(GV);
65+
RefEdges.insert(Index.getOrInsertValueInfo(GV));
6566
continue;
6667
}
6768
Worklist.push_back(Operand);
@@ -198,7 +199,7 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
198199
if (isa<DbgInfoIntrinsic>(I))
199200
continue;
200201
++NumInsts;
201-
findRefEdges(&I, RefEdges, Visited);
202+
findRefEdges(Index, &I, RefEdges, Visited);
202203
auto CS = ImmutableCallSite(&I);
203204
if (!CS)
204205
continue;
@@ -239,7 +240,9 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
239240
// to record the call edge to the alias in that case. Eventually
240241
// an alias summary will be created to associate the alias and
241242
// aliasee.
242-
CallGraphEdges[cast<GlobalValue>(CalledValue)].updateHotness(Hotness);
243+
CallGraphEdges[Index.getOrInsertValueInfo(
244+
cast<GlobalValue>(CalledValue))]
245+
.updateHotness(Hotness);
243246
} else {
244247
// Skip inline assembly calls.
245248
if (CI && CI->isInlineAsm())
@@ -254,15 +257,16 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
254257
ICallAnalysis.getPromotionCandidatesForInstruction(
255258
&I, NumVals, TotalCount, NumCandidates);
256259
for (auto &Candidate : CandidateProfileData)
257-
CallGraphEdges[Candidate.Value].updateHotness(
258-
getHotness(Candidate.Count, PSI));
260+
CallGraphEdges[Index.getOrInsertValueInfo(Candidate.Value)]
261+
.updateHotness(getHotness(Candidate.Count, PSI));
259262
}
260263
}
261264

262265
// Explicit add hot edges to enforce importing for designated GUIDs for
263266
// sample PGO, to enable the same inlines as the profiled optimized binary.
264267
for (auto &I : F.getImportGUIDs())
265-
CallGraphEdges[I].updateHotness(CalleeInfo::HotnessType::Hot);
268+
CallGraphEdges[Index.getOrInsertValueInfo(I)].updateHotness(
269+
CalleeInfo::HotnessType::Hot);
266270

267271
bool NonRenamableLocal = isNonRenamableLocal(F);
268272
bool NotEligibleForImport =
@@ -288,7 +292,7 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
288292
DenseSet<GlobalValue::GUID> &CantBePromoted) {
289293
SetVector<ValueInfo> RefEdges;
290294
SmallPtrSet<const User *, 8> Visited;
291-
findRefEdges(&V, RefEdges, Visited);
295+
findRefEdges(Index, &V, RefEdges, Visited);
292296
bool NonRenamableLocal = isNonRenamableLocal(V);
293297
GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal,
294298
/* LiveRoot = */ false);
@@ -317,12 +321,9 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
317321

318322
// Set LiveRoot flag on entries matching the given value name.
319323
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
320-
auto SummaryList =
321-
Index.findGlobalValueSummaryList(GlobalValue::getGUID(Name));
322-
if (SummaryList == Index.end())
323-
return;
324-
for (auto &Summary : SummaryList->second)
325-
Summary->setLiveRoot();
324+
if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
325+
for (auto &Summary : VI.getSummaryList())
326+
Summary->setLiveRoot();
326327
}
327328

328329
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
@@ -446,12 +447,16 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
446447
}
447448

448449
for (auto &GlobalList : Index) {
449-
assert(GlobalList.second.size() == 1 &&
450+
// Ignore entries for references that are undefined in the current module.
451+
if (GlobalList.second.SummaryList.empty())
452+
continue;
453+
454+
assert(GlobalList.second.SummaryList.size() == 1 &&
450455
"Expected module's index to have one summary per GUID");
451-
auto &Summary = GlobalList.second[0];
456+
auto &Summary = GlobalList.second.SummaryList[0];
452457
bool AllRefsCanBeExternallyReferenced =
453458
llvm::all_of(Summary->refs(), [&](const ValueInfo &VI) {
454-
return !CantBePromoted.count(VI.getValue()->getGUID());
459+
return !CantBePromoted.count(VI.getGUID());
455460
});
456461
if (!AllRefsCanBeExternallyReferenced) {
457462
Summary->setNotEligibleToImport();
@@ -461,9 +466,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
461466
if (auto *FuncSummary = dyn_cast<FunctionSummary>(Summary.get())) {
462467
bool AllCallsCanBeExternallyReferenced = llvm::all_of(
463468
FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) {
464-
auto GUID = Edge.first.isGUID() ? Edge.first.getGUID()
465-
: Edge.first.getValue()->getGUID();
466-
return !CantBePromoted.count(GUID);
469+
return !CantBePromoted.count(Edge.first.getGUID());
467470
});
468471
if (!AllCallsCanBeExternallyReferenced)
469472
Summary->setNotEligibleToImport();

0 commit comments

Comments
 (0)