[SampleFDO] Enhance profile remapping support for inline instance

[wmi]

Summary

Profile remapping is a feature to match a function in the module with its profile in sample profile if the function name and the name in profile look different but are equivalent using given remapping rules. This is a useful feature to keep the performance stable by specifying some remapping rules when sampleFDO targets are going through some large scale function signature change.

However, currently profile remapping support is only valid for outline function profile in SampleFDO. It cannot match a callee with an inline instance profile if they have different but equivalent names. We found that without the support for inline instance profile, remapping is less effective for some large scale change.

To add that support, the patch introduces a temporary remapper in addition to the current remapper used for outline function profile. The temporary remapper will be populated with remapping rules at the same time as the regular remapper. At a callsite when compiler tries to find an inline instance profile to match with the callee, it will be populated with all the inline instance profiles at the same location as the callsite first, then check whether the callee name exists in the remapper. After the matching is done, the remapper should be cleared except that the remapping rules should be kept, that is why it is called temporary remapper. The clearing is necessary to prevent the callee name from being mistakenly matched with an inline instance profile at a different location or with an outline profile.

In a planned large scale change of int64 type (long long) to int64_t (long), we found the performance of a google internal benchmark degraded by 2% if nothing was done. That was very significant for us. If current profile remapping was enabled, the performance degradation dropped to 1.2%, and it was still significant. If the profile remapping with the current patch was enabled, the performance degradation further dropped to 0.14%, and that was small enough to be acceptable.

[wmi]

Remove an unrelated file being accidently included.

[hoy]

Nice to see the perf regression drops to around 1% with this change!

llvm/include/llvm/ProfileData/SampleProf.h

434

Nit: Update comment with Remapper?

llvm/lib/ProfileData/SampleProf.cpp

223

Does this remove only the last callee name added to the the remapper? Maybe give a more explicit name for that?

[wmi]

Hongtao, thanks for the review!

llvm/include/llvm/ProfileData/SampleProf.h

434

Updated.

llvm/lib/ProfileData/SampleProf.cpp

223

Yes, it only removes the last callee name. I cannot find a good name so I just change the remapper to CalleeRemapper.

[wmi]

Address Hongtao's comment.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

218–222

I guess I didn't fully understand the clearing part - if a function (inlinee) is renamed, why do we not want to apply the remapping globally? E.g. for A->B, and C->B inlined profile, if B is renamed to D, we would want to match D to B for both A->D profile and C->D path, right? Then a global remapper including all names should suffice.

Could you share a concrete example as to why each function needs its own temp remapper for all its inlinees?

[wmi]

llvm/lib/ProfileData/SampleProf.cpp

218–222

Right, given two names B and D, ideally we want the remapper to tell us whether they are identical by returning true or false for remapper.equivalent(B, D). However, remapper doesn't work like this. To check whether B and D are identical, remapper needs to insert B into a foldingset first by parsing it, then check whether D has already existed in it by parsing D and trying to insert D (but not truely insert it).

Similarly, if we want to check whether an equivalent name exists in a nameset, remapper need to insert alll the names in the nameset before it can answer the query. So to answer the query whether the callee's name matches an inline instance profile at a specific callsite, it needs to clear names in the remapper inserted at other callsites before it inserts the callee's name at the current callsite into remapper.

Existing remapper doesn't have to be cleared throughout the sample profile loading pass because it insert all the names of outline instance in the sample profile, and the question it needs to answer in the whole pass is whether a function name has a match in the outline instances. So because of the way how remapper works, we need different remappers to answer different types of remapping queries.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

218–222

Maybe I'm still missing something, but the part that I don't understand is why would we need to have boundary for nameset and the queries? For mangle names, they all live in a single conceptual "namespace", regardless of whether it's name for inlinee or outlined function.

For inlinee's name, it's possible that we've inserted their names in global remapper when initializing with all outlined functions' names. If not, we could also insert it into global remapper right before checking for alternative names? To me the only difference for inlinee's name is that, we might discover those names a bit later as we may not see them when initializing the global remapper with outlined function names, but that doesn't seem to warrant creating boundaries for nameset (between outline vs inlinee, inlinees for one functions vs inlinees for another).

[wmi]

llvm/lib/ProfileData/SampleProf.cpp

218–222

why would we need to have boundary for nameset and the queries?

That is because the query we want to have is whether a callee matches any inline instance at a specific callsite location, not the inline stances at any other callsites. That is why we want boundary for nameset related with the current callsite, and need to clear the names related with other callsites.

For inlinee's name, it's possible that we've inserted their names in global remapper

Yes, many inline instances have already exist as outline instance in global remapper. If we use global remapper, we can know a callee match an inline/outline instance somewhere in the profile, but don't know whether the callee match an inline instance at the current callsite.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

218–222

