[PGO] Use a helper function to find all indirect call-sites

[xur]

Summary

This is another code cleanup suggested by Sean Silva in review of http://reviews.llvm.org/D17864.
It uses a helper function to find all the direct-calls-sites in a function. This makes the caller cleaner.
I also split the code into a separated file as this will be shared by another source file for indirect-call-promotion transformation (D17864).

Thanks,

-Rong

[xur]

Remove dead code.

-Rong

[silvas]

LGTM with a nit.

lib/Transforms/Instrumentation/IndirectCallSiteVisitor.h

36 ↗

(On Diff #52434)

Nit: I think we usually say static inline.

llvm/trunk/lib/Transforms/Instrumentation/IndirectCallSiteVisitor.h

+//===-- IndirectCallSiteVisitor.h - indirect call-sites visitor -----------===//
+//
+//                      The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements defines a visitor class and a helper function that find
+// all indirect call-sites in a function.
+
+#include "llvm/IR/InstVisitor.h"
+#include <vector>
+
+namespace llvm {
+// Visitor class that finds all indirect call sites.
+struct PGOIndirectCallSiteVisitor
+    : public InstVisitor<PGOIndirectCallSiteVisitor> {
+  std::vector<Instruction *> IndirectCallInsts;
+  PGOIndirectCallSiteVisitor() {}
+
+  void visitCallSite(CallSite CS) {
+    if (CS.getCalledFunction() || !CS.getCalledValue())
+      return;
+    Instruction *I = CS.getInstruction();
+    if (CallInst *CI = dyn_cast<CallInst>(I)) {
+      if (CI->isInlineAsm())
+        return;
+    }
+    IndirectCallInsts.push_back(I);
+  }
+};
+
+// Helper function that finds all indirect call sites.
+static inline std::vector<Instruction *> findIndirectCallSites(Function &F) {
+  PGOIndirectCallSiteVisitor ICV;
+  ICV.visit(F);
+  return ICV.IndirectCallInsts;
+}
+}

llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

 // in pass PGOInstrumentationGen. Class PGOUseEdge and UseBBInfo are the derived
 // class of PGOEdge and BBInfo, respectively. They contains extra data structure
 // used in populating profile counters.
 // The MST implementation is in Class CFGMST (CFGMST.h).
 //
 //===----------------------------------------------------------------------===//
 
 #include "CFGMST.h"
+#include "IndirectCallSiteVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/CFG.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InstIterator.h"
-#include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/ProfileData/InstrProfReader.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/Debug.h"
BasicBlock *FuncPGOInstrumentation<Edge, BBInfo>::getInstrBB(Edge *E) {
   unsigned SuccNum = GetSuccessorNumber(SrcBB, DestBB);
   BasicBlock *InstrBB = SplitCriticalEdge(TI, SuccNum);
   assert(InstrBB && "Critical edge is not split");
 
   E->Removed = true;
   return InstrBB;
 }
 
-// Visitor class that finds all indirect call sites.
-struct PGOIndirectCallSiteVisitor
-    : public InstVisitor<PGOIndirectCallSiteVisitor> {
-  std::vector<Instruction *> IndirectCallInsts;
-  PGOIndirectCallSiteVisitor() {}
-
-  void visitCallSite(CallSite CS) {
-    if (CS.getCalledFunction() || !CS.getCalledValue())
-      return;
-    Instruction *I = CS.getInstruction();
-    if (CallInst *CI = dyn_cast<CallInst>(I)) {
-      if (CI->isInlineAsm())
-        return;
-    }
-    IndirectCallInsts.push_back(I);
-  }
-};
-
 // Visit all edge and instrument the edges not in MST, and do value profiling.
 // Critical edges will be split.
 static void instrumentOneFunc(Function &F, Module *M,
                               BranchProbabilityInfo *BPI,
                               BlockFrequencyInfo *BFI) {
   unsigned NumCounters = 0;
   FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, true, BPI, BFI);
   for (auto &E : FuncInfo.MST.AllEdges) {
Builder.CreateCall(
          Builder.getInt64(FuncInfo.FunctionHash), Builder.getInt32(NumCounters),
          Builder.getInt32(I++)});
   }
 
   if (DisableValueProfiling)
     return;
 
   unsigned NumIndirectCallSites = 0;
-  PGOIndirectCallSiteVisitor ICV;
-  ICV.visit(F);
-  for (auto &I : ICV.IndirectCallInsts) {
+  for (auto &I : findIndirectCallSites(F)) {
     CallSite CS(I);
     Value *Callee = CS.getCalledValue();
     DEBUG(dbgs() << "Instrument one indirect call: CallSite Index = "
                  << NumIndirectCallSites << "\n");
     IRBuilder<> Builder(I);
     assert(Builder.GetInsertPoint() != I->getParent()->end() &&
            "Cannot get the Instrumentation point");
     Builder.CreateCall(

 void PGOUseFunc::annotateIndirectCallSites() {
   if (DisableValueProfiling)
     return;
 
   // Create the PGOFuncName meta data.
   createPGOFuncNameMetadata(F);
 
   unsigned IndirectCallSiteIndex = 0;
-  PGOIndirectCallSiteVisitor ICV;
-  ICV.visit(F);
+  auto IndirectCallSites = findIndirectCallSites(F);
   unsigned NumValueSites =
       ProfileRecord.getNumValueSites(IPVK_IndirectCallTarget);
-  if (NumValueSites != ICV.IndirectCallInsts.size()) {
+  if (NumValueSites != IndirectCallSites.size()) {
     std::string Msg =
         std::string("Inconsistent number of indirect call sites: ") +
         F.getName().str();
     auto &Ctx = M->getContext();
     Ctx.diagnose(
         DiagnosticInfoPGOProfile(M->getName().data(), Msg, DS_Warning));
     return;
   }
 
-  for (auto &I : ICV.IndirectCallInsts) {
+  for (auto &I : IndirectCallSites) {
     DEBUG(dbgs() << "Read one indirect call instrumentation: Index="
                  << IndirectCallSiteIndex << " out of " << NumValueSites
                  << "\n");
     annotateValueSite(*M, *I, ProfileRecord, IPVK_IndirectCallTarget,
                       IndirectCallSiteIndex, MaxNumAnnotations);
     IndirectCallSiteIndex++;
   }
 }