[SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen

[xur]

Summary

This is split from patch https://reviews.llvm.org/D95698

Break SampleProfileLoader into to a base and a derived class.
Base class (SampleProfileLoaderBaseImpl) includes the common
code for IR and MachineIR samples loader.
It will be templatelized in the following patch.

Inline and Probe related code will remain in the derived class of
SampleProfileLoader and stays in SampleProfile.cpp.

We need to refactor some functions:
(1) getInstWeight() to enable the code sharing -- put the core into getInstWeightImpl().
(2) emitAnnotation() and propagateWeights() to carve out the code specific to SampleProfileLoader.
(3) make getInstWeight() and findFunctionSamples() virtual and override

in SampleProfileLoader as they need to access the fields in the derived class.

In a following patch, I will move the base class to a new header file in
llvm/include/llvm/ProfileData

After that, I will make the base class a template. Some function will be specialized in
IR and MachineIR, respectively.

[davidxl]

Instead of having inlining keyword, it is better to move the out of line definitions as inline bodies in the class.

[davidxl]

Is the intention to move them (inline bodies) into the header? For those functions that are not, leave them as being defined out of line. Otherwise moving them into class body is a preparation for the next step (moving to header).

[xur]

I meant the functions move the header file. In the header file, we can
achieve the same by adding an inline keyword to the out of class function
definition (vs in class definition). Out of class definitions have better
readability because some of the functions are really large.

[davidxl]

Ok.

question: it seems SampleCoverageTracker implementation is independent of IR/MIR level. Do we need to templatize it ?

For SampleProfileLoader class, is there any part of of the code that is independent of the IR level which can be extracted into a base class (to avoid pulling a lot of definitions into header)?

[wmi]

I meant the functions move the header file. In the header file, we can achieve the same by adding an inline keyword to the out of class function definition (vs in class definition). Out of class definitions have better readability because some of the functions are really large.

Have you considered to define SampleProfileImpl.cc in addition to SampleProfileImpl.h so some large function can be put in .cc file and don't need inline keyword? But they need explicit instantiation.

[davidxl]

Good to know that SampleCoverageTracker can remain as it is (not templatized). Is it possible to not move it into the header? There are a few advantages:

1. reduce the size of the patch and merge churns;
2. a little better in compile time as the functions don't need to be built when building FlowSensitiveSampleLoader.

There are a few ways -- only move the class body to the header (so that inline keyword is not needed) or introduce some wrapper/helper function (taking tracker reference) with forward declaration of TrackerType.

[xur]

Here is the another way to refactor the SampleProfileLoader:

I tried Wei's idea of putting all the function bodies into a cpp file and explicit instantiate for IR and MachineIR.
That idea did not work: I place a new cpp file into llvm/lib/ProfileData. But that creates a circular dependency b/w ProfileData and CodeGen.
Current implementation in sample loader requires a complete definition of machine IR classes.
It might be possible to break the dependency by rewriting some of the code. But that would be too many changes. The benefit does not justify the cost -- we still have duplicated object for IR and MachineIR and we just combine them into one object.

[dblaikie]

(sorry if this comment is off-base, I haven't quite understood/followed the reviews, but some of the discussion here seems potentially problematic, so I'll say this... )

If code is being moved into a header to avoid layering/circular dependency violations, that's not OK. Since the layering violation would still exist (from a software design perspective, though not one a linker would diagnose/trip over). Google's build system, for instance, does enforce header layering & we use/support that to avoid creating header-only dependencies.

Another way of thinking about it: We shouldn't create a situation where it's non-trivial to move code between headers and implementation files. That should always be simple/trivial to do.

[davidxl]

I think what Rong means is that

1. moving code to a header is for code sharing between IR and codeGenPass
2. Wei's idea of avoiding the header move can lead to circular dependencies.

In other words, doing 1) is not to achieve circular dependency avoidance,
but just using 2) as an alternative way is not working.

[dblaikie]

In D95832#2547119, @davidxl wrote:

I think what Rong means is that

1. moving code to a header is for code sharing between IR and codeGenPass
2. Wei's idea of avoiding the header move can lead to circular dependencies.

In other words, doing 1) is not to achieve circular dependency avoidance,
but just using 2) as an alternative way is not working.

I'm not quite following - if there's no .cc file where the implementation of the header could live, if we wanted to write some of it out of line, that seems like there is a circular dependency at the source/design level even if it doesn't result in linker errors. It sounds like (2) is showing that (1) has a hidden circular dependency.

That idea did not work: I place a new cpp file into llvm/lib/ProfileData. But that creates a circular dependency b/w ProfileData and CodeGen.

Oh, I think I understand now - these are (going to be) templates, and so long as they remain templates that are implicitly instantiated, there's no dependency problem - but if we tried to explicitly instantiate the CodeGen version inside ProfileData, that would create a circular dependency. *nod*

Yes, using out-of-class-but-still marked 'inline' definitions (once they're templates they don't even need the inline keyword) may help readability by making the class easier to read through without long function bodies interleaved. (the only other option would be to put the implementation in a whole separate file and have some place in each library that includes both interface and implementation headers and performs the explicit instantiation there (in the appropriate libraries) - but that's probably overkill unless the header is included in many different places in each library, such that the explicit instantiation could significantly reduce compile-time/object size, etc)

Right right. -thanks for helping me understand. Sorry for the noise.

The goal here is to have Transforms/IPO/

[davidxl]

no it is not noise -- the questions you raised are very good :)

[xur]

Thanks David for explaining this for me. Sorry for not making it clear in
the original email.

[davidxl]

lgtm. Please wait for other reviewer's feedback too.

[wmi]

llvm/lib/Transforms/IPO/SampleProfile.cpp

474–475

The usages of ProfileIsCS are all in SampleProfileLoader member functions, so it doesn't need to be defined in SampleProfileLoaderImpl?

488