Ok, thanks for clarification. Looking closer at the code, I think I understand the reason now. You're iterating over the old names to see if anyone of them matches variant of new name (callee name at the call site), then you have to limit the temp remapper to contain only the new name of interest (callee name).

For outlined functions profiles, the remapper provides new_name -> old_profile mapping (SampleProfileReaderItaniumRemapper::getSamplesFor(StringRef Fname)). Dealing with name->profile mapping is tricky for inlinee's profiles. But would it be possible to tweak the remapper to support new_name->old_name lookup? With that, inlinee profile can be handled universally by retrieving old name for new callee name, then iterate the call site profiles to get inlinee's profile.

And to be consistent with SampleProfileReaderItaniumRemapper::getSamplesFor for outline profile, handling of call site profile can also be abstracted away into the remapper?

Specifically, I'm thinking about handling inlinees in SampleProfileReaderItaniumRemapper::applyRemapping too and keeping a Key to old_name map, instead of Key to samples map.

[wmi]

llvm/lib/ProfileData/SampleProf.cpp

218–222

That is a very good suggestion! Following the suggestion I get the same remapping result with simpler code, and I find I can also add support for indirect call with the simpler implementation and am working on it.

[wmi]

Address Wenlei's comment and add the support to search indirect call promotion candidate using remapper.

[wenlei]

Great - thanks for making the changes. Looks good with two minor comments.

llvm/lib/Transforms/IPO/SampleProfile.cpp

1067

Do we actually need this extra check? Renaming won't change wether a call is self-recursive, so the check on line 1059 should be good enough?

1932

hoist Remapper = Reader->getRemapper() out of the loop?

[wmi]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1067

Line 1059 only checks whether an inline instance at the callsite has the identical name with the parent function F. It is possible that the inline instance has a different but equivalent name as F. In that case, SymbolMap may map the name of the inline instance back to the definition of F and use F as the indirect call promotion target.

I can add one more check "R->getValue()->getName() != F.getName()" to the if (R != SymbolMap.end() ...) below and simplify the code a little bit.

1932

Good point.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1067

I see. Or R->getValue() != &F would work too?

[wmi]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1067

Yes, that is better.

[wmi]

Address Wenlei's comment.

[wenlei]

LGTM, thanks!

[wmi]

Thanks Wenlei for the review and the insightful suggestions!

[hoyFB]

Looks even better now!

llvm/include/llvm/ProfileData/SampleProf.h

 private:
   uint64_t NumSamples = 0;
   CallTargetMap CallTargets;
 };
 
 raw_ostream &operator<<(raw_ostream &OS, const SampleRecord &Sample);
 
 class FunctionSamples;
+class SampleProfileReaderItaniumRemapper;
 
 using BodySampleMap = std::map<LineLocation, SampleRecord>;
 // NOTE: Using a StringMap here makes parsed profiles consume around 17% more
 // memory, which is *very* significant for large profiles.
 using FunctionSamplesMap = std::map<std::string, FunctionSamples, std::less<>>;
 using CallsiteSampleMap = std::map<LineLocation, FunctionSamplesMap>;
 
 /// Representation of the samples collected for a function.
public:
   const FunctionSamplesMap *
   findFunctionSamplesMapAt(const LineLocation &Loc) const {
     auto iter = CallsiteSamples.find(Loc);
     if (iter == CallsiteSamples.end())
       return nullptr;
     return &iter->second;
   }
 
-  /// Returns a pointer to FunctionSamples at the given callsite location \p Loc
-  /// with callee \p CalleeName. If no callsite can be found, relax the
-  /// restriction to return the FunctionSamples at callsite location \p Loc
-  /// with the maximum total sample count.
-  const FunctionSamples *findFunctionSamplesAt(const LineLocation &Loc,
-                                               StringRef CalleeName) const {
-    std::string CalleeGUID;
-    CalleeName = getRepInFormat(CalleeName, UseMD5, CalleeGUID);
-
-    auto iter = CallsiteSamples.find(Loc);
-    if (iter == CallsiteSamples.end())
-      return nullptr;
-    auto FS = iter->second.find(CalleeName);
-    if (FS != iter->second.end())
-      return &FS->second;
-    // If we cannot find exact match of the callee name, return the FS with
-    // the max total count. Only do this when CalleeName is not provided,
-    // i.e., only for indirect calls.
-    if (!CalleeName.empty())
-      return nullptr;
-    uint64_t MaxTotalSamples = 0;
-    const FunctionSamples *R = nullptr;
-    for (const auto &NameFS : iter->second)
-      if (NameFS.second.getTotalSamples() >= MaxTotalSamples) {
-        MaxTotalSamples = NameFS.second.getTotalSamples();
-        R = &NameFS.second;
-      }
-    return R;
-  }
+  /// Returns a pointer to FunctionSamples at the given callsite location
+  /// \p Loc with callee \p CalleeName. If no callsite can be found, relax
+  /// the restriction to return the FunctionSamples at callsite location
+  /// \p Loc with the maximum total sample count. If \p Remapper is not
+  /// nullptr, use \p Remapper to find FunctionSamples with equivalent name
+  /// as \p CalleeName.
+  const FunctionSamples *
+  findFunctionSamplesAt(const LineLocation &Loc, StringRef CalleeName,
+                        SampleProfileReaderItaniumRemapper *Remapper) const;
 
   bool empty() const { return TotalSamples == 0; }
 
   /// Return the total number of samples collected inside the function.
   uint64_t getTotalSamples() const { return TotalSamples; }
 
   /// Return the total number of branch samples that have the function as the
   /// branch target. This should be equivalent to the sample of the first
