Skip to content

Commit 9667b91

Browse files
pccpcc
committed
Re-apply r302108, "IR: Use pointers instead of GUIDs to represent edges in the module summary. NFCI."
with a fix for the clang backend. llvm-svn: 302176
1 parent 3207d30 commit 9667b91

15 files changed

+218
-223
lines changed

‎clang/lib/CodeGen/BackendUtil.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -974,10 +974,14 @@ static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,
974974
// via a WriteIndexesThinBackend.
975975
FunctionImporter::ImportMapTy ImportList;
976976
for (auto &GlobalList : *CombinedIndex) {
977+
// Ignore entries for undefined references.
978+
if (GlobalList.second.SummaryList.empty())
979+
continue;
980+
977981
auto GUID = GlobalList.first;
978-
assert(GlobalList.second.size() == 1 &&
982+
assert(GlobalList.second.SummaryList.size() == 1 &&
979983
"Expected individual combined index to have one summary per GUID");
980-
auto &Summary = GlobalList.second[0];
984+
auto &Summary = GlobalList.second.SummaryList[0];
981985
// Skip the summaries for the importing module. These are included to
982986
// e.g. record required linkage changes.
983987
if (Summary->modulePath() == M->getModuleIdentifier())

‎clang/test/CodeGen/thinlto_backend.ll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,12 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
3535
target triple = "x86_64-unknown-linux-gnu"
3636

3737
declare void @f2()
38+
declare i8* @f3()
3839

3940
define void @f1() {
4041
call void @f2()
42+
; Make sure that the backend can handle undefined references.
43+
; Do an indirect call so that the undefined ref shows up in the combined index.
44+
call void bitcast (i8*()* @f3 to void()*)()
4145
ret void
4246
}

‎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)