Skip to content

Commit

Permalink
IR: Use pointers instead of GUIDs to represent edges in the module su…
Browse files Browse the repository at this point in the history 
…mmary. 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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@302108 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@pcc
pcc committed May 4, 2017
1 parent 7395a71 commit df22060
Show file tree
Hide file tree
Showing 13 changed files with 208 additions and 221 deletions.
152 changes: 71 additions & 81 deletions include/llvm/IR/ModuleSummaryIndex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,58 +45,54 @@ struct CalleeInfo {
}
};

/// Struct to hold value either by GUID or GlobalValue*. Values in combined
/// indexes as well as indirect calls are GUIDs, all others are GlobalValues.
struct ValueInfo {
/// The value representation used in this instance.
enum ValueInfoKind {
VI_GUID,
VI_Value,
};
class GlobalValueSummary;

/// Union of the two possible value types.
union ValueUnion {
GlobalValue::GUID Id;
const GlobalValue *GV;
ValueUnion(GlobalValue::GUID Id) : Id(Id) {}
ValueUnion(const GlobalValue *GV) : GV(GV) {}
};
typedef std::vector<std::unique_ptr<GlobalValueSummary>> GlobalValueSummaryList;

/// The value being represented.
ValueUnion TheValue;
/// The value representation.
ValueInfoKind Kind;
/// Constructor for a GUID value
ValueInfo(GlobalValue::GUID Id = 0) : TheValue(Id), Kind(VI_GUID) {}
/// Constructor for a GlobalValue* value
ValueInfo(const GlobalValue *V) : TheValue(V), Kind(VI_Value) {}
/// Accessor for GUID value
GlobalValue::GUID getGUID() const {
assert(Kind == VI_GUID && "Not a GUID type");
return TheValue.Id;
}
/// Accessor for GlobalValue* value
const GlobalValue *getValue() const {
assert(Kind == VI_Value && "Not a Value type");
return TheValue.GV;
}
bool isGUID() const { return Kind == VI_GUID; }
struct GlobalValueSummaryInfo {
/// The GlobalValue corresponding to this summary. This is only used in
/// per-module summaries.
const GlobalValue *GV = nullptr;

/// 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.
GlobalValueSummaryList SummaryList;
};

template <> struct DenseMapInfo<ValueInfo> {
static inline ValueInfo getEmptyKey() { return ValueInfo((GlobalValue *)-1); }
static inline ValueInfo getTombstoneKey() {
return ValueInfo((GlobalValue *)-2);
/// Map from global value GUID to corresponding summary structures. Use a
/// std::map rather than a DenseMap so that pointers to the map's value_type
/// (which are used by ValueInfo) are not invalidated by insertion. Also it will
/// likely incur less overhead, as the value type is not very small and the size
/// of the map is unknown, resulting in inefficiencies due to repeated
/// insertions and resizing.
typedef std::map<GlobalValue::GUID, GlobalValueSummaryInfo>
GlobalValueSummaryMapTy;

/// Struct that holds a reference to a particular GUID in a global value
/// summary.
struct ValueInfo {
const GlobalValueSummaryMapTy::value_type *Ref = nullptr;
ValueInfo() = default;
ValueInfo(const GlobalValueSummaryMapTy::value_type *Ref) : Ref(Ref) {}
operator bool() const { return Ref; }

GlobalValue::GUID getGUID() const { return Ref->first; }
const GlobalValue *getValue() const { return Ref->second.GV; }
ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
return Ref->second.SummaryList;
}
static bool isEqual(ValueInfo L, ValueInfo R) {
if (L.isGUID() != R.isGUID())
return false;
return L.isGUID() ? (L.getGUID() == R.getGUID())
: (L.getValue() == R.getValue());
};

template <> struct DenseMapInfo<ValueInfo> {
static inline ValueInfo getEmptyKey() {
return ValueInfo((GlobalValueSummaryMapTy::value_type *)-1);
}
static unsigned getHashValue(ValueInfo I) {
return I.isGUID() ? I.getGUID() : (uintptr_t)I.getValue();
static inline ValueInfo getTombstoneKey() {
return ValueInfo((GlobalValueSummaryMapTy::value_type *)-2);
}
static bool isEqual(ValueInfo L, ValueInfo R) { return L.Ref == R.Ref; }
static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.Ref; }
};

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

/// 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.
typedef std::vector<std::unique_ptr<GlobalValueSummary>> GlobalValueSummaryList;

/// Map from global value GUID to corresponding summary structures.
/// Use a std::map rather than a DenseMap since it will likely incur
/// less overhead, as the value type is not very small and the size
/// of the map is unknown, resulting in inefficiencies due to repeated
/// insertions and resizing.
typedef std::map<GlobalValue::GUID, GlobalValueSummaryList>
GlobalValueSummaryMapTy;

/// Type used for iterating through the global value summary map.
typedef GlobalValueSummaryMapTy::const_iterator const_gvsummary_iterator;
typedef GlobalValueSummaryMapTy::iterator gvsummary_iterator;
Expand Down Expand Up @@ -532,28 +515,34 @@ class ModuleSummaryIndex {
// YAML I/O support.
friend yaml::MappingTraits<ModuleSummaryIndex>;

GlobalValueSummaryMapTy::value_type *
getOrInsertValuePtr(GlobalValue::GUID GUID) {
return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo{}).first;
}

public:
gvsummary_iterator begin() { return GlobalValueMap.begin(); }
const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
gvsummary_iterator end() { return GlobalValueMap.end(); }
const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
size_t size() const { return GlobalValueMap.size(); }

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

/// Get the list of global value summary objects for a given value name.
const const_gvsummary_iterator
findGlobalValueSummaryList(StringRef ValueName) const {
return GlobalValueMap.find(GlobalValue::getGUID(ValueName));
/// Return a ValueInfo for \p GUID.
ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID) {
return ValueInfo(getOrInsertValuePtr(GUID));
}

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

/// Return the GUID for \p OriginalId in the OidGuidMap.
Expand All @@ -565,17 +554,18 @@ class ModuleSummaryIndex {
/// Add a global value summary for a value of the given name.
void addGlobalValueSummary(StringRef ValueName,
std::unique_ptr<GlobalValueSummary> Summary) {
addOriginalName(GlobalValue::getGUID(ValueName),
Summary->getOriginalName());
GlobalValueMap[GlobalValue::getGUID(ValueName)].push_back(
std::move(Summary));
addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
std::move(Summary));
}

/// Add a global value summary for a value of the given GUID.
void addGlobalValueSummary(GlobalValue::GUID ValueGUID,
/// Add a global value summary for the given ValueInfo.
void addGlobalValueSummary(ValueInfo VI,
std::unique_ptr<GlobalValueSummary> Summary) {
addOriginalName(ValueGUID, Summary->getOriginalName());
GlobalValueMap[ValueGUID].push_back(std::move(Summary));
addOriginalName(VI.getGUID(), Summary->getOriginalName());
// 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.Ref)
->second.SummaryList.push_back(std::move(Summary));
}

/// Add an original name for the value of the given GUID.
Expand All @@ -593,16 +583,16 @@ class ModuleSummaryIndex {
/// not found.
GlobalValueSummary *findSummaryInModule(GlobalValue::GUID ValueGUID,
StringRef ModuleId) const {
auto CalleeInfoList = findGlobalValueSummaryList(ValueGUID);
if (CalleeInfoList == end()) {
auto CalleeInfo = getValueInfo(ValueGUID);
if (!CalleeInfo) {
return nullptr; // This function does not have a summary
}
auto Summary =
llvm::find_if(CalleeInfoList->second,
llvm::find_if(CalleeInfo.getSummaryList(),
[&](const std::unique_ptr<GlobalValueSummary> &Summary) {
return Summary->modulePath() == ModuleId;
});
if (Summary == CalleeInfoList->second.end())
if (Summary == CalleeInfo.getSummaryList().end())
return nullptr;
return Summary->get();
}
Expand Down
4 changes: 2 additions & 2 deletions include/llvm/IR/ModuleSummaryIndexYAML.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
for (auto &FSum : FSums) {
GlobalValueSummary::GVFlags GVFlags(GlobalValue::ExternalLinkage, false,
false);
Elem.push_back(llvm::make_unique<FunctionSummary>(
Elem.SummaryList.push_back(llvm::make_unique<FunctionSummary>(
GVFlags, 0, ArrayRef<ValueInfo>{},
ArrayRef<FunctionSummary::EdgeTy>{}, std::move(FSum.TypeTests),
std::move(FSum.TypeTestAssumeVCalls),
Expand All @@ -213,7 +213,7 @@ template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
static void output(IO &io, GlobalValueSummaryMapTy &V) {
for (auto &P : V) {
std::vector<FunctionSummaryYaml> FSums;
for (auto &Sum : P.second) {
for (auto &Sum : P.second.SummaryList) {
if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get()))
FSums.push_back(FunctionSummaryYaml{
FSum->type_tests(), FSum->type_test_assume_vcalls(),
Expand Down
43 changes: 23 additions & 20 deletions lib/Analysis/ModuleSummaryAnalysis.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ using namespace llvm;
// Walk through the operands of a given User via worklist iteration and populate
// the set of GlobalValue references encountered. Invoked either on an
// Instruction or a GlobalVariable (which walks its initializer).
static void findRefEdges(const User *CurUser, SetVector<ValueInfo> &RefEdges,
static void findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
SetVector<ValueInfo> &RefEdges,
SmallPtrSet<const User *, 8> &Visited) {
SmallVector<const User *, 32> Worklist;
Worklist.push_back(CurUser);
Expand All @@ -61,7 +62,7 @@ static void findRefEdges(const User *CurUser, SetVector<ValueInfo> &RefEdges,
// the reference set unless it is a callee. Callees are handled
// specially by WriteFunction and are added to a separate list.
if (!(CS && CS.isCallee(&OI)))
RefEdges.insert(GV);
RefEdges.insert(Index.getOrInsertValueInfo(GV));
continue;
}
Worklist.push_back(Operand);
Expand Down Expand Up @@ -198,7 +199,7 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
if (isa<DbgInfoIntrinsic>(I))
continue;
++NumInsts;
findRefEdges(&I, RefEdges, Visited);
findRefEdges(Index, &I, RefEdges, Visited);
auto CS = ImmutableCallSite(&I);
if (!CS)
continue;
Expand Down Expand Up @@ -239,7 +240,9 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
// to record the call edge to the alias in that case. Eventually
// an alias summary will be created to associate the alias and
// aliasee.
CallGraphEdges[cast<GlobalValue>(CalledValue)].updateHotness(Hotness);
CallGraphEdges[Index.getOrInsertValueInfo(
cast<GlobalValue>(CalledValue))]
.updateHotness(Hotness);
} else {
// Skip inline assembly calls.
if (CI && CI->isInlineAsm())
Expand All @@ -254,15 +257,16 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
ICallAnalysis.getPromotionCandidatesForInstruction(
&I, NumVals, TotalCount, NumCandidates);
for (auto &Candidate : CandidateProfileData)
CallGraphEdges[Candidate.Value].updateHotness(
getHotness(Candidate.Count, PSI));
CallGraphEdges[Index.getOrInsertValueInfo(Candidate.Value)]
.updateHotness(getHotness(Candidate.Count, PSI));
}
}

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

bool NonRenamableLocal = isNonRenamableLocal(F);
bool NotEligibleForImport =
Expand All @@ -288,7 +292,7 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
DenseSet<GlobalValue::GUID> &CantBePromoted) {
SetVector<ValueInfo> RefEdges;
SmallPtrSet<const User *, 8> Visited;
findRefEdges(&V, RefEdges, Visited);
findRefEdges(Index, &V, RefEdges, Visited);
bool NonRenamableLocal = isNonRenamableLocal(V);
GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal,
/* LiveRoot = */ false);
Expand Down Expand Up @@ -317,12 +321,9 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,

// Set LiveRoot flag on entries matching the given value name.
static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
auto SummaryList =
Index.findGlobalValueSummaryList(GlobalValue::getGUID(Name));
if (SummaryList == Index.end())
return;
for (auto &Summary : SummaryList->second)
Summary->setLiveRoot();
if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
for (auto &Summary : VI.getSummaryList())
Summary->setLiveRoot();
}

ModuleSummaryIndex llvm::buildModuleSummaryIndex(
Expand Down Expand Up @@ -446,12 +447,16 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
}

for (auto &GlobalList : Index) {
assert(GlobalList.second.size() == 1 &&
// Ignore entries for references that are undefined in the current module.
if (GlobalList.second.SummaryList.empty())
continue;

assert(GlobalList.second.SummaryList.size() == 1 &&
"Expected module's index to have one summary per GUID");
auto &Summary = GlobalList.second[0];
auto &Summary = GlobalList.second.SummaryList[0];
bool AllRefsCanBeExternallyReferenced =
llvm::all_of(Summary->refs(), [&](const ValueInfo &VI) {
return !CantBePromoted.count(VI.getValue()->getGUID());
return !CantBePromoted.count(VI.getGUID());
});
if (!AllRefsCanBeExternallyReferenced) {
Summary->setNotEligibleToImport();
Expand All @@ -461,9 +466,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex(
if (auto *FuncSummary = dyn_cast<FunctionSummary>(Summary.get())) {
bool AllCallsCanBeExternallyReferenced = llvm::all_of(
FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) {
auto GUID = Edge.first.isGUID() ? Edge.first.getGUID()
: Edge.first.getValue()->getGUID();
return !CantBePromoted.count(GUID);
return !CantBePromoted.count(Edge.first.getGUID());
});
if (!AllCallsCanBeExternallyReferenced)
Summary->setNotEligibleToImport();
Expand Down
Loading

0 comments on commit df22060

Please sign in to comment.