public:
   /// from.
   ///
   /// The FunctionSamples of the instruction (Machine or IR) associated to
   /// \p DIL 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.
   ///
   /// \returns the FunctionSamples pointer to the inlined instance.
-  const FunctionSamples *findFunctionSamples(const DILocation *DIL) const;
+  /// If \p Remapper is not nullptr, it will be used to find matching
+  /// FunctionSamples with not exactly the same but equivalent name.
+  const FunctionSamples *findFunctionSamples(
+      const DILocation *DIL,
+      SampleProfileReaderItaniumRemapper *Remapper = nullptr) const;
 
   static SampleProfileFormat Format;
 
   /// Whether the profile uses MD5 to represent string.
   static bool UseMD5;
 
   /// GUIDToFuncNameMap saves the mapping from GUID to the symbol name, for
   /// all the function symbols defined or declared in current module.
   DenseMap<uint64_t, StringRef> *GUIDToFuncNameMap = nullptr;
 
   // Assume the input \p Name is a name coming from FunctionSamples itself.
   // If UseMD5 is true, the name is already a GUID and we
   // don't want to return the GUID of GUID.
   static uint64_t getGUID(StringRef Name) {
     return UseMD5 ? std::stoull(Name.data()) : Function::getGUID(Name);
   }
 
+  // Find all the names in the current FunctionSamples including names in
+  // all the inline instances and names of call targets.
+  void findAllNames(DenseSet<StringRef> &NameSet) const;
+
 private:
   /// Mangled name of the function.
   StringRef Name;
 
   /// Total number of samples collected inside this function.
   ///
   /// Samples are cumulative, they include all the samples collected
   /// inside this function and all its inlined callees.

llvm/include/llvm/ProfileData/SampleProfReader.h

 //          in the text format documentation above).
 //        FUNCTION BODY
 //          A FUNCTION BODY entry describing the inlined function.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_PROFILEDATA_SAMPLEPROFREADER_H
 #define LLVM_PROFILEDATA_SAMPLEPROFREADER_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/ProfileSummary.h"
public:
   void insert(StringRef FunctionName) { Remappings->insert(FunctionName); }
 
   /// Query whether there is equivalent in the remapper which has been
   /// inserted.
   bool exist(StringRef FunctionName) {
     return Remappings->lookup(FunctionName);
   }
 
-  /// Return the samples collected for function \p F if remapper knows
-  /// it is present in SampleMap.
-  FunctionSamples *getSamplesFor(StringRef FunctionName);
+  /// Return the equivalent name in the profile for \p FunctionName if
+  /// it exists.
+  Optional<StringRef> lookUpNameInProfile(StringRef FunctionName);
 
 private:
   // The buffer holding the content read from remapping file.
   std::unique_ptr<MemoryBuffer> Buffer;
   std::unique_ptr<SymbolRemappingReader> Remappings;
-  DenseMap<SymbolRemappingReader::Key, FunctionSamples *> SampleMap;
+  // Map remapping key to the name in the profile. By looking up the
+  // key in the remapper, a given new name can be mapped to the
+  // cannonical name using the NameMap.
+  DenseMap<SymbolRemappingReader::Key, StringRef> NameMap;
   // The Reader the remapper is servicing.
   SampleProfileReader &Reader;
   // Indicate whether remapping has been applied to the profile read
   // by Reader -- by calling applyRemapping.
   bool RemappingApplied = false;
 };
 
 /// Sample-based profile reader.
FunctionSamples *getOrCreateSamplesFor(const Function &F) {
     std::string FGUID;
     StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
     CanonName = getRepInFormat(CanonName, useMD5(), FGUID);
     return &Profiles[CanonName];
   }
 
   /// Return the samples collected for function \p F.
   virtual FunctionSamples *getSamplesFor(StringRef Fname) {
-    if (Remapper) {
-      if (auto FS = Remapper->getSamplesFor(Fname))
-        return FS;
-    }
     std::string FGUID;
     Fname = getRepInFormat(Fname, useMD5(), FGUID);
     auto It = Profiles.find(Fname);
     if (It != Profiles.end())
       return &It->second;
+
+    if (Remapper) {
+      if (auto NameInProfile = Remapper->lookUpNameInProfile(Fname)) {
+        auto It = Profiles.find(*NameInProfile);
+        if (It != Profiles.end())
+          return &It->second;
+      }
+    }
     return nullptr;
   }
 
   /// Return all the profiles.
   StringMap<FunctionSamples> &getProfiles() { return Profiles; }
 
   /// Report a parse error message.
   void reportError(int64_t LineNumber, Twine Msg) const {
public:
   /// It includes all the names that have samples either in outline instance
   /// or inline instance.
   virtual std::vector<StringRef> *getNameTable() { return nullptr; }
   virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) { return false; };
 
   /// Return whether names in the profile are all MD5 numbers.
   virtual bool useMD5() { return false; }
 