Will FSAFDO need ProfAccForSymsInList to know which function is new? If not, we can leave it in SampleLoader class.

846–847

Why not remove this part and just call SampleProfileLoaderBaseImpl::getInstWeight below instead of SampleProfileLoaderBaseImpl::getInstWeightImpl?

llvm/test/Transforms/SampleProfile/inline-coverage.ll

34 ↗

(On Diff #321963)

Why does this CHECK need to be moved lower?

[xur]

Thanks, Wei!

llvm/lib/Transforms/IPO/SampleProfile.cpp

474–475

Yes. ProfileIsCS is not longer used in base class after we break some of the methods.
Thanks for catching this.

488

Is this only used in inline?
I'm not sure if this flag will affect the branch probability so I put into base to be safe, and I have it in FSAFDO.

If this only intends for using in inline, I can move it to base.

846–847

If we move this part down and combine the call to getInstrWeightImpl, the semantic is not the same.
I did not look deep into this, but instead of returning error_code(), we could get 0 (and calls to findCalleeFunctionSamples().

llvm/test/Transforms/SampleProfile/inline-coverage.ll

34 ↗

(On Diff #321963)

I did not dig into this. The calls to remark are in the same order. But diagnostic prints different order. I don't think this matter too much. Isn't it.

[wmi]

llvm/lib/Transforms/IPO/SampleProfile.cpp

488

It is used to set function EntryCount. For old symbol (which show up in the binary where profile is collected) without profile, we set its EntryCount to 0 instead of -1, indicating it is very cold instead of unknown. It is also used in callsiteIsHot function. It won't affect branch probablity, so I think FSAFDO may not need it.

846–847

It won't change the semantic. findCalleeFunctionSamples calls findFunctionSamples inside of it too. If findFunctionSamples returns nullptr for a call inst, findCalleeFunctionSamples will return nullptr as well, and the "return 0" branch won't be taken.

llvm/test/Transforms/SampleProfile/inline-coverage.ll

34 ↗

(On Diff #321963)

It could expose some hidden behavior change somewhere so may be good to find out.

[xur]

Integrated Wei's review comments.
The test case turns out to be my problem. Also fixed it.

[wmi]

LGTM.

llvm/lib/Transforms/IPO/SampleProfile.cpp

 
 using BlockWeightMap = DenseMap<const BasicBlock *, uint64_t>;
 using EquivalenceClassMap = DenseMap<const BasicBlock *, const BasicBlock *>;
 using Edge = std::pair<const BasicBlock *, const BasicBlock *>;
 using EdgeWeightMap = DenseMap<Edge, uint64_t>;
 using BlockEdgeMap =
     DenseMap<const BasicBlock *, SmallVector<const BasicBlock *, 8>>;
 
-class SampleProfileLoader;
-
 class SampleCoverageTracker {
 public:
   bool markSamplesUsed(const FunctionSamples *FS, uint32_t LineOffset,
                        uint32_t Discriminator, uint64_t Samples);
   unsigned computeCoverage(unsigned Used, unsigned Total) const;
   unsigned countUsedRecords(const FunctionSamples *FS,
                             ProfileSummaryInfo *PSI) const;
   unsigned countBodyRecords(const FunctionSamples *FS,
return LHS.CalleeSamples->getGUID(LHS.CalleeSamples->getName()) <
            RHS.CalleeSamples->getGUID(RHS.CalleeSamples->getName());
   }
 };
 
 using CandidateQueue =
     PriorityQueue<InlineCandidate, std::vector<InlineCandidate>,
                   CandidateComparer>;
 
-/// Sample profile pass.
-///
-/// This pass reads profile data from the file specified by
-/// -sample-profile-file and annotates every affected function with the
-/// profile information found in that file.
-class SampleProfileLoader {
+class SampleProfileLoaderBaseImpl {
 public:
-  SampleProfileLoader(
-      StringRef Name, StringRef RemapName, ThinOrFullLTOPhase LTOPhase,
-      std::function<AssumptionCache &(Function &)> GetAssumptionCache,
-      std::function<TargetTransformInfo &(Function &)> GetTargetTransformInfo,
-      std::function<const TargetLibraryInfo &(Function &)> GetTLI)
-      : GetAC(std::move(GetAssumptionCache)),
-        GetTTI(std::move(GetTargetTransformInfo)), GetTLI(std::move(GetTLI)),
-        Filename(std::string(Name)), RemappingFilename(std::string(RemapName)),
-        LTOPhase(LTOPhase) {}
-
-  bool doInitialization(Module &M, FunctionAnalysisManager *FAM = nullptr);
-  bool runOnModule(Module &M, ModuleAnalysisManager *AM,
-                   ProfileSummaryInfo *_PSI, CallGraph *CG);
-
+  SampleProfileLoaderBaseImpl(std::string Name) : Filename(Name) {}
   void dump() { Reader->dump(); }
 
 protected:
   friend class SampleCoverageTracker;
 
-  bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
   unsigned getFunctionLoc(Function &F);
-  bool emitAnnotations(Function &F);
-  ErrorOr<uint64_t> getInstWeight(const Instruction &I);
-  ErrorOr<uint64_t> getProbeWeight(const Instruction &I);
+  virtual ErrorOr<uint64_t> getInstWeight(const Instruction &Inst);
+  ErrorOr<uint64_t> getInstWeightImpl(const Instruction &Inst);
   ErrorOr<uint64_t> getBlockWeight(const BasicBlock *BB);
-  const FunctionSamples *findCalleeFunctionSamples(const CallBase &I) const;
-  std::vector<const FunctionSamples *>
-  findIndirectCallFunctionSamples(const Instruction &I, uint64_t &Sum) const;
-  mutable DenseMap<const DILocation *, const FunctionSamples *> DILocation2SampleMap;
-  const FunctionSamples *findFunctionSamples(const Instruction &I) const;
-  // Attempt to promote indirect call and also inline the promoted call
-  bool tryPromoteAndInlineCandidate(
-      Function &F, InlineCandidate &Candidate, uint64_t SumOrigin,
-      uint64_t &Sum, DenseSet<Instruction *> &PromotedInsns,
-      SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
-  bool inlineHotFunctions(Function &F,
-                          DenseSet<GlobalValue::GUID> &InlinedGUIDs);
-  InlineCost shouldInlineCandidate(InlineCandidate &Candidate);
-  bool getInlineCandidate(InlineCandidate *NewCandidate, CallBase *CB);
-  bool
-  tryInlineCandidate(InlineCandidate &Candidate,
-                     SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
-  bool
-  inlineHotFunctionsWithPriority(Function &F,
-                                 DenseSet<GlobalValue::GUID> &InlinedGUIDs);
-  // Inline cold/small functions in addition to hot ones
-  bool shouldInlineColdCallee(CallBase &CallInst);
-  void emitOptimizationRemarksForInlineCandidates(
-      const SmallVectorImpl<CallBase *> &Candidates, const Function &F,
-      bool Hot);
+  mutable DenseMap<const DILocation *, const FunctionSamples *>
+      DILocation2SampleMap;
+  virtual const FunctionSamples *
+  findFunctionSamples(const Instruction &I) const;
   void printEdgeWeight(raw_ostream &OS, Edge E);
   void printBlockWeight(raw_ostream &OS, const BasicBlock *BB) const;
   void printBlockEquivalence(raw_ostream &OS, const BasicBlock *BB);
   bool computeBlockWeights(Function &F);
   void findEquivalenceClasses(Function &F);
   template <bool IsPostDom>
   void findEquivalencesFor(BasicBlock *BB1, ArrayRef<BasicBlock *> Descendants,
                            DominatorTreeBase<BasicBlock, IsPostDom> *DomTree);
 
   void propagateWeights(Function &F);
   uint64_t visitEdge(Edge E, unsigned *NumUnknownEdges, Edge *UnknownEdge);
   void buildEdges(Function &F);
-  std::vector<Function *> buildFunctionOrder(Module &M, CallGraph *CG);
   bool propagateThroughEdges(Function &F, bool UpdateBlockCount);
-  void computeDominanceAndLoopInfo(Function &F);
   void clearFunctionData();
+  void computeDominanceAndLoopInfo(Function &F);
+  bool
+  computeAndPropagateWeights(Function &F,
+                             const DenseSet<GlobalValue::GUID> &InlinedGUIDs);
+  void emitCoverageRemarks(Function &F);
 
   /// Map basic blocks to their computed weights.
   ///
   /// The weight of a basic block is defined to be the maximum
   /// of all the instruction weights in that block.
   BlockWeightMap BlockWeights;
 
   /// Map edges to their computed weights.
protected:
   /// Equivalence classes for block weights.
   ///
   /// Two blocks BB1 and BB2 are in the same equivalence class if they
   /// dominate and post-dominate each other, and they are in the same loop
   /// nest. When this happens, the two blocks are guaranteed to execute
   /// the same number of times.
   EquivalenceClassMap EquivalenceClass;
 
-  /// Map from function name to Function *. Used to find the function from
-  /// the function name. If the function name contains suffix, additional
-  /// entry is added to map from the stripped name to the function if there
-  /// is one-to-one mapping.
-  StringMap<Function *> SymbolMap;
-
   /// Dominance, post-dominance and loop information.
   std::unique_ptr<DominatorTree> DT;
   std::unique_ptr<PostDominatorTree> PDT;
   std::unique_ptr<LoopInfo> LI;
 
-  std::function<AssumptionCache &(Function &)> GetAC;
-  std::function<TargetTransformInfo &(Function &)> GetTTI;
-  std::function<const TargetLibraryInfo &(Function &)> GetTLI;
-
   /// Predecessors for each basic block in the CFG.
   BlockEdgeMap Predecessors;
 
   /// Successors for each basic block in the CFG.
   BlockEdgeMap Successors;
 
   SampleCoverageTracker CoverageTracker;
 
   /// Profile reader object.
   std::unique_ptr<SampleProfileReader> Reader;
 
-  /// Profile tracker for different context.
-  std::unique_ptr<SampleContextTracker> ContextTracker;
-
   /// Samples collected for the body of this function.
   FunctionSamples *Samples = nullptr;
 
   /// Name of the profile file to load.
   std::string Filename;
 
+  /// Profile Summary Info computed from sample profile.
+  ProfileSummaryInfo *PSI = nullptr;
+
+  /// Optimization Remark Emitter used to emit diagnostic remarks.
+  OptimizationRemarkEmitter *ORE = nullptr;
+};
+
+/// Sample profile pass.
+///
+/// This pass reads profile data from the file specified by
+/// -sample-profile-file and annotates every affected function with the
+/// profile information found in that file.
+class SampleProfileLoader : public SampleProfileLoaderBaseImpl {
+public:
+  SampleProfileLoader(
+      StringRef Name, StringRef RemapName, ThinOrFullLTOPhase LTOPhase,
+      std::function<AssumptionCache &(Function &)> GetAssumptionCache,
+      std::function<TargetTransformInfo &(Function &)> GetTargetTransformInfo,
+      std::function<const TargetLibraryInfo &(Function &)> GetTLI)
+      : SampleProfileLoaderBaseImpl(std::string(Name)),
+        GetAC(std::move(GetAssumptionCache)),
+        GetTTI(std::move(GetTargetTransformInfo)), GetTLI(std::move(GetTLI)),
+        RemappingFilename(std::string(RemapName)), LTOPhase(LTOPhase) {}
+
+  bool doInitialization(Module &M, FunctionAnalysisManager *FAM = nullptr);
+  bool runOnModule(Module &M, ModuleAnalysisManager *AM,
+                   ProfileSummaryInfo *_PSI, CallGraph *CG);
+
+protected:
+  bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
+  bool emitAnnotations(Function &F);
+  ErrorOr<uint64_t> getInstWeight(const Instruction &I) override;
+  ErrorOr<uint64_t> getProbeWeight(const Instruction &I);
+  const FunctionSamples *findCalleeFunctionSamples(const CallBase &I) const;
+  const FunctionSamples *
+  findFunctionSamples(const Instruction &I) const override;
+  std::vector<const FunctionSamples *>
+  findIndirectCallFunctionSamples(const Instruction &I, uint64_t &Sum) const;
+  // Attempt to promote indirect call and also inline the promoted call
+  bool tryPromoteAndInlineCandidate(
+      Function &F, InlineCandidate &Candidate, uint64_t SumOrigin,
+      uint64_t &Sum, DenseSet<Instruction *> &PromotedInsns,
+      SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
+  bool inlineHotFunctions(Function &F,
+                          DenseSet<GlobalValue::GUID> &InlinedGUIDs);
+  InlineCost shouldInlineCandidate(InlineCandidate &Candidate);
+  bool getInlineCandidate(InlineCandidate *NewCandidate, CallBase *CB);
+  bool
+  tryInlineCandidate(InlineCandidate &Candidate,
+                     SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
+  bool
+  inlineHotFunctionsWithPriority(Function &F,
+                                 DenseSet<GlobalValue::GUID> &InlinedGUIDs);
+  // Inline cold/small functions in addition to hot ones
+  bool shouldInlineColdCallee(CallBase &CallInst);
+  void emitOptimizationRemarksForInlineCandidates(
+      const SmallVectorImpl<CallBase *> &Candidates, const Function &F,
+      bool Hot);
+  std::vector<Function *> buildFunctionOrder(Module &M, CallGraph *CG);
+  void generateMDProfMetadata(Function &F);
+
+  /// Map from function name to Function *. Used to find the function from
+  /// the function name. If the function name contains suffix, additional
+  /// entry is added to map from the stripped name to the function if there
+  /// is one-to-one mapping.
+  StringMap<Function *> SymbolMap;
+
+  std::function<AssumptionCache &(Function &)> GetAC;
+  std::function<TargetTransformInfo &(Function &)> GetTTI;
+  std::function<const TargetLibraryInfo &(Function &)> GetTLI;
+
+  /// Profile tracker for different context.
+  std::unique_ptr<SampleContextTracker> ContextTracker;
+
   /// Name of the profile remapping file to load.
   std::string RemappingFilename;
 
   /// Flag indicating whether the profile input loaded successfully.
   bool ProfileIsValid = false;
 
   /// Flag indicating whether input profile is context-sensitive
   bool ProfileIsCS = false;
 
   /// Flag indicating which LTO/ThinLTO phase the pass is invoked in.
   ///
   /// We need to know the LTO phase because for example in ThinLTOPrelink
   /// phase, in annotation, we should not promote indirect calls. Instead,
   /// we will mark GUIDs that needs to be annotated to the function.
   ThinOrFullLTOPhase LTOPhase;
 
-  /// Profile Summary Info computed from sample profile.
-  ProfileSummaryInfo *PSI = nullptr;
-
   /// Profle Symbol list tells whether a function name appears in the binary
   /// used to generate the current profile.
   std::unique_ptr<ProfileSymbolList> PSL;
 
   /// Total number of samples collected in this profile.
   ///
   /// This is the sum of all the samples collected in all the functions executed
   /// at runtime.
   uint64_t TotalCollectedSamples = 0;
 
-  /// Optimization Remark Emitter used to emit diagnostic remarks.
-  OptimizationRemarkEmitter *ORE = nullptr;
-
   // Information recorded when we declined to inline a call site
   // because we have determined it is too cold is accumulated for
   // each callee function. Initially this is just the entry count.
   struct NotInlinedProfileInfo {
     uint64_t entryCount;
   };
   DenseMap<Function *, NotInlinedProfileInfo> notInlinedCallInfo;
 

 unsigned SampleCoverageTracker::computeCoverage(unsigned Used,
                                                 unsigned Total) const {
   assert(Used <= Total &&
          "number of used records cannot exceed the total number of records");
   return Total > 0 ? Used * 100 / Total : 100;
 }
 
 /// Clear all the per-function data used to load samples and propagate weights.
-void SampleProfileLoader::clearFunctionData() {
+void SampleProfileLoaderBaseImpl::clearFunctionData() {
   BlockWeights.clear();
   EdgeWeights.clear();
   VisitedBlocks.clear();
   VisitedEdges.clear();
   EquivalenceClass.clear();
   DT = nullptr;
   PDT = nullptr;
   LI = nullptr;
   Predecessors.clear();
   Successors.clear();
   CoverageTracker.clear();
 }
 
 #ifndef NDEBUG
 /// Print the weight of edge \p E on stream \p OS.
 ///
 /// \param OS  Stream to emit the output to.
 /// \param E  Edge to print.
-void SampleProfileLoader::printEdgeWeight(raw_ostream &OS, Edge E) {
+void SampleProfileLoaderBaseImpl::printEdgeWeight(raw_ostream &OS, Edge E) {
   OS << "weight[" << E.first->getName() << "->" << E.second->getName()
      << "]: " << EdgeWeights[E] << "\n";
 }
 
 /// Print the equivalence class of block \p BB on stream \p OS.
 ///
 /// \param OS  Stream to emit the output to.
 /// \param BB  Block to print.
-void SampleProfileLoader::printBlockEquivalence(raw_ostream &OS,
+void SampleProfileLoaderBaseImpl::printBlockEquivalence(raw_ostream &OS,
                                                         const BasicBlock *BB) {
   const BasicBlock *Equiv = EquivalenceClass[BB];
   OS << "equivalence[" << BB->getName()
      << "]: " << ((Equiv) ? EquivalenceClass[BB]->getName() : "NONE") << "\n";
 }
 
 /// Print the weight of block \p BB on stream \p OS.
 ///
 /// \param OS  Stream to emit the output to.
 /// \param BB  Block to print.
-void SampleProfileLoader::printBlockWeight(raw_ostream &OS,
+void SampleProfileLoaderBaseImpl::printBlockWeight(raw_ostream &OS,
                                                    const BasicBlock *BB) const {
   const auto &I = BlockWeights.find(BB);
   uint64_t W = (I == BlockWeights.end() ? 0 : I->second);
   OS << "weight[" << BB->getName() << "]: " << W << "\n";
 }
 #endif
 
 /// Get the weight for an instruction.
 ///
 /// The "weight" of an instruction \p Inst is the number of samples
 /// collected on that instruction at runtime. To retrieve it, we
 /// need to compute the line number of \p Inst relative to the start of its
 /// function. We use HeaderLineno to compute the offset. We then
 /// look up the samples collected for \p Inst using BodySamples.
 ///
 /// \param Inst Instruction to query.
 ///
 /// \returns the weight of \p Inst.
+ErrorOr<uint64_t>
+SampleProfileLoaderBaseImpl::getInstWeight(const Instruction &Inst) {
+  return getInstWeightImpl(Inst);
+}
+
 ErrorOr<uint64_t> SampleProfileLoader::getInstWeight(const Instruction &Inst) {
   if (FunctionSamples::ProfileIsProbeBased)
     return getProbeWeight(Inst);
 
   const DebugLoc &DLoc = Inst.getDebugLoc();
   if (!DLoc)
     return std::error_code();
 
-  const FunctionSamples *FS = findFunctionSamples(Inst);
-  if (!FS)
-    return std::error_code();
-
   // Ignore all intrinsics, phinodes and branch instructions.
-  // Branch and phinodes instruction usually contains debug info from sources outside of
-  // the residing basic block, thus we ignore them during annotation.
+  // Branch and phinodes instruction usually contains debug info from sources
+  // outside of the residing basic block, thus we ignore them during annotation.
   if (isa<BranchInst>(Inst) || isa<IntrinsicInst>(Inst) || isa<PHINode>(Inst))
     return std::error_code();
 
   // If a direct call/invoke instruction is inlined in profile
   // (findCalleeFunctionSamples returns non-empty result), but not inlined here,
   // it means that the inlined callsite has no sample, thus the call
   // instruction should have 0 count.
   if (!ProfileIsCS)
     if (const auto *CB = dyn_cast<CallBase>(&Inst))
       if (!CB->isIndirectCall() && findCalleeFunctionSamples(*CB))
         return 0;
 
+  return getInstWeightImpl(Inst);
+}
+
+ErrorOr<uint64_t>
+SampleProfileLoaderBaseImpl::getInstWeightImpl(const Instruction &Inst) {
+  const FunctionSamples *FS = findFunctionSamples(Inst);
+  if (!FS)
+    return std::error_code();
+
+  const DebugLoc &DLoc = Inst.getDebugLoc();
+  if (!DLoc)
+    return std::error_code();
+
   const DILocation *DIL = DLoc;
   uint32_t LineOffset = FunctionSamples::getOffset(DIL);
   uint32_t Discriminator = DIL->getBaseDiscriminator();
   ErrorOr<uint64_t> R = FS->findSamplesAt(LineOffset, Discriminator);
   if (R) {
     bool FirstMark =
         CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator, R.get());
     if (FirstMark) {

 /// Compute the weight of a basic block.
 ///
 /// The weight of basic block \p BB is the maximum weight of all the
 /// instructions in BB.
 ///
 /// \param BB The basic block to query.
 ///
 /// \returns the weight for \p BB.
-ErrorOr<uint64_t> SampleProfileLoader::getBlockWeight(const BasicBlock *BB) {
+ErrorOr<uint64_t>
+SampleProfileLoaderBaseImpl::getBlockWeight(const BasicBlock *BB) {
   uint64_t Max = 0;
   bool HasWeight = false;
   for (auto &I : BB->getInstList()) {
     const ErrorOr<uint64_t> &R = getInstWeight(I);
     if (R) {
       Max = std::max(Max, R.get());
       HasWeight = true;
     }
   }
   return HasWeight ? ErrorOr<uint64_t>(Max) : std::error_code();
 }
 
 /// Compute and store the weights of every basic block.
 ///
 /// This populates the BlockWeights map by computing
 /// the weights of every basic block in the CFG.
 ///
 /// \param F The function to query.
-bool SampleProfileLoader::computeBlockWeights(Function &F) {
+bool SampleProfileLoaderBaseImpl::computeBlockWeights(Function &F) {
   bool Changed = false;
   LLVM_DEBUG(dbgs() << "Block weights\n");
   for (const auto &BB : F) {
     ErrorOr<uint64_t> Weight = getBlockWeight(&BB);
     if (Weight) {
       BlockWeights[&BB] = Weight.get();
       VisitedBlocks.insert(&BB);
       Changed = true;

 ///
 /// The FunctionSamples of an instruction \p Inst is the inlined instance
 /// in which that instruction is coming from. We traverse the inline stack
 /// of that instruction, and match it with the tree nodes in the profile.
 ///
 /// \param Inst Instruction to query.
 ///
 /// \returns the FunctionSamples pointer to the inlined instance.
+const FunctionSamples *SampleProfileLoaderBaseImpl::findFunctionSamples(
+    const Instruction &Inst) const {
+  const DILocation *DIL = Inst.getDebugLoc();
+  if (!DIL)
+    return Samples;
+
+  auto it = DILocation2SampleMap.try_emplace(DIL, nullptr);
+  if (it.second) {
+    it.first->second = Samples->findFunctionSamples(DIL, Reader->getRemapper());
+  }
+  return it.first->second;
+}
+
 const FunctionSamples *
 SampleProfileLoader::findFunctionSamples(const Instruction &Inst) const {
   if (FunctionSamples::ProfileIsProbeBased) {
     Optional<PseudoProbe> Probe = extractProbe(Inst);
     if (!Probe)
       return nullptr;
   }
 

 /// equivalence class to \p BB1.
 ///
 /// \param BB1  Block to check.
 /// \param Descendants  Descendants of \p BB1 in either the dom or pdom tree.
 /// \param DomTree  Opposite dominator tree. If \p Descendants is filled
 ///                 with blocks from \p BB1's dominator tree, then
 ///                 this is the post-dominator tree, and vice versa.
 template <bool IsPostDom>
-void SampleProfileLoader::findEquivalencesFor(
+void SampleProfileLoaderBaseImpl::findEquivalencesFor(
     BasicBlock *BB1, ArrayRef<BasicBlock *> Descendants,
     DominatorTreeBase<BasicBlock, IsPostDom> *DomTree) {
   const BasicBlock *EC = EquivalenceClass[BB1];
   uint64_t Weight = BlockWeights[EC];
   for (const auto *BB2 : Descendants) {
     bool IsDomParent = DomTree->dominates(BB2, BB1);
     bool IsInSameLoop = LI->getLoopFor(BB1) == LI->getLoopFor(BB2);
     if (BB1 != BB2 && IsDomParent && IsInSameLoop) {

 ///
 /// Since samples may be missing from blocks, we can fill in the gaps by setting
 /// the weights of all the blocks in the same equivalence class to the same
 /// weight. To compute the concept of equivalence, we use dominance and loop
 /// information. Two blocks B1 and B2 are in the same equivalence class if B1
 /// dominates B2, B2 post-dominates B1 and both are in the same loop.
 ///
 /// \param F The function to query.
-void SampleProfileLoader::findEquivalenceClasses(Function &F) {
+void SampleProfileLoaderBaseImpl::findEquivalenceClasses(Function &F) {
   SmallVector<BasicBlock *, 8> DominatedBBs;
   LLVM_DEBUG(dbgs() << "\nBlock equivalence classes\n");
   // Find equivalence sets based on dominance and post-dominance information.
   for (auto &BB : F) {
     BasicBlock *BB1 = &BB;
 
     // Compute BB1's equivalence class once.
     if (EquivalenceClass.count(BB1)) {

 /// If \p E has not been visited before, we copy to \p UnknownEdge
 /// and increment the count of unknown edges.
 ///
 /// \param E  Edge to visit.
 /// \param NumUnknownEdges  Current number of unknown edges.
 /// \param UnknownEdge  Set if E has not been visited before.
 ///
 /// \returns E's weight, if known. Otherwise, return 0.
-uint64_t SampleProfileLoader::visitEdge(Edge E, unsigned *NumUnknownEdges,
+uint64_t SampleProfileLoaderBaseImpl::visitEdge(Edge E,
+                                                unsigned *NumUnknownEdges,
                                                 Edge *UnknownEdge) {
   if (!VisitedEdges.count(E)) {
     (*NumUnknownEdges)++;
     *UnknownEdge = E;
     return 0;
   }
 
   return EdgeWeights[E];
 }
 
 /// Propagate weights through incoming/outgoing edges.
 ///
 /// If the weight of a basic block is known, and there is only one edge
 /// with an unknown weight, we can calculate the weight of that edge.
 ///
 /// Similarly, if all the edges have a known count, we can calculate the
 /// count of the basic block, if needed.
 ///
 /// \param F  Function to process.
 /// \param UpdateBlockCount  Whether we should update basic block counts that
 ///                          has already been annotated.
 ///
 /// \returns  True if new weights were assigned to edges or blocks.
-bool SampleProfileLoader::propagateThroughEdges(Function &F,
+bool SampleProfileLoaderBaseImpl::propagateThroughEdges(Function &F,
                                                         bool UpdateBlockCount) {
   bool Changed = false;
   LLVM_DEBUG(dbgs() << "\nPropagation through edges\n");
   for (const auto &BI : F) {
     const BasicBlock *BB = &BI;
     const BasicBlock *EC = EquivalenceClass[BB];
 
     // Visit all the predecessor and successor edges to determine
     // which ones have a weight assigned already. Note that it doesn't
bool SampleProfileLoaderBaseImpl::propagateThroughEdges(Function &F,
 
   return Changed;
 }
 
 /// Build in/out edge lists for each basic block in the CFG.
 ///
 /// We are interested in unique edges. If a block B1 has multiple
 /// edges to another block B2, we only add a single B1->B2 edge.
-void SampleProfileLoader::buildEdges(Function &F) {
+void SampleProfileLoaderBaseImpl::buildEdges(Function &F) {
   for (auto &BI : F) {
     BasicBlock *B1 = &BI;
 
     // Add predecessors for B1.
     SmallPtrSet<BasicBlock *, 16> Visited;
     if (!Predecessors[B1].empty())
       llvm_unreachable("Found a stale predecessors list in a basic block.");
     for (BasicBlock *B2 : predecessors(B1))

 ///   edge will be the weight of the block minus the sum of all the known
 ///   edges. If the sum of all the known edges is larger than BB's weight,
 ///   we set the unknown edge weight to zero.
 ///
 /// - If there is a self-referential edge, and the weight of the block is
 ///   known, the weight for that edge is set to the weight of the block
 ///   minus the weight of the other incoming edges to that block (if
 ///   known).
-void SampleProfileLoader::propagateWeights(Function &F) {
+void SampleProfileLoaderBaseImpl::propagateWeights(Function &F) {
   bool Changed = true;
   unsigned I = 0;
 
   // If BB weight is larger than its corresponding loop's header BB weight,
   // use the BB weight to replace the loop header BB weight.
   for (auto &BI : F) {
     BasicBlock *BB = &BI;
     Loop *L = LI->getLoopFor(BB);
void SampleProfileLoaderBaseImpl::propagateWeights(Function &F) {
   }
 
   // The 3rd propagation pass allows adjust annotated BB weights that are
   // obviously wrong.
   Changed = true;
   while (Changed && I++ < SampleProfileMaxPropagateIterations) {
     Changed = propagateThroughEdges(F, true);
   }
+}
 
+/// Generate branch weight metadata for all branches in \p F.
+///
+/// Branch weights are computed out of instruction samples using a
+/// propagation heuristic. Propagation proceeds in 3 phases:
+///
+/// 1- Assignment of block weights. All the basic blocks in the function
+///    are initial assigned the same weight as their most frequently
+///    executed instruction.
+///
+/// 2- Creation of equivalence classes. Since samples may be missing from
+///    blocks, we can fill in the gaps by setting the weights of all the
+///    blocks in the same equivalence class to the same weight. To compute
+///    the concept of equivalence, we use dominance and loop information.
+///    Two blocks B1 and B2 are in the same equivalence class if B1
+///    dominates B2, B2 post-dominates B1 and both are in the same loop.
+///
+/// 3- Propagation of block weights into edges. This uses a simple
+///    propagation heuristic. The following rules are applied to every
+///    block BB in the CFG:
+///
+///    - If BB has a single predecessor/successor, then the weight
+///      of that edge is the weight of the block.
+///
+///    - If all the edges are known except one, and the weight of the
+///      block is already known, the weight of the unknown edge will
+///      be the weight of the block minus the sum of all the known
+///      edges. If the sum of all the known edges is larger than BB's weight,
+///      we set the unknown edge weight to zero.
+///
+///    - If there is a self-referential edge, and the weight of the block is
+///      known, the weight for that edge is set to the weight of the block
+///      minus the weight of the other incoming edges to that block (if
+///      known).
+///
+/// Since this propagation is not guaranteed to finalize for every CFG, we
+/// only allow it to proceed for a limited number of iterations (controlled
+/// by -sample-profile-max-propagate-iterations).
+///
+/// FIXME: Try to replace this propagation heuristic with a scheme
+/// that is guaranteed to finalize. A work-list approach similar to
+/// the standard value propagation algorithm used by SSA-CCP might
+/// work here.
+///
+/// \param F The function to query.
+///
+/// \returns true if \p F was modified. Returns false, otherwise.
+bool SampleProfileLoaderBaseImpl::computeAndPropagateWeights(
+    Function &F, const DenseSet<GlobalValue::GUID> &InlinedGUIDs) {
+  bool Changed = (InlinedGUIDs.size() != 0);
+
+  // Compute basic block weights.
+  Changed |= computeBlockWeights(F);
+
+  if (Changed) {
+    // Add an entry count to the function using the samples gathered at the
+    // function entry.
+    // Sets the GUIDs that are inlined in the profiled binary. This is used
+    // for ThinLink to make correct liveness analysis, and also make the IR
+    // match the profiled binary before annotation.
+    F.setEntryCount(
+        ProfileCount(Samples->getHeadSamples() + 1, Function::PCT_Real),
+        &InlinedGUIDs);
+
+    // Compute dominance and loop info needed for propagation.
+    computeDominanceAndLoopInfo(F);
+
+    // Find equivalence classes.
+    findEquivalenceClasses(F);
+
+    // Propagate weights to all edges.
+    propagateWeights(F);
+  }
+
+  return Changed;
+}
+
+void SampleProfileLoaderBaseImpl::emitCoverageRemarks(Function &F) {
+  // If coverage checking was requested, compute it now.
+  if (SampleProfileRecordCoverage) {
+    unsigned Used = CoverageTracker.countUsedRecords(Samples, PSI);
+    unsigned Total = CoverageTracker.countBodyRecords(Samples, PSI);
+    unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
+    if (Coverage < SampleProfileRecordCoverage) {
+      F.getContext().diagnose(DiagnosticInfoSampleProfile(
+          F.getSubprogram()->getFilename(), getFunctionLoc(F),
+          Twine(Used) + " of " + Twine(Total) + " available profile records (" +
+              Twine(Coverage) + "%) were applied",
+          DS_Warning));
+    }
+  }
+
+  if (SampleProfileSampleCoverage) {
+    uint64_t Used = CoverageTracker.getTotalUsedSamples();
+    uint64_t Total = CoverageTracker.countBodySamples(Samples, PSI);
+    unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
+    if (Coverage < SampleProfileSampleCoverage) {
+      F.getContext().diagnose(DiagnosticInfoSampleProfile(
+          F.getSubprogram()->getFilename(), getFunctionLoc(F),
+          Twine(Used) + " of " + Twine(Total) + " available profile samples (" +
+              Twine(Coverage) + "%) were applied",
+          DS_Warning));
+    }
+  }
+}
+
+// Generate MD_prof metadata for every branch instruction using the
+// edge weights computed during propagation.
+void SampleProfileLoader::generateMDProfMetadata(Function &F) {
   // Generate MD_prof metadata for every branch instruction using the
   // edge weights computed during propagation.
   LLVM_DEBUG(dbgs() << "\nPropagation complete. Setting branch weights\n");
   LLVMContext &Ctx = F.getContext();
   MDBuilder MDB(Ctx);
   for (auto &BI : F) {
     BasicBlock *BB = &BI;
 

 /// retrieves the line number where the function is defined. This is
 /// line 0 for all the samples read from the profile file. Every line
 /// number is relative to this line.
 ///
 /// \param F  Function object to query.
 ///
 /// \returns the line number where \p F is defined. If it returns 0,
 ///          it means that there is no debug information available for \p F.
-unsigned SampleProfileLoader::getFunctionLoc(Function &F) {
+unsigned SampleProfileLoaderBaseImpl::getFunctionLoc(Function &F) {
   if (DISubprogram *S = F.getSubprogram())
     return S->getLine();
 
   if (NoWarnSampleUnused)
     return 0;
 
   // If the start of \p F is missing, emit a diagnostic to inform the user
   // about the missed opportunity.
   F.getContext().diagnose(DiagnosticInfoSampleProfile(
       "No debug information found in function " + F.getName() +
           ": Function profile not used",
       DS_Warning));
   return 0;
 }
 
-void SampleProfileLoader::computeDominanceAndLoopInfo(Function &F) {
+void SampleProfileLoaderBaseImpl::computeDominanceAndLoopInfo(Function &F) {
   DT.reset(new DominatorTree);
   DT->recalculate(F);
 
   PDT.reset(new PostDominatorTree(F));
 
   LI.reset(new LoopInfo);
   LI->analyze(*DT);
 }
 
-/// Generate branch weight metadata for all branches in \p F.
-///
-/// Branch weights are computed out of instruction samples using a
-/// propagation heuristic. Propagation proceeds in 3 phases:
-///
-/// 1- Assignment of block weights. All the basic blocks in the function
-///    are initial assigned the same weight as their most frequently
-///    executed instruction.
-///
-/// 2- Creation of equivalence classes. Since samples may be missing from
-///    blocks, we can fill in the gaps by setting the weights of all the
-///    blocks in the same equivalence class to the same weight. To compute
-///    the concept of equivalence, we use dominance and loop information.
-///    Two blocks B1 and B2 are in the same equivalence class if B1
-///    dominates B2, B2 post-dominates B1 and both are in the same loop.
-///
-/// 3- Propagation of block weights into edges. This uses a simple
-///    propagation heuristic. The following rules are applied to every
-///    block BB in the CFG:
-///
-///    - If BB has a single predecessor/successor, then the weight
-///      of that edge is the weight of the block.
-///
-///    - If all the edges are known except one, and the weight of the
-///      block is already known, the weight of the unknown edge will
-///      be the weight of the block minus the sum of all the known
-///      edges. If the sum of all the known edges is larger than BB's weight,
-///      we set the unknown edge weight to zero.
-///
-///    - If there is a self-referential edge, and the weight of the block is
-///      known, the weight for that edge is set to the weight of the block
-///      minus the weight of the other incoming edges to that block (if
-///      known).
-///
-/// Since this propagation is not guaranteed to finalize for every CFG, we
-/// only allow it to proceed for a limited number of iterations (controlled
-/// by -sample-profile-max-propagate-iterations).
-///
-/// FIXME: Try to replace this propagation heuristic with a scheme
-/// that is guaranteed to finalize. A work-list approach similar to
-/// the standard value propagation algorithm used by SSA-CCP might
-/// work here.
-///
 /// Once all the branch weights are computed, we emit the MD_prof
 /// metadata on BB using the computed values for each of its branches.
 ///
 /// \param F The function to query.
 ///
 /// \returns true if \p F was modified. Returns false, otherwise.
 bool SampleProfileLoader::emitAnnotations(Function &F) {
   bool Changed = false;
bool SampleProfileLoader::emitAnnotations(Function &F) {
   }
 
   DenseSet<GlobalValue::GUID> InlinedGUIDs;
   if (ProfileIsCS && CallsitePrioritizedInline)
     Changed |= inlineHotFunctionsWithPriority(F, InlinedGUIDs);
   else
     Changed |= inlineHotFunctions(F, InlinedGUIDs);
 
-  // Compute basic block weights.
-  Changed |= computeBlockWeights(F);
-
-  if (Changed) {
-    // Add an entry count to the function using the samples gathered at the
-    // function entry.
-    // Sets the GUIDs that are inlined in the profiled binary. This is used
-    // for ThinLink to make correct liveness analysis, and also make the IR
-    // match the profiled binary before annotation.
-    F.setEntryCount(
-        ProfileCount(Samples->getHeadSamples() + 1, Function::PCT_Real),
-        &InlinedGUIDs);
-
-    // Compute dominance and loop info needed for propagation.
-    computeDominanceAndLoopInfo(F);
-
-    // Find equivalence classes.
-    findEquivalenceClasses(F);
+  Changed |= computeAndPropagateWeights(F, InlinedGUIDs);
 
-    // Propagate weights to all edges.
-    propagateWeights(F);
-  }
-
-  // If coverage checking was requested, compute it now.
-  if (SampleProfileRecordCoverage) {
-    unsigned Used = CoverageTracker.countUsedRecords(Samples, PSI);
-    unsigned Total = CoverageTracker.countBodyRecords(Samples, PSI);
-    unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
-    if (Coverage < SampleProfileRecordCoverage) {
-      F.getContext().diagnose(DiagnosticInfoSampleProfile(
-          F.getSubprogram()->getFilename(), getFunctionLoc(F),
-          Twine(Used) + " of " + Twine(Total) + " available profile records (" +
-              Twine(Coverage) + "%) were applied",
-          DS_Warning));
-    }
-  }
+  if (Changed)
+    generateMDProfMetadata(F);
 
-  if (SampleProfileSampleCoverage) {
-    uint64_t Used = CoverageTracker.getTotalUsedSamples();
-    uint64_t Total = CoverageTracker.countBodySamples(Samples, PSI);
-    unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
-    if (Coverage < SampleProfileSampleCoverage) {
-      F.getContext().diagnose(DiagnosticInfoSampleProfile(
-          F.getSubprogram()->getFilename(), getFunctionLoc(F),
-          Twine(Used) + " of " + Twine(Total) + " available profile samples (" +
-              Twine(Coverage) + "%) were applied",
-          DS_Warning));
-    }
-  }
+  emitCoverageRemarks(F);
   return Changed;
 }
 
 char SampleProfileLoaderLegacyPass::ID = 0;
 
 INITIALIZE_PASS_BEGIN(SampleProfileLoaderLegacyPass, "sample-profile",
                       "Sample Profile loader", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)