[llvm] d8e15dc - [PGO] Minor instrumentation code cleanup (NFC)
Christian Ulmann via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 27 09:10:49 PDT 2023
Author: Christian Ulmann
Date: 2023-04-27T16:10:10Z
New Revision: d8e15dc4ae90b9c585147221761d725b46b6cc4e
URL: https://github.com/llvm/llvm-project/commit/d8e15dc4ae90b9c585147221761d725b46b6cc4e
DIFF: https://github.com/llvm/llvm-project/commit/d8e15dc4ae90b9c585147221761d725b46b6cc4e.diff
LOG: [PGO] Minor instrumentation code cleanup (NFC)
This commit cleans up some parts of the PGO instrumentation. Most
importantly, it removes a template parameter shadowing of a class name
that could lead to confusion.
Reviewed By: gysit
Differential Revision: https://reviews.llvm.org/D149324
Added:
Modified:
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 34fb5ed3bba7..2c73f5a7eeaf 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -440,32 +440,32 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
SelectInstVisitor(Function &Func, bool HasSingleByteCoverage)
: F(Func), HasSingleByteCoverage(HasSingleByteCoverage) {}
- void countSelects(Function &Func) {
+ void countSelects() {
NSIs = 0;
Mode = VM_counting;
- visit(Func);
+ visit(F);
}
// Visit the IR stream and instrument all select instructions. \p
// Ind is a pointer to the counter index variable; \p TotalNC
// is the total number of counters; \p FNV is the pointer to the
// PGO function name var; \p FHash is the function hash.
- void instrumentSelects(Function &Func, unsigned *Ind, unsigned TotalNC,
- GlobalVariable *FNV, uint64_t FHash) {
+ void instrumentSelects(unsigned *Ind, unsigned TotalNC, GlobalVariable *FNV,
+ uint64_t FHash) {
Mode = VM_instrument;
CurCtrIdx = Ind;
TotalNumCtrs = TotalNC;
FuncHash = FHash;
FuncNameVar = FNV;
- visit(Func);
+ visit(F);
}
// Visit the IR stream and annotate all select instructions.
- void annotateSelects(Function &Func, PGOUseFunc *UF, unsigned *Ind) {
+ void annotateSelects(PGOUseFunc *UF, unsigned *Ind) {
Mode = VM_annotate;
UseFunc = UF;
CurCtrIdx = Ind;
- visit(Func);
+ visit(F);
}
void instrumentOneSelectInst(SelectInst &SI);
@@ -479,17 +479,11 @@ struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
unsigned getNumOfSelectInsts() const { return NSIs; }
};
-} // end anonymous namespace
-
-namespace {
-
-/// An MST based instrumentation for PGO
-///
-/// Implements a Minimum Spanning Tree (MST) based instrumentation for PGO
-/// in the function level.
+/// This class implements the CFG edges for the Minimum Spanning Tree (MST)
+/// based instrumentation.
+/// Note that the CFG can be a multi-graph. So there might be multiple edges
+/// with the same SrcBB and DestBB.
struct PGOEdge {
- // This class implements the CFG edges. Note the CFG can be a multi-graph.
- // So there might be multiple edges with same SrcBB and DestBB.
const BasicBlock *SrcBB;
const BasicBlock *DestBB;
uint64_t Weight;
@@ -500,31 +494,26 @@ struct PGOEdge {
PGOEdge(const BasicBlock *Src, const BasicBlock *Dest, uint64_t W = 1)
: SrcBB(Src), DestBB(Dest), Weight(W) {}
- // Return the information string of an edge.
+ /// Return the information string of an edge.
std::string infoString() const {
return (Twine(Removed ? "-" : " ") + (InMST ? " " : "*") +
- (IsCritical ? "c" : " ") + " W=" + Twine(Weight)).str();
+ (IsCritical ? "c" : " ") + " W=" + Twine(Weight))
+ .str();
}
};
-// This class stores the auxiliary information for each BB.
-struct BBInfo {
- BBInfo *Group;
+/// This class stores the auxiliary information for each BB in the MST.
+struct PGOBBInfo {
+ PGOBBInfo *Group;
uint32_t Index;
uint32_t Rank = 0;
- BBInfo(unsigned IX) : Group(this), Index(IX) {}
+ PGOBBInfo(unsigned IX) : Group(this), Index(IX) {}
- // Return the information string of this object.
+ /// Return the information string of this object.
std::string infoString() const {
return (Twine("Index=") + Twine(Index)).str();
}
-
- // Empty function -- only applicable to UseBBInfo.
- void addOutEdge(PGOEdge *E LLVM_ATTRIBUTE_UNUSED) {}
-
- // Empty function -- only applicable to UseBBInfo.
- void addInEdge(PGOEdge *E LLVM_ATTRIBUTE_UNUSED) {}
};
// This class implements the CFG edges. Note the CFG can be a multi-graph.
@@ -581,9 +570,9 @@ template <class Edge, class BBInfo> class FuncPGOInstrumentation {
BBInfo *findBBInfo(const BasicBlock *BB) const { return MST.findBBInfo(BB); }
// Dump edges and BB information.
- void dumpInfo(std::string Str = "") const {
- MST.dumpEdges(dbgs(), Twine("Dump Function ") + FuncName + " Hash: " +
- Twine(FunctionHash) + "\t" + Str);
+ void dumpInfo(StringRef Str = "") const {
+ MST.dumpEdges(dbgs(), Twine("Dump Function ") + FuncName +
+ " Hash: " + Twine(FunctionHash) + "\t" + Str);
}
FuncPGOInstrumentation(
@@ -600,7 +589,7 @@ template <class Edge, class BBInfo> class FuncPGOInstrumentation {
if (BCI && PGOViewBlockCoverageGraph)
BCI->viewBlockCoverageGraph();
// This should be done before CFG hash computation.
- SIVisitor.countSelects(Func);
+ SIVisitor.countSelects();
ValueSites[IPVK_MemOPSize] = VPC.get(IPVK_MemOPSize);
if (!IsCS) {
NumOfPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
@@ -733,7 +722,7 @@ template <class Edge, class BBInfo>
void FuncPGOInstrumentation<Edge, BBInfo>::renameComdatFunction() {
if (!canRenameComdat(F, ComdatMembers))
return;
- std::string OrigName = F.getName().str();
+ StringRef OrigName = F.getName();
std::string NewFuncName =
Twine(F.getName() + "." + Twine(FunctionHash)).str();
F.setName(Twine(NewFuncName));
@@ -765,8 +754,8 @@ void FuncPGOInstrumentation<Edge, BBInfo>::renameComdatFunction() {
}
}
-// Collect all the BBs that will be instruments and return them in
-// InstrumentBBs and setup InEdges/OutEdge for UseBBInfo.
+/// Collect all the BBs that will be instruments and add them to
+/// `InstrumentBBs`.
template <class Edge, class BBInfo>
void FuncPGOInstrumentation<Edge, BBInfo>::getInstrumentBBs(
std::vector<BasicBlock *> &InstrumentBBs) {
@@ -788,18 +777,6 @@ void FuncPGOInstrumentation<Edge, BBInfo>::getInstrumentBBs(
if (InstrBB)
InstrumentBBs.push_back(InstrBB);
}
-
- // Set up InEdges/OutEdges for all BBs.
- for (auto &E : MST.AllEdges) {
- if (E->Removed)
- continue;
- const BasicBlock *SrcBB = E->SrcBB;
- const BasicBlock *DestBB = E->DestBB;
- BBInfo &SrcInfo = getBBInfo(SrcBB);
- BBInfo &DestInfo = getBBInfo(DestBB);
- SrcInfo.addOutEdge(E.get());
- DestInfo.addInEdge(E.get());
- }
}
// Given a CFG E to be instrumented, find which BB to place the instrumented
@@ -905,7 +882,7 @@ static void instrumentOneFunc(
SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, BPI, BFI);
}
- FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(
+ FuncPGOInstrumentation<PGOEdge, PGOBBInfo> FuncInfo(
F, TLI, ComdatMembers, true, BPI, BFI, IsCS, PGOInstrumentEntry,
PGOBlockCoverage);
@@ -956,7 +933,7 @@ static void instrumentOneFunc(
}
// Now instrument select instructions:
- FuncInfo.SIVisitor.instrumentSelects(F, &I, NumCounters, FuncInfo.FuncNameVar,
+ FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, FuncInfo.FuncNameVar,
FuncInfo.FunctionHash);
assert(I == NumCounters);
@@ -1035,7 +1012,7 @@ struct PGOUseEdge : public PGOEdge {
using DirectEdges = SmallVector<PGOUseEdge *, 2>;
// This class stores the auxiliary information for each BB.
-struct UseBBInfo : public BBInfo {
+struct PGOUseBBInfo : public PGOBBInfo {
uint64_t CountValue = 0;
bool CountValid;
int32_t UnknownCountInEdge = 0;
@@ -1043,10 +1020,7 @@ struct UseBBInfo : public BBInfo {
DirectEdges InEdges;
DirectEdges OutEdges;
- UseBBInfo(unsigned IX) : BBInfo(IX), CountValid(false) {}
-
- UseBBInfo(unsigned IX, uint64_t C)
- : BBInfo(IX), CountValue(C), CountValid(true) {}
+ PGOUseBBInfo(unsigned IX) : PGOBBInfo(IX), CountValid(false) {}
// Set the profile count value for this BB.
void setBBInfoCount(uint64_t Value) {
@@ -1057,8 +1031,9 @@ struct UseBBInfo : public BBInfo {
// Return the information string of this object.
std::string infoString() const {
if (!CountValid)
- return BBInfo::infoString();
- return (Twine(BBInfo::infoString()) + " Count=" + Twine(CountValue)).str();
+ return PGOBBInfo::infoString();
+ return (Twine(PGOBBInfo::infoString()) + " Count=" + Twine(CountValue))
+ .str();
}
// Add an OutEdge and update the edge count.
@@ -1141,22 +1116,21 @@ class PGOUseFunc {
InstrProfRecord &getProfileRecord() { return ProfileRecord; }
// Return the auxiliary BB information.
- UseBBInfo &getBBInfo(const BasicBlock *BB) const {
+ PGOUseBBInfo &getBBInfo(const BasicBlock *BB) const {
return FuncInfo.getBBInfo(BB);
}
// Return the auxiliary BB information if available.
- UseBBInfo *findBBInfo(const BasicBlock *BB) const {
+ PGOUseBBInfo *findBBInfo(const BasicBlock *BB) const {
return FuncInfo.findBBInfo(BB);
}
Function &getFunc() const { return F; }
- void dumpInfo(std::string Str = "") const {
- FuncInfo.dumpInfo(Str);
- }
+ void dumpInfo(StringRef Str = "") const { FuncInfo.dumpInfo(Str); }
uint64_t getProgramMaxCount() const { return ProgramMaxCount; }
+
private:
Function &F;
Module *M;
@@ -1164,7 +1138,7 @@ class PGOUseFunc {
ProfileSummaryInfo *PSI;
// This member stores the shared information with class PGOGenFunc.
- FuncPGOInstrumentation<PGOUseEdge, UseBBInfo> FuncInfo;
+ FuncPGOInstrumentation<PGOUseEdge, PGOUseBBInfo> FuncInfo;
// The maximum count value in the profile. This is only used in PGO use
// compilation.
@@ -1192,9 +1166,6 @@ class PGOUseFunc {
// one unknown edge.
void setEdgeCount(DirectEdges &Edges, uint64_t Value);
- // Return FuncName string;
- std::string getFuncName() const { return FuncInfo.FuncName; }
-
// Set the hot/cold inline hints based on the count values.
// FIXME: This function should be removed once the functionality in
// the inliner is implemented.
@@ -1208,6 +1179,24 @@ class PGOUseFunc {
} // end anonymous namespace
+/// Set up InEdges/OutEdges for all BBs in the MST.
+static void
+setupBBInfoEdges(FuncPGOInstrumentation<PGOUseEdge, PGOUseBBInfo> &FuncInfo) {
+ // This is not required when there is block coverage inference.
+ if (FuncInfo.BCI)
+ return;
+ for (auto &E : FuncInfo.MST.AllEdges) {
+ if (E->Removed)
+ continue;
+ const BasicBlock *SrcBB = E->SrcBB;
+ const BasicBlock *DestBB = E->DestBB;
+ PGOUseBBInfo &SrcInfo = FuncInfo.getBBInfo(SrcBB);
+ PGOUseBBInfo &DestInfo = FuncInfo.getBBInfo(DestBB);
+ SrcInfo.addOutEdge(E.get());
+ DestInfo.addInEdge(E.get());
+ }
+}
+
// Visit all the edges and assign the count value for the instrumented
// edges and the BB. Return false on error.
bool PGOUseFunc::setInstrumentedCounts(
@@ -1215,6 +1204,9 @@ bool PGOUseFunc::setInstrumentedCounts(
std::vector<BasicBlock *> InstrumentBBs;
FuncInfo.getInstrumentBBs(InstrumentBBs);
+
+ setupBBInfoEdges(FuncInfo);
+
unsigned NumCounters =
InstrumentBBs.size() + FuncInfo.SIVisitor.getNumOfSelectInsts();
// The number of counters here should match the number of counters
@@ -1228,7 +1220,7 @@ bool PGOUseFunc::setInstrumentedCounts(
uint32_t I = 0;
for (BasicBlock *InstrBB : InstrumentBBs) {
uint64_t CountValue = CountFromProfile[I++];
- UseBBInfo &Info = getBBInfo(InstrBB);
+ PGOUseBBInfo &Info = getBBInfo(InstrBB);
// If we reach here, we know that we have some nonzero count
// values in this function. The entry count should not be 0.
// Fix it if necessary.
@@ -1253,7 +1245,7 @@ bool PGOUseFunc::setInstrumentedCounts(
if (E->Removed || E->InMST)
continue;
const BasicBlock *SrcBB = E->SrcBB;
- UseBBInfo &SrcInfo = getBBInfo(SrcBB);
+ PGOUseBBInfo &SrcInfo = getBBInfo(SrcBB);
// If only one out-edge, the edge profile count should be the same as BB
// profile count.
@@ -1261,7 +1253,7 @@ bool PGOUseFunc::setInstrumentedCounts(
setEdgeCount(E.get(), SrcInfo.CountValue);
else {
const BasicBlock *DestBB = E->DestBB;
- UseBBInfo &DestInfo = getBBInfo(DestBB);
+ PGOUseBBInfo &DestInfo = getBBInfo(DestBB);
// If only one in-edge, the edge profile count should be the same as BB
// profile count.
if (DestInfo.CountValid && DestInfo.InEdges.size() == 1)
@@ -1292,8 +1284,7 @@ void PGOUseFunc::setEdgeCount(DirectEdges &Edges, uint64_t Value) {
}
// Emit function metadata indicating PGO profile mismatch.
-static void annotateFunctionWithHashMismatch(Function &F,
- LLVMContext &ctx) {
+static void annotateFunctionWithHashMismatch(Function &F, LLVMContext &ctx) {
const char MetadataName[] = "instr_prof_hash_mismatch";
SmallVector<Metadata *, 2> Names;
// If this metadata already exists, ignore.
@@ -1301,7 +1292,7 @@ static void annotateFunctionWithHashMismatch(Function &F,
if (Existing) {
MDTuple *Tuple = cast<MDTuple>(Existing);
for (const auto &N : Tuple->operands()) {
- if (cast<MDString>(N.get())->getString() == MetadataName)
+ if (cast<MDString>(N.get())->getString() == MetadataName)
return;
Names.push_back(N.get());
}
@@ -1644,8 +1635,9 @@ bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
dbgs() << "Inconsistent number of counts, skipping this function");
Ctx.diagnose(DiagnosticInfoPGOProfile(
M->getName().data(),
- Twine("Inconsistent number of counts in ") + F.getName().str()
- + Twine(": the profile may be stale or there is a function name collision."),
+ Twine("Inconsistent number of counts in ") + F.getName().str() +
+ Twine(": the profile may be stale or there is a function name "
+ "collision."),
DS_Warning));
return false;
}
@@ -1772,7 +1764,7 @@ void PGOUseFunc::populateCounters() {
// For efficient traversal, it's better to start from the end as most
// of the instrumented edges are at the end.
for (auto &BB : reverse(F)) {
- UseBBInfo *Count = findBBInfo(&BB);
+ PGOUseBBInfo *Count = findBBInfo(&BB);
if (Count == nullptr)
continue;
if (!Count->CountValid) {
@@ -1811,7 +1803,7 @@ void PGOUseFunc::populateCounters() {
}
LLVM_DEBUG(dbgs() << "Populate counts in " << NumPasses << " passes.\n");
- (void) NumPasses;
+ (void)NumPasses;
#ifndef NDEBUG
// Assert every BB has a valid counter.
for (auto &BB : F) {
@@ -1837,7 +1829,7 @@ void PGOUseFunc::populateCounters() {
markFunctionAttributes(FuncEntryCount, FuncMaxCount);
// Now annotate select instructions
- FuncInfo.SIVisitor.annotateSelects(F, this, &CountPosition);
+ FuncInfo.SIVisitor.annotateSelects(this, &CountPosition);
assert(CountPosition == ProfileCountSize);
LLVM_DEBUG(FuncInfo.dumpInfo("after reading profile."));
@@ -1861,7 +1853,7 @@ void PGOUseFunc::setBranchWeights() {
continue;
// We have a non-zero Branch BB.
- const UseBBInfo &BBCountInfo = getBBInfo(&BB);
+ const PGOUseBBInfo &BBCountInfo = getBBInfo(&BB);
unsigned Size = BBCountInfo.OutEdges.size();
SmallVector<uint64_t, 2> EdgeCounts(Size, 0);
uint64_t MaxCount = 0;
@@ -1886,11 +1878,11 @@ void PGOUseFunc::setBranchWeights() {
// when there is no exit block and the code exits via a noreturn function.
auto &Ctx = M->getContext();
Ctx.diagnose(DiagnosticInfoPGOProfile(
- M->getName().data(),
- Twine("Profile in ") + F.getName().str() +
- Twine(" partially ignored") +
- Twine(", possibly due to the lack of a return path."),
- DS_Warning));
+ M->getName().data(),
+ Twine("Profile in ") + F.getName().str() +
+ Twine(" partially ignored") +
+ Twine(", possibly due to the lack of a return path."),
+ DS_Warning));
}
}
}
@@ -1912,7 +1904,7 @@ void PGOUseFunc::annotateIrrLoopHeaderWeights() {
// duplication.
if (BFI->isIrrLoopHeader(&BB) || isIndirectBrTarget(&BB)) {
Instruction *TI = BB.getTerminator();
- const UseBBInfo &BBCountInfo = getBBInfo(&BB);
+ const PGOUseBBInfo &BBCountInfo = getBBInfo(&BB);
setIrrLoopHeaderMetadata(M, TI, BBCountInfo.CountValue);
}
}
@@ -1995,8 +1987,8 @@ void PGOUseFunc::annotateValueSites(uint32_t Kind) {
Ctx.diagnose(DiagnosticInfoPGOProfile(
M->getName().data(),
Twine("Inconsistent number of value sites for ") +
- Twine(ValueProfKindDescr[Kind]) +
- Twine(" profiling in \"") + F.getName().str() +
+ Twine(ValueProfKindDescr[Kind]) + Twine(" profiling in \"") +
+ F.getName().str() +
Twine("\", possibly due to the use of a stale profile."),
DS_Warning));
return;
@@ -2174,7 +2166,7 @@ static void verifyFuncBFI(PGOUseFunc &Func, LoopInfo &LI,
BlockFrequencyInfo NBFI(F, NBPI, LI);
// bool PrintFunc = false;
bool HotBBOnly = PGOVerifyHotBFI;
- std::string Msg;
+ StringRef Msg;
OptimizationRemarkEmitter ORE(&F);
unsigned BBNum = 0, BBMisMatchNum = 0, NonZeroBBNum = 0;
@@ -2474,7 +2466,7 @@ PreservedAnalyses PGOInstrumentationUse::run(Module &M,
static std::string getSimpleNodeName(const BasicBlock *Node) {
if (!Node->getName().empty())
- return std::string(Node->getName());
+ return Node->getName().str();
std::string SimpleNodeName;
raw_string_ostream OS(SimpleNodeName);
@@ -2483,8 +2475,7 @@ static std::string getSimpleNodeName(const BasicBlock *Node) {
}
void llvm::setProfMetadata(Module *M, Instruction *TI,
- ArrayRef<uint64_t> EdgeCounts,
- uint64_t MaxCount) {
+ ArrayRef<uint64_t> EdgeCounts, uint64_t MaxCount) {
MDBuilder MDB(M->getContext());
assert(MaxCount > 0 && "Bad max count");
uint64_t Scale = calculateCountScale(MaxCount);
@@ -2573,7 +2564,7 @@ template <> struct DOTGraphTraits<PGOUseFunc *> : DefaultDOTGraphTraits {
raw_string_ostream OS(Result);
OS << getSimpleNodeName(Node) << ":\\l";
- UseBBInfo *BI = Graph->findBBInfo(Node);
+ PGOUseBBInfo *BI = Graph->findBBInfo(Node);
OS << "Count : ";
if (BI && BI->CountValid)
OS << BI->CountValue << "\\l";
More information about the llvm-commits
mailing list