+  SampleProfileReaderItaniumRemapper *getRemapper() { return Remapper.get(); }
+
 protected:
   /// Map every function to its associated profile.
   ///
   /// The profile of every function executed at runtime is collected
   /// in the structure FunctionSamples. This maps function objects
   /// to their corresponding profiles.
   StringMap<FunctionSamples> Profiles;
 

llvm/lib/ProfileData/SampleProf.cpp

 //=-- SampleProf.cpp - Sample profiling format support --------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains common definitions used in the reading and writing of
 // sample profile data.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ProfileData/SampleProf.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DebugInfoMetadata.h"
+#include "llvm/ProfileData/SampleProfReader.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
raw_ostream &llvm::sampleprof::operator<<(raw_ostream &OS,
   return OS;
 }
 
 unsigned FunctionSamples::getOffset(const DILocation *DIL) {
   return (DIL->getLine() - DIL->getScope()->getSubprogram()->getLine()) &
       0xffff;
 }
 
-const FunctionSamples *
-FunctionSamples::findFunctionSamples(const DILocation *DIL) const {
+const FunctionSamples *FunctionSamples::findFunctionSamples(
+    const DILocation *DIL, SampleProfileReaderItaniumRemapper *Remapper) const {
   assert(DIL);
   SmallVector<std::pair<LineLocation, StringRef>, 10> S;
 
   const DILocation *PrevDIL = DIL;
   for (DIL = DIL->getInlinedAt(); DIL; DIL = DIL->getInlinedAt()) {
     S.push_back(std::make_pair(
         LineLocation(getOffset(DIL), DIL->getBaseDiscriminator()),
         PrevDIL->getScope()->getSubprogram()->getLinkageName()));
     PrevDIL = DIL;
   }
   if (S.size() == 0)
     return this;
   const FunctionSamples *FS = this;
   for (int i = S.size() - 1; i >= 0 && FS != nullptr; i--) {
-    FS = FS->findFunctionSamplesAt(S[i].first, S[i].second);
+    FS = FS->findFunctionSamplesAt(S[i].first, S[i].second, Remapper);
   }
   return FS;
 }
 
+void FunctionSamples::findAllNames(DenseSet<StringRef> &NameSet) const {
+  NameSet.insert(Name);
+  for (const auto &BS : BodySamples)
+    for (const auto &TS : BS.second.getCallTargets())
+      NameSet.insert(TS.getKey());
+
+  for (const auto &CS : CallsiteSamples) {
+    for (const auto &NameFS : CS.second) {
+      NameSet.insert(NameFS.first);
+      NameFS.second.findAllNames(NameSet);
+    }
+  }
+}
+
+const FunctionSamples *FunctionSamples::findFunctionSamplesAt(
+    const LineLocation &Loc, StringRef CalleeName,
+    SampleProfileReaderItaniumRemapper *Remapper) const {
+  std::string CalleeGUID;
+  CalleeName = getRepInFormat(CalleeName, UseMD5, CalleeGUID);
+
+  auto iter = CallsiteSamples.find(Loc);
+  if (iter == CallsiteSamples.end())
+    return nullptr;
+  auto FS = iter->second.find(CalleeName);
+  if (FS != iter->second.end())
+    return &FS->second;
+  if (Remapper) {
+    if (auto NameInProfile = Remapper->lookUpNameInProfile(CalleeName)) {
+      auto FS = iter->second.find(*NameInProfile);
+      if (FS != iter->second.end())
+        return &FS->second;
+    }
+  }
+  // If we cannot find exact match of the callee name, return the FS with
+  // the max total count. Only do this when CalleeName is not provided,
+  // i.e., only for indirect calls.
+  if (!CalleeName.empty())
+    return nullptr;
+  uint64_t MaxTotalSamples = 0;
+  const FunctionSamples *R = nullptr;
+  for (const auto &NameFS : iter->second)
+    if (NameFS.second.getTotalSamples() >= MaxTotalSamples) {
+      MaxTotalSamples = NameFS.second.getTotalSamples();
+      R = &NameFS.second;
+    }
+  return R;
+}
+
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void FunctionSamples::dump() const { print(dbgs(), 0); }
 #endif
 
 std::error_code ProfileSymbolList::read(const uint8_t *Data,
                                         uint64_t ListSize) {
   const char *ListStart = reinterpret_cast<const char *>(Data);
   uint64_t Size = 0;

llvm/lib/ProfileData/SampleProfReader.cpp

Ctx.diagnose(DiagnosticInfoSampleProfile(
         Reader.getBuffer()->getBufferIdentifier(),
         "Profile data remapping cannot be applied to profile data "
         "in compact format (original mangled names are not available).",
         DS_Warning));
     return;
   }
 
   assert(Remappings && "should be initialized while creating remapper");
-  for (auto &Sample : Reader.getProfiles())
-    if (auto Key = Remappings->insert(Sample.first()))
-      SampleMap.insert({Key, &Sample.second});
+  for (auto &Sample : Reader.getProfiles()) {
+    DenseSet<StringRef> NamesInSample;
+    Sample.second.findAllNames(NamesInSample);
+    for (auto &Name : NamesInSample)
+      if (auto Key = Remappings->insert(Name))
+        NameMap.insert({Key, Name});
+  }
 
   RemappingApplied = true;
 }
 
