[PGO] Minor instrumentation code cleanup (NFC)

[Dinistro]

Summary

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.

[gysit]

LGTM!

[Dinistro]

Add BCI check to make this really NFC and, thus, fix the bolt builds.

[Dinistro]

The CI crash is caused by another change in bolt that only occurs sporadically. Potential causing revision: https://reviews.llvm.org/D149014

[jgorbe]

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

725

I'm seeing a test failure on test/Transforms/PGOProfile/comdat_rename.ll in our internal buils, that bisects to this patch:

error: CHECK: expected string not found in input
; CHECK: @f = weak alias void (), ptr @f.[[SINGLEBB_HASH]]
         ^
<stdin>:15:37: note: scanning from here
$aef.742261418966908927 = comdat any
                                    ^
<stdin>:15:37: note: with "SINGLEBB_HASH" equal to "742261418966908927"
$aef.742261418966908927 = comdat any
                                    ^
<stdin>:30:2: note: possible intended match here
@"\CD" = weak alias void (), ptr @f.742261418966908927
 ^

Unfortunately, I haven't been able to reproduce in my clone of upstream sources so I don't have any easy repro instructions yet. Turning OrigName back into a std::string fixes the problem for me, and I believe (but haven't confirmed yet) that the problem comes because OrigName is now a reference to F.getName(), but then we call F.setName() and invalidate the reference before calling GlobalAlias::create. Possibly the debug standard library in my failing build is writing 0xCD into the deleted buffer to mark it as invalid, causing the @f/@"\CD" mismatch reported by the test.

[Dinistro]

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

725

Sorry for the inconvenience and thanks for hunting down the bug. Should be fixed in https://reviews.llvm.org/rGc67079f1be713fa558b3773562bd1eeb156cadbd

[jgorbe]

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

725

Thank you!

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

struct SelectInstVisitor : public InstVisitor<SelectInstVisitor> {
   GlobalVariable *FuncNameVar = nullptr;
   uint64_t FuncHash = 0;
   PGOUseFunc *UseFunc = nullptr;
   bool HasSingleByteCoverage;
 
   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);
   void annotateOneSelectInst(SelectInst &SI);
 
   // Visit \p SI instruction and perform tasks according to visit mode.
   void visitSelectInst(SelectInst &SI);
 
   // Return the number of select instructions. This needs be called after
   // countSelects().
   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;
   bool InMST = false;
   bool Removed = false;
   bool IsCritical = false;
 
   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.
 template <class Edge, class BBInfo> class FuncPGOInstrumentation {
 private:
   Function &F;
 
   // Is this is context-sensitive instrumentation.
public:
 
   // Return the auxiliary BB information.
   BBInfo &getBBInfo(const BasicBlock *BB) const { return MST.getBBInfo(BB); }
 
   // Return the auxiliary BB information if available.
   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(
       Function &Func, TargetLibraryInfo &TLI,
       std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
       bool CreateGlobalVar = false, BranchProbabilityInfo *BPI = nullptr,
       BlockFrequencyInfo *BFI = nullptr, bool IsCS = false,
       bool InstrumentFuncEntry = true, bool HasSingleByteCoverage = false)
       : F(Func), IsCS(IsCS), ComdatMembers(ComdatMembers), VPC(Func, TLI),
         TLI(TLI), ValueSites(IPVK_Last + 1),
         SIVisitor(Func, HasSingleByteCoverage),
         MST(F, InstrumentFuncEntry, BPI, BFI),
         BCI(constructBCI(Func, HasSingleByteCoverage, InstrumentFuncEntry)) {
     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();
       NumOfPGOMemIntrinsics += ValueSites[IPVK_MemOPSize].size();
       NumOfPGOBB += MST.BBInfos.size();
       ValueSites[IPVK_IndirectCallTarget] = VPC.get(IPVK_IndirectCallTarget);
     } else {
       NumOfCSPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
static bool canRenameComdat(
   return true;
 }
 
 // Append the CFGHash to the Comdat function name.
 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));
   GlobalAlias::create(GlobalValue::WeakAnyLinkage, OrigName, &F);
   FuncName = Twine(FuncName + "." + Twine(FunctionHash)).str();
   Comdat *NewComdat;
   Module *M = F.getParent();
   // For AvailableExternallyLinkage functions, change the linkage to
void FuncPGOInstrumentation<Edge, BBInfo>::renameComdatFunction() {
   NewComdat->setSelectionKind(OrigComdat->getSelectionKind());
 
   for (auto &&CM : make_range(ComdatMembers.equal_range(OrigComdat))) {
     // Must be a function.
     cast<Function>(CM.second)->setComdat(NewComdat);
   }
 }
 
-// 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) {
   if (BCI) {
     for (auto &BB : F)
       if (BCI->shouldInstrumentBlock(BB))
         InstrumentBBs.push_back(&BB);
     return;
   }
 
   // Use a worklist as we will update the vector during the iteration.
   std::vector<Edge *> EdgeList;
   EdgeList.reserve(MST.AllEdges.size());
   for (auto &E : MST.AllEdges)
     EdgeList.push_back(E.get());
 
   for (auto &E : EdgeList) {
     BasicBlock *InstrBB = getInstrBB(E);
     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
 // code. The function will split the critical edge if necessary.
 template <class Edge, class BBInfo>
 BasicBlock *FuncPGOInstrumentation<Edge, BBInfo>::getInstrBB(Edge *E) {
   if (E->InMST || E->Removed)
     return nullptr;
static void instrumentOneFunc(
     std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
     bool IsCS) {
   if (!PGOBlockCoverage) {
     // Split indirectbr critical edges here before computing the MST rather than
     // later in getInstrBB() to avoid invalidating it.
     SplitIndirectBrCriticalEdges(F, /*IgnoreBlocksWithoutPHI=*/false, BPI, BFI);
   }
 
-  FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(
+  FuncPGOInstrumentation<PGOEdge, PGOBBInfo> FuncInfo(
       F, TLI, ComdatMembers, true, BPI, BFI, IsCS, PGOInstrumentEntry,
       PGOBlockCoverage);
 
   Type *I8PtrTy = Type::getInt8PtrTy(M->getContext());
   auto Name = ConstantExpr::getBitCast(FuncInfo.FuncNameVar, I8PtrTy);
   auto CFGHash = ConstantInt::get(Type::getInt64Ty(M->getContext()),
                                   FuncInfo.FunctionHash);
   if (PGOFunctionEntryCoverage) {
for (auto *InstrBB : InstrumentBBs) {
     Builder.CreateCall(
         Intrinsic::getDeclaration(M, PGOBlockCoverage
                                          ? Intrinsic::instrprof_cover
                                          : Intrinsic::instrprof_increment),
         {Name, CFGHash, Builder.getInt32(NumCounters), Builder.getInt32(I++)});
   }
 
   // Now instrument select instructions:
-  FuncInfo.SIVisitor.instrumentSelects(F, &I, NumCounters, FuncInfo.FuncNameVar,
+  FuncInfo.SIVisitor.instrumentSelects(&I, NumCounters, FuncInfo.FuncNameVar,
                                        FuncInfo.FunctionHash);
   assert(I == NumCounters);
 
   if (DisableValueProfiling)
     return;
 
   NumOfPGOICall += FuncInfo.ValueSites[IPVK_IndirectCallTarget].size();
 
std::string infoString() const {
     return (Twine(PGOEdge::infoString()) + "  Count=" + Twine(CountValue))
         .str();
   }
 };
 
 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;
   int32_t UnknownCountOutEdge = 0;
   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) {
     CountValue = Value;
     CountValid = true;
   }
 
   // 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.
   void addOutEdge(PGOUseEdge *E) {
     OutEdges.push_back(E);
     UnknownCountOutEdge++;
   }
 
public:
 
   // Return the function hash.
   uint64_t getFuncHash() const { return FuncInfo.FunctionHash; }
 
   // Return the profile record for this function;
   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;
   BlockFrequencyInfo *BFI;
   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.
   uint64_t ProgramMaxCount;
 
   // Position of counter that remains to be read.
   uint32_t CountPosition = 0;
 
private:
 
   // Find the Instrumented BB and set the value. Return false on error.
   bool setInstrumentedCounts(const std::vector<uint64_t> &CountFromProfile);
 
   // Set the edge counter value for the unknown edge -- there should be only
   // 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.
   void markFunctionAttributes(uint64_t EntryCount, uint64_t MaxCount) {
     if (PSI->isHotCount(EntryCount))
       FreqAttr = FFA_Hot;
     else if (PSI->isColdCount(MaxCount))
       FreqAttr = FFA_Cold;
   }
 };
 
 } // 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(
     const std::vector<uint64_t> &CountFromProfile) {
 
   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
   // in profile. Return if they mismatch.
   if (NumCounters != CountFromProfile.size()) {
     return false;
   }
   auto *FuncEntry = &*F.begin();
 
   // Set the profile count to the Instrumented BBs.
   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.
     if (InstrBB == FuncEntry && CountValue == 0)
       CountValue = 1;
     Info.setBBInfoCount(CountValue);
   }
   ProfileCountSize = CountFromProfile.size();
   CountPosition = I;
 
   // Set the edge count and update the count of unknown edges for BBs.
   auto setEdgeCount = [this](PGOUseEdge *E, uint64_t Value) -> void {
     E->setEdgeCount(Value);
     this->getBBInfo(E->SrcBB).UnknownCountOutEdge--;
     this->getBBInfo(E->DestBB).UnknownCountInEdge--;
   };
 
   // Set the profile count the Instrumented edges. There are BBs that not in
   // MST but not instrumented. Need to set the edge count value so that we can
   // populate the profile counts later.
   for (auto &E : FuncInfo.MST.AllEdges) {
     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.
     if (SrcInfo.CountValid && SrcInfo.OutEdges.size() == 1)
       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)
         setEdgeCount(E.get(), DestInfo.CountValue);
     }
     if (E->CountValid)
       continue;
     // E's count should have been set from profile. If not, this meenas E skips
for (auto &E : Edges) {
     getBBInfo(E->SrcBB).UnknownCountOutEdge--;
     getBBInfo(E->DestBB).UnknownCountInEdge--;
     return;
   }
   llvm_unreachable("Cannot find the unknown count edge");
 }
 
 // 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.
   auto *Existing = F.getMetadata(LLVMContext::MD_annotation);
   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());
     }
   }
 
   MDBuilder MDB(ctx);
   Names.push_back(MDB.createString(MetadataName));
   MDNode *MD = MDTuple::get(ctx, Names);
bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
   getBBInfo(nullptr).UnknownCountOutEdge = 2;
   getBBInfo(nullptr).UnknownCountInEdge = 2;
 
   if (!setInstrumentedCounts(CountFromProfile)) {
     LLVM_DEBUG(
         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;
   }
   ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
   return true;
 }
 
 void PGOUseFunc::populateCoverage(IndexedInstrProfReader *PGOReader) {
void PGOUseFunc::populateCounters() {
   unsigned NumPasses = 0;
   while (Changes) {
     NumPasses++;
     Changes = false;
 
     // 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) {
         if (Count->UnknownCountOutEdge == 0) {
           Count->CountValue = sumEdgeCount(Count->OutEdges);
           Count->CountValid = true;
           Changes = true;
         } else if (Count->UnknownCountInEdge == 0) {
for (auto &BB : reverse(F)) {
           setEdgeCount(Count->InEdges, Total);
           Changes = true;
         }
       }
     }
   }
 
   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) {
     auto BI = findBBInfo(&BB);
     if (BI == nullptr)
       continue;
     assert(BI->CountValid && "BB count is not valid");
   }
#endif
 
   // Fix the obviously inconsistent entry count.
   if (FuncMaxCount > 0 && FuncEntryCount == 0)
     FuncEntryCount = 1;
   F.setEntryCount(ProfileCount(FuncEntryCount, Function::PCT_Real));
   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."));
 }
 
 // Assign the scaled count values to the BB with multiple out edges.
 void PGOUseFunc::setBranchWeights() {
   // Generate MD_prof metadata for every branch instruction.
   LLVM_DEBUG(dbgs() << "\nSetting branch weights for func " << F.getName()
                     << " IsCS=" << IsCS << "\n");
   for (auto &BB : F) {
     Instruction *TI = BB.getTerminator();
     if (TI->getNumSuccessors() < 2)
       continue;
     if (!(isa<BranchInst>(TI) || isa<SwitchInst>(TI) ||
           isa<IndirectBrInst>(TI) || isa<InvokeInst>(TI) ||
           isa<CallBrInst>(TI)))
       continue;
 
     if (getBBInfo(&BB).CountValue == 0)
       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;
     for (unsigned s = 0; s < Size; s++) {
       const PGOUseEdge *E = BBCountInfo.OutEdges[s];
       const BasicBlock *SrcBB = E->SrcBB;
       const BasicBlock *DestBB = E->DestBB;
       if (DestBB == nullptr)
         continue;
       unsigned SuccNum = GetSuccessorNumber(SrcBB, DestBB);
       uint64_t EdgeCount = E->CountValue;
       if (EdgeCount > MaxCount)
         MaxCount = EdgeCount;
       EdgeCounts[SuccNum] = EdgeCount;
     }
 
     if (MaxCount)
       setProfMetadata(M, TI, EdgeCounts, MaxCount);
     else {
       // A zero MaxCount can come about when we have a BB with a positive
       // count, and whose successor blocks all have 0 count. This can happen
       // 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));
     }
   }
 }
 
 static bool isIndirectBrTarget(BasicBlock *BB) {
   for (BasicBlock *Pred : predecessors(BB)) {
     if (isa<IndirectBrInst>(Pred->getTerminator()))
       return true;
   }
   return false;
 }
 
 void PGOUseFunc::annotateIrrLoopHeaderWeights() {
   LLVM_DEBUG(dbgs() << "\nAnnotating irreducible loop header weights.\n");
   // Find irr loop headers
   for (auto &BB : F) {
     // As a heuristic also annotate indrectbr targets as they have a high chance
     // to become an irreducible loop header after the indirectbr tail
     // 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);
     }
   }
 }
 
 void SelectInstVisitor::instrumentOneSelectInst(SelectInst &SI) {
   Module *M = F.getParent();
   IRBuilder<> Builder(&SI);
void PGOUseFunc::annotateValueSites(uint32_t Kind) {
   unsigned ValueSiteIndex = 0;
   auto &ValueSites = FuncInfo.ValueSites[Kind];
   unsigned NumValueSites = ProfileRecord.getNumValueSites(Kind);
   if (NumValueSites != ValueSites.size()) {
     auto &Ctx = M->getContext();
     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;
   }
 
   for (VPCandidateInfo &I : ValueSites) {
     LLVM_DEBUG(dbgs() << "Read one value site profile (kind = " << Kind
                       << "): Index = " << ValueSiteIndex << " out of "

 static void verifyFuncBFI(PGOUseFunc &Func, LoopInfo &LI,
                           BranchProbabilityInfo &NBPI,
                           uint64_t HotCountThreshold,
                           uint64_t ColdCountThreshold) {
   Function &F = Func.getFunc();
   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;
   for (auto &BBI : F) {
     uint64_t CountValue = 0;
     uint64_t BFICountValue = 0;
 
     if (Func.getBBInfo(&BBI).CountValid)
if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName, *FS,
                             LookupTLI, LookupBPI, LookupBFI, PSI, IsCS))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
 }
 
 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);
   Node->printAsOperand(OS, false);
   return OS.str();
 }
 
 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);
   SmallVector<unsigned, 4> Weights;
   for (const auto &ECI : EdgeCounts)
     Weights.push_back(scaleBranchCount(ECI, Scale));
 
   LLVM_DEBUG(dbgs() << "Weight is: "; for (const auto &W
static std::string getGraphName(const PGOUseFunc *G) {
     return std::string(G->getFunc().getName());
   }
 
   std::string getNodeLabel(const BasicBlock *Node, const PGOUseFunc *Graph) {
     std::string Result;
     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";
     else
       OS << "Unknown\\l";
 
     if (!PGOInstrSelect)
       return Result;