-FunctionSamples *
-SampleProfileReaderItaniumRemapper::getSamplesFor(StringRef Fname) {
+Optional<StringRef>
+SampleProfileReaderItaniumRemapper::lookUpNameInProfile(StringRef Fname) {
   if (auto Key = Remappings->lookup(Fname))
-    return SampleMap.lookup(Key);
-  return nullptr;
+    return NameMap.lookup(Key);
+  return None;
 }
 
 /// Prepare a memory buffer for the contents of \p Filename.
 ///
 /// \returns an error code indicating the status of the buffer.
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 setupMemoryBuffer(const Twine &Filename) {
   auto BufferOrErr = MemoryBuffer::getFileOrSTDIN(Filename);

llvm/lib/Transforms/IPO/SampleProfile.cpp

if (Function *Callee = Inst.getCalledFunction())
     CalleeName = Callee->getName();
 
   const FunctionSamples *FS = findFunctionSamples(Inst);
   if (FS == nullptr)
     return nullptr;
 
   return FS->findFunctionSamplesAt(LineLocation(FunctionSamples::getOffset(DIL),
                                                 DIL->getBaseDiscriminator()),
-                                   CalleeName);
+                                   CalleeName, Reader->getRemapper());
 }
 
 /// Returns a vector of FunctionSamples that are the indirect call targets
 /// of \p Inst. The vector is sorted by the total number of samples. Stores
 /// the total call count of the indirect call in \p Sum.
 std::vector<const FunctionSamples *>
 SampleProfileLoader::findIndirectCallFunctionSamples(
     const Instruction &Inst, uint64_t &Sum) const {

 const FunctionSamples *
 SampleProfileLoader::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);
+    it.first->second = Samples->findFunctionSamples(DIL, Reader->getRemapper());
   return it.first->second;
 }
 
 bool SampleProfileLoader::inlineCallInstruction(CallBase &CB) {
   if (ExternalInlineAdvisor) {
     auto Advice = ExternalInlineAdvisor->getAdvice(CB);
     if (!Advice->isInliningRecommended()) {
       Advice->recordUnattemptedInlining();
for (CallBase *I : CIS) {
           continue;
         uint64_t Sum;
         for (const auto *FS : findIndirectCallFunctionSamples(*I, Sum)) {
           if (IsThinLTOPreLink) {
             FS->findInlinedFunctions(InlinedGUIDs, F.getParent(),
                                      PSI->getOrCompHotCountThreshold());
             continue;
           }
-          auto CalleeFunctionName = FS->getFuncName();
+          if (!callsiteIsHot(FS, PSI))
+            continue;
+
+          const char *Reason = "Callee function not available";
+          // R->getValue() != &F is to prevent promoting a recursive call.
           // If it is a recursive call, we do not inline it as it could bloat
           // the code exponentially. There is way to better handle this, e.g.
           // clone the caller first, and inline the cloned caller if it is
           // recursive. As llvm does not inline recursive calls, we will
           // simply ignore it instead of handling it explicitly.
-          if (CalleeFunctionName == F.getName())
-            continue;
-
-          if (!callsiteIsHot(FS, PSI))
-            continue;
-
-          const char *Reason = "Callee function not available";
+          auto CalleeFunctionName = FS->getFuncName();
           auto R = SymbolMap.find(CalleeFunctionName);
           if (R != SymbolMap.end() && R->getValue() &&
               !R->getValue()->isDeclaration() &&
               R->getValue()->getSubprogram() &&
               R->getValue()->hasFnAttribute("use-sample-profile") &&
+              R->getValue() != &F &&
               isLegalToPromote(*I, R->getValue(), &Reason)) {
             uint64_t C = FS->getEntrySamples();
             auto &DI =
                 pgo::promoteIndirectCall(*I, R->getValue(), C, Sum, false, ORE);
             Sum -= C;
             PromotedInsns.insert(I);
             // If profile mismatches, we should not attempt to inline DI.
             if ((isa<CallInst>(DI) || isa<InvokeInst>(DI)) &&
SampleProfileLoader::buildFunctionOrder(Module &M, CallGraph *CG) {
   std::reverse(FunctionOrderList.begin(), FunctionOrderList.end());
   return FunctionOrderList;
 }
 
 bool SampleProfileLoader::doInitialization(Module &M,
                                            FunctionAnalysisManager *FAM) {
   auto &Ctx = M.getContext();
 
-  std::unique_ptr<SampleProfileReaderItaniumRemapper> RemapReader;
   auto ReaderOrErr =
       SampleProfileReader::create(Filename, Ctx, RemappingFilename);
   if (std::error_code EC = ReaderOrErr.getError()) {
     std::string Msg = "Could not open profile: " + EC.message();
     Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg));
     return false;
   }
   Reader = std::move(ReaderOrErr.get());
if (M.getProfileSummary(/* IsCS */ false) == nullptr) {
     M.setProfileSummary(Reader->getSummary().getMD(M.getContext()),
                         ProfileSummary::PSK_Sample);
     PSI->refresh();
   }
   // Compute the total number of samples collected in this profile.
   for (const auto &I : Reader->getProfiles())
     TotalCollectedSamples += I.second.getTotalSamples();
 
+  auto Remapper = Reader->getRemapper();
   // Populate the symbol map.
   for (const auto &N_F : M.getValueSymbolTable()) {
     StringRef OrigName = N_F.getKey();
     Function *F = dyn_cast<Function>(N_F.getValue());
     if (F == nullptr)
       continue;
     SymbolMap[OrigName] = F;
     auto pos = OrigName.find('.');
     if (pos != StringRef::npos) {
       StringRef NewName = OrigName.substr(0, pos);
       auto r = SymbolMap.insert(std::make_pair(NewName, F));
       // Failiing to insert means there is already an entry in SymbolMap,
       // thus there are multiple functions that are mapped to the same
       // stripped name. In this case of name conflicting, set the value
       // to nullptr to avoid confusion.
       if (!r.second)
         r.first->second = nullptr;
+      OrigName = NewName;
+    }
+    // Insert the remapped names into SymbolMap.
+    if (Remapper) {
+      if (auto MapName = Remapper->lookUpNameInProfile(OrigName)) {
+        if (*MapName == OrigName)
+          continue;
+        SymbolMap.insert(std::make_pair(*MapName, F));
+      }
     }
   }
 
   bool retval = false;
   for (auto F : buildFunctionOrder(M, CG)) {
     assert(!F->isDeclaration());
     clearFunctionData();
     retval |= runOnFunction(*F, AM);

llvm/test/Transforms/SampleProfile/Inputs/remap-2.prof

+test:15680:2500
+ 1: 100
+ 4: 100
+ 5: 3000 xoo:1000
+ 5: _ZN3foo3barERKN1N1XINS_4quuxEEE:2000
+  1: 2000
+ 6: _ZN1N1XE:2500
+  1: 2500
+
+_ZN1M1X1YE:15680:2500
+ 1: 100
+ 4: 100
+ 5: 3000 xoo:1000
+ 5: _ZN1M1X1YE:2000
+  1: 2000
+

llvm/test/Transforms/SampleProfile/remap-2.ll

+; RUN: opt %s -passes=sample-profile -sample-profile-file=%S/Inputs/remap-2.prof -sample-profile-remapping-file=%S/Inputs/remap.map -S | FileCheck %s
+; Check profile remapping works for searching inline instance, searching
+; indirect call promotion candidate and prevent recursive inline.
+
[e-mail] = common global i32 zeroinitializer, align 16
[e-mail] = common global i32 zeroinitializer, align 16
+
+define i32 @_ZN3foo3barERKN1M1XINS_6detail3quxEEE() #0 !dbg !9 {
+entry:
+  %t0 = load i32, i32* @x.addr, align 4
+  %t1 = load i32, i32* @y.addr, align 4
+  %add = add nsw i32 %t0, %t1
+  ret i32 %add
+}
+
+define i32 @_ZN1M1XE() #0 !dbg !10 {
+entry:
+  %t0 = load i32, i32* @x.addr, align 4
+  %t1 = load i32, i32* @y.addr, align 4
+  %sub = sub nsw i32 %t0, %t1
+  ret i32 %sub
+}
+
+define void @test(i32 ()*) #0 !dbg !4 {
+  %t2 = alloca i32 ()*
+  store i32 ()* %0, i32 ()** %t2
+  %t3 = load i32 ()*, i32 ()** %t2
+; Check call i32 %t3 has been indirect call promoted and call i32 @_ZN1M1XE
+; has been inlined.
+; CHECK-LABEL: @test(
+; CHECK: icmp eq i32 ()* %t3, @_ZN3foo3barERKN1M1XINS_6detail3quxEEE
+; CHECK-NOT: call i32 @_ZN1M1XE
+  %t4 = call i32 %t3(), !dbg !7
+  %t5 = call i32 @_ZN1M1XE(), !dbg !8
+  ret void
+}
+
+define void @_ZN1M1X1YE(i32 ()*) #0 !dbg !11 {
+  %t2 = alloca i32 ()*
+  store i32 ()* %0, i32 ()** %t2
+  %t3 = load i32 ()*, i32 ()** %t2
+; Check call i32 %t3 has got its profile but is not indirect call promoted
+; because the promotion candidate is a recursive call to the current function.
+; CHECK-LABEL: @_ZN1M1X1YE(
+; CHECK: call i32 %t3(), {{.*}} !prof ![[PROFID:[0-9]+]]
+; CHECK-NOT: icmp eq i32 ()* %t3, @_ZN1M1X1YE
+  %t4 = call i32 %t3(), !dbg !12
+  ret void
+}
+
+; CHECK: ![[PROFID]] = !{!"VP", i32 0, i64 3000
+
+attributes #0 = { "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!13, !14}
+!llvm.ident = !{!15}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.5 ", isOptimized: false, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !2, globals: !2, imports: !2)
+!1 = !DIFile(filename: "calls.cc", directory: ".")
+!2 = !{}
+!4 = distinct !DISubprogram(name: "test", line: 3, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!5 = !DIFile(filename: "calls.cc", directory: ".")
+!6 = !DISubroutineType(types: !2)
+!7 = !DILocation(line: 8, scope: !4)
+!8 = !DILocation(line: 9, scope: !4)
+!9 = distinct !DISubprogram(name: "_ZN3foo3barERKN1M1XINS_6detail3quxEEE", line: 15, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!10 = distinct !DISubprogram(name: "_ZN1M1XE", line: 20, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!11 = distinct !DISubprogram(name: "_ZN1M1X1YE", line: 25, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0, scopeLine: 3, file: !1, scope: !5, type: !6, retainedNodes: !2)
+!12 = !DILocation(line: 30, scope: !11)
+!13 = !{i32 2, !"Dwarf Version", i32 4}
+!14 = !{i32 1, !"Debug Info Version", i32 3}
+!15 = !{!"clang version 3.5 "}
+

llvm/unittests/ProfileData/SampleProfTest.cpp

void verifyProfileSummary(ProfileSummary &Summary, Module &M,
                             const bool AddPartialField,
                             const bool AddPartialProfileRatioField) {
     LLVMContext &Context = M.getContext();
     const bool IsPartialProfile = Summary.isPartialProfile();
     const double PartialProfileRatio = Summary.getPartialProfileRatio();
     auto VerifySummary = [IsPartialProfile, PartialProfileRatio](
                              ProfileSummary &Summary) mutable {
       ASSERT_EQ(ProfileSummary::PSK_Sample, Summary.getKind());
-      ASSERT_EQ(137392u, Summary.getTotalCount());
-      ASSERT_EQ(8u, Summary.getNumCounts());
+      ASSERT_EQ(138211u, Summary.getTotalCount());
+      ASSERT_EQ(10u, Summary.getNumCounts());
       ASSERT_EQ(4u, Summary.getNumFunctions());
       ASSERT_EQ(1437u, Summary.getMaxFunctionCount());
       ASSERT_EQ(60351u, Summary.getMaxCount());
       ASSERT_EQ(IsPartialProfile, Summary.isPartialProfile());
       ASSERT_EQ(PartialProfileRatio, Summary.getPartialProfileRatio());
 
       uint32_t Cutoff = 800000;
       auto Predicate = [&Cutoff](const ProfileSummaryEntry &PE) {
         return PE.Cutoff == Cutoff;
       };
       std::vector<ProfileSummaryEntry> &Details = Summary.getDetailedSummary();
       auto EightyPerc = find_if(Details, Predicate);
       Cutoff = 900000;
       auto NinetyPerc = find_if(Details, Predicate);
       Cutoff = 950000;
       auto NinetyFivePerc = find_if(Details, Predicate);
       Cutoff = 990000;
       auto NinetyNinePerc = find_if(Details, Predicate);
       ASSERT_EQ(60000u, EightyPerc->MinCount);
       ASSERT_EQ(12557u, NinetyPerc->MinCount);
       ASSERT_EQ(12557u, NinetyFivePerc->MinCount);
-      ASSERT_EQ(610u, NinetyNinePerc->MinCount);
+      ASSERT_EQ(600u, NinetyNinePerc->MinCount);
     };
     VerifySummary(Summary);
 
     // Test that conversion of summary to and from Metadata works.
     Metadata *MD =
         Summary.getMD(Context, AddPartialField, AddPartialProfileRatioField);
     ASSERT_TRUE(MD);
     ProfileSummary *PS = ProfileSummary::getFromMD(MD);
void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
     FooSamples.addTotalSamples(7711);
     FooSamples.addHeadSamples(610);
     FooSamples.addBodySamples(1, 0, 610);
     FooSamples.addBodySamples(2, 0, 600);
     FooSamples.addBodySamples(4, 0, 60000);
     FooSamples.addBodySamples(8, 0, 60351);
     FooSamples.addBodySamples(10, 0, 605);
 
+    // Add inline instance with name "_Z3gooi".
+    StringRef GooName("_Z3gooi");
+    auto &GooSamples =
+        FooSamples.functionSamplesAt(LineLocation(7, 0))[GooName.str()];
+    GooSamples.setName(GooName);
+    GooSamples.addTotalSamples(502);
+    GooSamples.addBodySamples(3, 0, 502);
+
+    // Add inline instance with name "_Z3hooi".
+    StringRef HooName("_Z3hooi");
+    auto &HooSamples =
+        GooSamples.functionSamplesAt(LineLocation(9, 0))[HooName.str()];
+    HooSamples.setName(HooName);
+    HooSamples.addTotalSamples(317);
+    HooSamples.addBodySamples(4, 0, 317);
+
     StringRef BarName("_Z3bari");
     FunctionSamples BarSamples;
     BarSamples.setName(BarName);
     BarSamples.addTotalSamples(20301);
     BarSamples.addHeadSamples(1437);
     BarSamples.addBodySamples(1, 0, 1437);
     // Test how reader/writer handles unmangled names.
     StringRef MconstructName("_M_construct<char *>");
FunctionType *fn_type =
         FunctionType::get(Type::getVoidTy(Context), {}, false);
 
     SmallVector<char, 128> RemapPath;
     StringRef RemapFile;
     if (Remap) {
       createRemapFile(RemapPath, RemapFile);
       FooName = "_Z4fauxi";
       BarName = "_Z3barl";
+      GooName = "_Z3gool";
+      HooName = "_Z3hool";
     }
 
     M.getOrInsertFunction(FooName, fn_type);
     M.getOrInsertFunction(BarName, fn_type);
     M.getOrInsertFunction(BooName, fn_type);
 
     ProfileSymbolList List;
     if (Format == SampleProfileFormat::SPF_Ext_Binary) {
void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
     FunctionSamples *ReadFooSamples = Reader->getSamplesFor(FooName);
     ASSERT_TRUE(ReadFooSamples != nullptr);
     if (!UseMD5) {
       ASSERT_EQ("_Z3fooi", ReadFooSamples->getName());
     }
     ASSERT_EQ(7711u, ReadFooSamples->getTotalSamples());
     ASSERT_EQ(610u, ReadFooSamples->getHeadSamples());
 
+    // Try to find a FunctionSamples with GooName at given callsites containing
+    // inline instance for GooName. Test the correct FunctionSamples can be
+    // found with Remapper support.
+    const FunctionSamples *ReadGooSamples =
+        ReadFooSamples->findFunctionSamplesAt(LineLocation(7, 0), GooName,
+                                              Reader->getRemapper());
+    ASSERT_TRUE(ReadGooSamples != nullptr);
+    ASSERT_EQ(502u, ReadGooSamples->getTotalSamples());
+
+    // Try to find a FunctionSamples with GooName at given callsites containing
+    // no inline instance for GooName. Test no FunctionSamples will be
+    // found with Remapper support.
+    const FunctionSamples *ReadGooSamplesAgain =
+        ReadFooSamples->findFunctionSamplesAt(LineLocation(9, 0), GooName,
+                                              Reader->getRemapper());
+    ASSERT_TRUE(ReadGooSamplesAgain == nullptr);
+
+    // The inline instance of Hoo is inside of the inline instance of Goo.
+    // Try to find a FunctionSamples with HooName at given callsites containing
+    // inline instance for HooName. Test the correct FunctionSamples can be
+    // found with Remapper support.
+    const FunctionSamples *ReadHooSamples =
+        ReadGooSamples->findFunctionSamplesAt(LineLocation(9, 0), HooName,
+                                              Reader->getRemapper());
+    ASSERT_TRUE(ReadHooSamples != nullptr);
+    ASSERT_EQ(317u, ReadHooSamples->getTotalSamples());
+
     FunctionSamples *ReadBarSamples = Reader->getSamplesFor(BarName);
     ASSERT_TRUE(ReadBarSamples != nullptr);
     if (!UseMD5) {
       ASSERT_EQ("_Z3bari", ReadBarSamples->getName());
     }
     ASSERT_EQ(20301u, ReadBarSamples->getTotalSamples());
     ASSERT_EQ(1437u, ReadBarSamples->getHeadSamples());
     ErrorOr<SampleRecord::CallTargetMap> CTMap =