[SampleFDO] Support enabling -funique-internal-linkage-name

[wmi]

Summary

Now we have -funique-internal-linkage-name support, we want to flip it on by default since it is beneficial to have separate sample profiles for different internal symbols with the same name. As a preparation, we want to avoid regression caused by the flip.

There are two kinds of possible regressions. One kind of regression is when we flip -funique-internal-linkage-name on, the profile is collected from binary built without -funique-internal-linkage-name so it has no uniq suffix, but the IR in the optimized build contains the suffix. This may introduce transient regression. In rare case, user may also want to disable -funique-internal-linkage-name after it has been enabled for a while, to triage something for example. In that case, profile will contain the suffix but IR in the optimized build won't.

To avoid such mismatch, we introduce a NameTable section flag indicating whether there is any name in the profile containing uniq suffix. Compiler will decide whether to keep uniq suffix during name canonicalization depending on the NameTable section flag. The flag is only available for extbinary format. For other formats, by default compiler will keep uniq suffix so they will only experience transient regression when -funique-internal-linkage-name is just flipped.

Another type of regression is caused by places where we miss to call getCanonicalFnName. Those places are fixed.

[wmi]

I tested the patch. For our internal search benchmark, there was no regression when we enabled the flag in optimized build but still used a profile without uniq suffix. If we enabled the flag and also used a profile containing uniq suffix, we saw 0.5% improvement.

[wenlei]

In D96932#2570512, @wmi wrote:

I tested the patch. For our internal search benchmark, there was no regression when we enabled the flag in optimized build but still used a profile without uniq suffix. If we enabled the flag and also used a profile containing uniq suffix, we saw 0.5% improvement.

Thanks for the improvements. We also noticed the discrepancy in name canonicalization between SymbolMap insertion and getCanonicalFnName, and was going to fix it. We're enabling unique name for CSSPGO too because CFG checksum is much less tolerant and mismatch checksum throttles performance. We saw 10% perf delta on spec2017/perlbench w/ and w/o unique name for CSSPGO. But wasn't expecting something like 0.5% for baseline AutoFDO on larger workloads.. Thanks for the data point..

Another type of regression is caused by places where we miss to call getCanonicalFnName. Those places are fixed.

For context tracker, we have canonicalization done in llvm-profgen as part of profile generation which is why we didn't canonicalize on the compiler side.. Now with the new model that let compiler decide whether to strip suffix based on section flag, we should skip canonicalization llvm-profgen, does that sound right? I guess your profile generation tool doesn't do name canonicalization?

[wmi]

Another type of regression is caused by places where we miss to call getCanonicalFnName. Those places are fixed.

For context tracker, we have canonicalization done in llvm-profgen as part of profile generation which is why we didn't canonicalize on the compiler side.. Now with the new model that let compiler decide whether to strip suffix based on section flag, we should skip canonicalization llvm-profgen, does that sound right? I guess your profile generation tool doesn't do name canonicalization?

Another usage of canonicalization is to merge profile. For example, suppose we have a function clone from const propagation, and we want to merge the profile from the clone with the profile from its original body. It is easier to handle it through llvm-profgen and make the profile size smaller before feed it into compiler. Our profile generation tool does name canonicalization.

[tmsriram]

Thanks for doing this! Much appreciated!

llvm/include/llvm/ProfileData/SampleProf.h

787–798

As an aside, where does ".part." come from?

808

Do we need a test where "llvm", "part" and "__uniq" appear in the symbol name?  Also, isn't it "__part"?

llvm/include/llvm/ProfileData/SampleProfReader.h

458

typo: "__uniq"

llvm/lib/ProfileData/SampleProf.cpp

44

Why is this true by default?

276

Could you also add a code comment on how this default could be wrong if at all and its implications?

llvm/lib/ProfileData/SampleProfWriter.cpp

212

Another suggestion to store __uniq as a constant somewhere and reuse that .

llvm/lib/Transforms/IPO/SampleContextTracker.cpp

214

Saw this code snippet for the second time, also used in SampleProf.cpp. Could we refactor to one function?

[hoy]

Thanks for working on this. Good to know unique linkage names helps AutoFDO with a 0.5% gain from Google benchmarks.

In D96932#2572160, @wmi wrote:

Another type of regression is caused by places where we miss to call getCanonicalFnName. Those places are fixed.

For context tracker, we have canonicalization done in llvm-profgen as part of profile generation which is why we didn't canonicalize on the compiler side.. Now with the new model that let compiler decide whether to strip suffix based on section flag, we should skip canonicalization llvm-profgen, does that sound right? I guess your profile generation tool doesn't do name canonicalization?

Another usage of canonicalization is to merge profile. For example, suppose we have a function clone from const propagation, and we want to merge the profile from the clone with the profile from its original body. It is easier to handle it through llvm-profgen and make the profile size smaller before feed it into compiler. Our profile generation tool does name canonicalization.

Good point about the cloned functions. I'm wondering if the canonicalization should be uniformly done in the compiler in every place does name mapping, i.e, from Function to Profile or backwards. The profile generator may not have a full view of the IR and it may not strip away the suffixes accurately without seeing the sample-profile-suffix-elision-policy attribute. cc @wlei

llvm/include/llvm/ProfileData/SampleProf.h

787–798

Can you please add a comment about the order of these suffixes? Looks like it's in reversed appending order, i.e., "._uniq." is always the first suffix to the original name.

[wmi]

For context tracker, we have canonicalization done in llvm-profgen as part of profile generation which is why we didn't canonicalize on the compiler side.. Now with the new model that let compiler decide whether to strip suffix based on section flag, we should skip canonicalization llvm-profgen, does that sound right? I guess your profile generation tool doesn't do name canonicalization?

Another usage of canonicalization is to merge profile. For example, suppose we have a function clone from const propagation, and we want to merge the profile from the clone with the profile from its original body. It is easier to handle it through llvm-profgen and make the profile size smaller before feed it into compiler. Our profile generation tool does name canonicalization.

Good point about the cloned functions. I'm wondering if the canonicalization should be uniformly done in the compiler in every place does name mapping, i.e, from Function to Profile or backwards. The profile generator may not have a full view of the IR and it may not strip away the suffixes accurately without seeing the sample-profile-suffix-elision-policy attribute. cc @wlei

Right, that is valid concern. I remember the function attribute was added because Go lang was trying to add some SampleFDO support and they wanted to use a different elision policy than C++, but they havn't really used it, so we currently get away with it by using a same elision strategy for all symbols when we process a profile. I agree it is better to do all the canonicalization work in compiler. Profile merging is less of a concern.

llvm/include/llvm/ProfileData/SampleProf.h

787–798

It is there for a long time. I guess it is copied from gcc because gcc has such suffix like '.isra.' and '.part.' . I cannot find the suffix in llvm code base. I just notice that bb section add '.part.' suffix and I havn't understand in which case we add the '.part.' suffix. If it needs some change as well, we can address it in a separate patch.

808

I updated the test to include the case with name like "symbol.__uniq.123.llvm.456"

llvm/include/llvm/ProfileData/SampleProfReader.h

458

Thanks for the catch. Fixed.

llvm/lib/ProfileData/SampleProf.cpp

44

Because for extbinary profile format, we can have the flag in the binary showing the profile has name with ".__uniq." suffix, so nothing to be worry about.

However, for other types of format, we don't have the flag to showing that. We can examine each name when we read the profile to see whether there is the suffix, but that will increase compile time. The solution is, for other format, we make the default behavior to assume there is ".uniq." suffix (i.e. make FunctionSamples::HasUniqSuffix default true). It will have profile mismatch when the profile doesn't contain ".uniq." suffix while the IR in the optimized build contains ".__uniq." suffix. That will be a transient regression for the format other than extbinary. I think that is the tradeoff to minimize impact for profile other than extbinary format when we enable -funique-internal-linkage-name by default.

276

I added a comment for it.

llvm/lib/ProfileData/SampleProfWriter.cpp

212

Good point. Done.

llvm/lib/Transforms/IPO/SampleContextTracker.cpp

214

Good suggestion. Done.

[wmi]

Address Sri and Hongtao's comments.

[hoy]

llvm/include/llvm/ProfileData/SampleProf.h

631

Nit: canonicalization?

632

How about making this a static field of FunctionSamples?

734

Is SymbolMap always available to this function? Looks like it could be null for other functions like findFunctionSamplesAt.

llvm/lib/Transforms/IPO/SampleProfile.cpp

1792

Nit: "" -> StringRef() ?

[wmi]

llvm/include/llvm/ProfileData/SampleProf.h

631

Thanks, fixed.

632

That will look clearer, but I am not sure whether it will introduce data race problem. Look at the old problem Wenlei fixed: https://reviews.llvm.org/rG4b99b58a847c8424d9992a1e92a9f8ae7e4d7b51. The existing static members are of simple types and are all initialized with the same value for all the modules so they may be fine. SymbolMap read/write could be prone to such problem.

734

Yes, the function is used to collect function GUID with profile but not present in current module for ThinLTO importing. That function is only used in SampleLoaderPass so SymbolMap is always available. findFunctionSamplesAt has been used more widely than just SampleLoaderPass so SymbolMap is not always available.

llvm/lib/Transforms/IPO/SampleProfile.cpp

1792

Fixed.

[wmi]

Address Hongtao's comment.

[hoy]

llvm/include/llvm/ProfileData/SampleProf.h

632

I actually meant to make the SymbolMap pointer as a static field. Does that sound safe?

[wmi]

llvm/include/llvm/ProfileData/SampleProf.h

632

Each module will have its own SymbolMap so the thread for one module may see other module's SymbolMap pointer.

I think the other static vars in FunctionSamples can get away with the problem only because they are all initialized to the same value for different modules using the same profile.

[hoy]

llvm/include/llvm/ProfileData/SampleProf.h

632

Oh yeah, SymbolMap is per module. Threading will be a problem.

734

As far as I understand, SymbolMap is always available in compiler but not tools. Does that mean tools should never do canonicalization? Sorry, forgot to ask this earlier.

[wmi]

llvm/include/llvm/ProfileData/SampleProf.h

734

Right, like we discussed here: https://reviews.llvm.org/D96932#2580530, I agree it is better to let compiler do all the canonicalization work.

[hoy]

Are the test failures related?

llvm/include/llvm/ProfileData/SampleProf.h

734

Gotcha. We have a couple places in llvm-profgen that call getCanonicalFnName which may not work well with unique linkage names. Do you think they should be removed?

[wmi]

In D96932#2583927, @hoy wrote:

Are the test failures related?

Thanks for reminding me. I don't have the test failure locally. The error says "Unrecognized sample profile encoding format". I guess it is because those profiles are binaries and the diff cannot be applied on them properly.

llvm/include/llvm/ProfileData/SampleProf.h

734

Yes, they should be removed. Do you think whether I should wait to commit it after llvm-profgen is changed accordingly? Is it possible to cause llvm-profgen problem or it may be fine as long as -funique-internal-linkage-names hasn't been enabled by default?

[hoy]

BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.

llvm/include/llvm/ProfileData/SampleProf.h

734

It's fine to have this in first. We already identified the unique linkage name problem with llvm-profgen and started removing the getCanonicalFnName calls. cc @wlei

[wmi]

In D96932#2584061, @hoy wrote:

BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.

Right. Do context string need to be canonicalized? If it is needed, the current canonicalization needs some enhancement to support that.

[hoy]

In D96932#2585513, @wmi wrote:

In D96932#2584061, @hoy wrote:

BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.

Right. Do context string need to be canonicalized? If it is needed, the current canonicalization needs some enhancement to support that.

A couple places in samplecontexttracker will need call getCanonicalFnName. What enhancement is needed in addition to that?

[wmi]

In D96932#2585530, @hoy wrote:

In D96932#2585513, @wmi wrote:

In D96932#2584061, @hoy wrote:

BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.

Right. Do context string need to be canonicalized? If it is needed, the current canonicalization needs some enhancement to support that.

A couple places in samplecontexttracker will need call getCanonicalFnName. What enhancement is needed in addition to that?

For "foo1.llvm.xxx:3 @ foo2.llvm.xxx:2 @ foo3.__uniq.xxx", do we need getCanonicalFnName to canonicalize the whole context string? Or it is divided in samplecontexttracker and getCanonicalFnName will be called for each level?

[hoy]

In D96932#2585567, @wmi wrote:

In D96932#2585530, @hoy wrote:

In D96932#2585513, @wmi wrote:

In D96932#2584061, @hoy wrote:

BTW, in case of duplicated functions due to multi-versioning, the profile generator may not be able to merge them as expected without properly canonicalizing function names, the compiler might be able to achieve that though. As of now, it seems that the last profile for a given canonical name always supersedes all others in the compiler reader. This may need a change for a "merge" instead.

Right. Do context string need to be canonicalized? If it is needed, the current canonicalization needs some enhancement to support that.

A couple places in samplecontexttracker will need call getCanonicalFnName. What enhancement is needed in addition to that?

For "foo1.llvm.xxx:3 @ foo2.llvm.xxx:2 @ foo3.__uniq.xxx", do we need getCanonicalFnName to canonicalize the whole context string? Or it is divided in samplecontexttracker and getCanonicalFnName will be called for each level?

Good point. Yes, it's a concerned that the suffixes have to be removed by compiler where the underlying profile context strings are not reusable and new strings have to be made. @wenlei will have more comments about this.

[wenlei]

Actually we are not canonicalizing the names in profile from compiler side, right? We are only canonicalizing names from IR to match names from profile.

Thinking about it more, doing canonicalization in compiler for CSSPGO may be trickier, because we try to reuse a single string for a context. It's feasible to canonicalize context strings in compiler, but it will have cost.

One kind of regression is when we flip -funique-internal-linkage-name on, the profile is collected from binary built without -funique-internal-linkage-name so it has no uniq suffix, but the IR in the optimized build contains the suffix. This may introduce transient regression.

This could happen when we migrate. To tackle this one, SecFlagUniqSuffix added in this patch is good enough and we don't need to canonicalize names in profile from compiler side, as they can still be done in the profile generation tools.

In rare case, user may also want to disable -funique-internal-linkage-name after it has been enabled for a while, to triage something for example. In that case, profile will contain the suffix but IR in the optimized build won't.

How is this handled currently? Also to me this feel not so important and it's more like debugging helper. If we don't intend to support such case, I don't see a need to move all name canonicalization from profile generation tool into the compiler..

llvm/lib/ProfileData/SampleProfWriter.cpp

211

Ideally, this flag should be derived from whether the binary was built with unique names. If we try to derive the flag from existing names, whether a profile happens to not contain static symbol (even if -funique-internal-linkage-name is always on) may affect how names are canonicalized for pgo pass2 (if we have new static symbols there)..

Not sure if it would happen in practice, but this is more from consistency angle rather than a practical concern..

[wmi]

In D96932#2586378, @wenlei wrote:

Actually we are not canonicalizing the names in profile from compiler side, right? We are only canonicalizing names from IR to match names from profile.

That is right.

Thinking about it more, doing canonicalization in compiler for CSSPGO may be trickier, because we try to reuse a single string for a context. It's feasible to canonicalize context strings in compiler, but it will have cost.

One kind of regression is when we flip -funique-internal-linkage-name on, the profile is collected from binary built without -funique-internal-linkage-name so it has no uniq suffix, but the IR in the optimized build contains the suffix. This may introduce transient regression.

This could happen when we migrate. To tackle this one, SecFlagUniqSuffix added in this patch is good enough and we don't need to canonicalize names in profile from compiler side, as they can still be done in the profile generation tools.

In rare case, user may also want to disable -funique-internal-linkage-name after it has been enabled for a while, to triage something for example. In that case, profile will contain the suffix but IR in the optimized build won't.

How is this handled currently? Also to me this feel not so important and it's more like debugging helper. If we don't intend to support such case, I don't see a need to move all name canonicalization from profile generation tool into the compiler.

Thanks for pointing it out. I found I made a wrong declaration about it. I mistakenly thought the profile name was canonicalized, and even that is true, it is still tricky to support this scenario. Sorry about the wrong statement. But hopefully the case is rare enough and we don't need to support it too.

The need to move canonicalization to compiler is that profile generation tool cannot see the function attribute. An example is if the profile is generated from a go/c++ mixed code, go may have different canonicalization rule than c++. To be consistent with compiler, the profile generation tool needs to know which symbol is a go symbol and which is a c++ symbol. I don't know whether it is always easy to tell just from the name pattern. Currently it is still not an issue for us (go team did some work on autofdo support before but is not actively working on the support now), but I don't know whether it is needed for us in the future.

[wenlei]

In D96932#2586797, @wmi wrote:

In D96932#2586378, @wenlei wrote:

Actually we are not canonicalizing the names in profile from compiler side, right? We are only canonicalizing names from IR to match names from profile.

That is right.

Thinking about it more, doing canonicalization in compiler for CSSPGO may be trickier, because we try to reuse a single string for a context. It's feasible to canonicalize context strings in compiler, but it will have cost.

One kind of regression is when we flip -funique-internal-linkage-name on, the profile is collected from binary built without -funique-internal-linkage-name so it has no uniq suffix, but the IR in the optimized build contains the suffix. This may introduce transient regression.

This could happen when we migrate. To tackle this one, SecFlagUniqSuffix added in this patch is good enough and we don't need to canonicalize names in profile from compiler side, as they can still be done in the profile generation tools.

In rare case, user may also want to disable -funique-internal-linkage-name after it has been enabled for a while, to triage something for example. In that case, profile will contain the suffix but IR in the optimized build won't.

How is this handled currently? Also to me this feel not so important and it's more like debugging helper. If we don't intend to support such case, I don't see a need to move all name canonicalization from profile generation tool into the compiler.

Thanks for pointing it out. I found I made a wrong declaration about it. I mistakenly thought the profile name was canonicalized, and even that is true, it is still tricky to support this scenario. Sorry about the wrong statement. But hopefully the case is rare enough and we don't need to support it too.

The need to move canonicalization to compiler is that profile generation tool cannot see the function attribute. An example is if the profile is generated from a go/c++ mixed code, go may have different canonicalization rule than c++. To be consistent with compiler, the profile generation tool needs to know which symbol is a go symbol and which is a c++ symbol. I don't know whether it is always easy to tell just from the name pattern. Currently it is still not an issue for us (go team did some work on autofdo support before but is not actively working on the support now), but I don't know whether it is needed for us in the future.

Thanks for the clarification. In that case, agreed that we don't need to support the 2nd case of mismatch. For profile name canonicalization inside compiler, since it will have cost for csspgo, I'd suggest we leave it in llvm-profgen (we don't strip unique suffix there, but still handle others, i.e. getCanonicalFnName(.., "selected")) until the need becomes more substantial.. This change isn't doing that for either afdo or csspgo, so looks good on that front.

[wenlei]

llvm/include/llvm/ProfileData/SampleProf.h

846

If this is ok for inlinee, is it ok for standalone functions as well? I think this SymbolMap lookup has lead to extra function argument in quite a few places, so wondering how critical is that given that it still doesn't cover the inlined cases.

IIUC, with SecFlagUniqSuffix plus using "selected" policy and the fix for canonicalization upon symbolMap insertion, this patch already supported migration to turn on -funique-internal-linkage-name without regression, if c++/go mix isn't a concern, which sounds like a separate issue for the future. And if we want to cover c++/go mix, perhaps something that handles both inline and non-inline cases would be needed?

[wmi]

llvm/include/llvm/ProfileData/SampleProf.h

846

Right, currently if we simply use "selected" strategy for all here, it is of no problem in terms of supporting -funique-internal-linkage-names. Using SymbolMap here is a best effort but like you said we need to add extra function argument in a few places. Using "selected" strategy by default here and leaving the change of using SymbolMap till the point when we really need it sounds good with me.

Fundamentally I think compiler should keep declaration when a function is all inlined. The inlined and deleted function could have its name referred in debug information, and it is possible their function attributes will be needed for multiple usages.

[wmi]

Address Wenlei's comment. Also change the default elide strategy to "selected" because I think that is the most commonly used strategy.

[wmi]

Fixed a typo.

[wenlei]

lgtm, thanks.

llvm/include/llvm/ProfileData/SampleProf.h

 // Section specific flags are defined here.
 // !!!Note: Everytime a new enum class is created here, please add
 // a new check in verifySecFlag.
 enum class SecNameTableFlags : uint32_t {
   SecFlagInValid = 0,
   SecFlagMD5Name = (1 << 0),
   // Store MD5 in fixed length instead of ULEB128 so NameTable can be
   // accessed like an array.
-  SecFlagFixedLengthMD5 = (1 << 1)
+  SecFlagFixedLengthMD5 = (1 << 1),
+  // Profile contains ".__uniq." suffix name. Compiler shouldn't strip
+  // the suffix when doing profile matching when seeing the flag.
+  SecFlagUniqSuffix = (1 << 2)
 };
 enum class SecProfSummaryFlags : uint32_t {
   SecFlagInValid = 0,
   /// SecFlagPartial means the profile is for common/shared code.
   /// The common profile is usually merged from profiles collected
   /// from running other targets.
   SecFlagPartial = (1 << 0)
 };
public:
   /// 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
   /// instruction of the symbol. But as we directly get this info for raw
sampleprof_error merge(const FunctionSamples &Other, uint64_t Weight = 1) {
     return Result;
   }
 
   /// Recursively traverses all children, if the total sample count of the
   /// corresponding function is no less than \p Threshold, add its corresponding
   /// GUID to \p S. Also traverse the BodySamples to add hot CallTarget's GUID
   /// to \p S.
   void findInlinedFunctions(DenseSet<GlobalValue::GUID> &S, const Module *M,
+                            const StringMap<Function *> &SymbolMap,
                             uint64_t Threshold) const {
     if (TotalSamples <= Threshold)
       return;
     auto isDeclaration = [](const Function *F) {
       return !F || F->isDeclaration();
     };
-    if (isDeclaration(M->getFunction(getFuncName()))) {
+    if (isDeclaration(SymbolMap.lookup(getFuncName()))) {
       // Add to the import list only when it's defined out of module.
       S.insert(getGUID(Name));
     }
     // Import hot CallTargets, which may not be available in IR because full
     // profile annotation cannot be done until backend compilation in ThinLTO.
     for (const auto &BS : BodySamples)
       for (const auto &TS : BS.second.getCallTargets())
         if (TS.getValue() > Threshold) {
-          const Function *Callee = M->getFunction(getFuncName(TS.getKey()));
+          const Function *Callee = SymbolMap.lookup(getFuncName(TS.getKey()));
           if (isDeclaration(Callee))
             S.insert(getGUID(TS.getKey()));
         }
     for (const auto &CS : CallsiteSamples)
       for (const auto &NameFS : CS.second)
-        NameFS.second.findInlinedFunctions(S, M, Threshold);
+        NameFS.second.findInlinedFunctions(S, M, SymbolMap, Threshold);
   }
 
   /// Set the name of the function.
   void setName(StringRef FunctionName) { Name = FunctionName; }
 
   /// Return the function name.
   StringRef getName() const { return Name; }
 
public:
   /// Return the canonical name for a function, taking into account
   /// suffix elision policy attributes.
   static StringRef getCanonicalFnName(const Function &F) {
     auto AttrName = "sample-profile-suffix-elision-policy";
     auto Attr = F.getFnAttribute(AttrName).getValueAsString();
     return getCanonicalFnName(F.getName(), Attr);
   }
 
-  static StringRef getCanonicalFnName(StringRef FnName, StringRef Attr = "") {
-    static const char *knownSuffixes[] = { ".llvm.", ".part." };
+  /// Name suffixes which canonicalization should handle to avoid
+  /// profile mismatch.
+  static constexpr const char *LLVMSuffix = ".llvm.";
+  static constexpr const char *PartSuffix = ".part.";
+  static constexpr const char *UniqSuffix = ".__uniq.";
+
+  static StringRef getCanonicalFnName(StringRef FnName,
+                                      StringRef Attr = "selected") {
+    // Note the sequence of the suffixes in the knownSuffixes array matters.
+    // If suffix "A" is appended after the suffix "B", "A" should be in front
+    // of "B" in knownSuffixes.
+    const char *knownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix};
     if (Attr == "" || Attr == "all") {
       return FnName.split('.').first;
     } else if (Attr == "selected") {
       StringRef Cand(FnName);
       for (const auto &Suf : knownSuffixes) {
         StringRef Suffix(Suf);
+        // If the profile contains ".__uniq." suffix, don't strip the
+        // suffix for names in the IR.
+        if (Suffix == UniqSuffix && FunctionSamples::HasUniqSuffix)
+          continue;
         auto It = Cand.rfind(Suffix);
         if (It == StringRef::npos)
-          return Cand;
+          continue;
         auto Dit = Cand.rfind('.');
         if (Dit == It + Suffix.size() - 1)
           Cand = Cand.substr(0, It);
       }
       return Cand;
     } else if (Attr == "none") {
       return FnName;
     } else {
public:
   }
 
   /// Returns the line offset to the start line of the subprogram.
   /// We assume that a single function will not exceed 65535 LOC.
   static unsigned getOffset(const DILocation *DIL);
 
   /// Returns a unique call site identifier for a given debug location of a call
   /// instruction. This is wrapper of two scenarios, the probe-based profile and
   /// regular profile, to hide implementation details from the sample loader and
   /// the context tracker.
   static LineLocation getCallSiteIdentifier(const DILocation *DIL);
 
   /// Get the FunctionSamples of the inline instance where DIL originates
   /// from.
   ///
   /// The FunctionSamples of the instruction (Machine or IR) associated to
   /// \p DIL is the inlined instance in which that instruction is coming from.
public:
 
   void setContext(const SampleContext &FContext) { Context = FContext; }
 
   static SampleProfileFormat Format;
 
   /// Whether the profile uses MD5 to represent string.
   static bool UseMD5;
 
+  /// Whether the profile contains any ".__uniq." suffix in a name.
+  static bool HasUniqSuffix;
+
   /// 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) {

llvm/include/llvm/ProfileData/SampleProfReader.h

public:
   }
 
   /// The implementaion to read sample profiles from the associated file.
   virtual std::error_code readImpl() = 0;
 
   /// Print the profile for \p FName on stream \p OS.
   void dumpFunctionProfile(StringRef FName, raw_ostream &OS = dbgs());
 
-  virtual void collectFuncsFrom(const Module &M) {}
+  /// Collect functions with definitions in Module M. For reader which
+  /// support loading function profiles on demand, return true when the
+  /// reader has been given a module. Always return false for reader
+  /// which doesn't support loading function profiles on demand.
+  virtual bool collectFuncsFromModule() { return false; }
 
   /// Print all the profiles on stream \p OS.
   void dump(raw_ostream &OS = dbgs());
 
   /// Return the samples collected for function \p F.
   FunctionSamples *getSamplesFor(const Function &F) {
     // The function name may have been updated by adding suffix. Call
     // a helper to (optionally) strip off suffixes so that we can
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; }
 
   /// Don't read profile without context if the flag is set. This is only meaningful
   /// for ExtBinary format.
   virtual void setSkipFlatProf(bool Skip) {}
+  /// Return whether any name in the profile contains ".__uniq." suffix.
+  virtual bool hasUniqSuffix() { return false; }
 
   SampleProfileReaderItaniumRemapper *getRemapper() { return Remapper.get(); }
 
+  void setModule(const Module *Mod) { M = Mod; }
+
 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;
 
protected:
   /// Whether function profiles are context-sensitive.
   bool ProfileIsCS = false;
 
   /// Number of context-sensitive profiles.
   uint32_t CSProfileCount = 0;
 
   /// \brief The format of sample.
   SampleProfileFormat Format = SPF_None;
+
+  /// \brief The current module being compiled if SampleProfileReader
+  /// is used by compiler. If SampleProfileReader is used by other
+  /// tools which are not compiler, M is usually nullptr.
+  const Module *M = nullptr;
 };
 
 class SampleProfileReaderText : public SampleProfileReader {
 public:
   SampleProfileReaderText(std::unique_ptr<MemoryBuffer> B, LLVMContext &C)
       : SampleProfileReader(std::move(B), C, SPF_Text) {}
 
   /// Read and validate the file header.
protected:
 
   std::unique_ptr<ProfileSymbolList> ProfSymList;
 
   /// The table mapping from function name to the offset of its FunctionSample
   /// towards file start.
   DenseMap<StringRef, uint64_t> FuncOffsetTable;
   /// The set containing the functions to use when compiling a module.
   DenseSet<StringRef> FuncsToUse;
-  /// Use all functions from the input profile.
-  bool UseAllFuncs = true;
 
   /// Use fixed length MD5 instead of ULEB128 encoding so NameTable doesn't
   /// need to be read in up front and can be directly accessed using index.
   bool FixedLengthMD5 = false;
   /// The starting address of NameTable containing fixed length MD5.
   const uint8_t *MD5NameMemStart = nullptr;
 
   /// If MD5 is used in NameTable section, the section saves uint64_t data.
public:
   std::error_code readImpl() override;
 
   /// Get the total size of all \p Type sections.
   uint64_t getSectionSize(SecType Type);
   /// Get the total size of header and all sections.
   uint64_t getFileSize();
   virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) override;
 
-  /// Collect functions with definitions in Module \p M.
-  void collectFuncsFrom(const Module &M) override;
+  /// Collect functions with definitions in Module M. Return true if
+  /// the reader has been given a module.
+  bool collectFuncsFromModule() override;
 
   /// Return whether names in the profile are all MD5 numbers.
   virtual bool useMD5() override { return MD5StringBuf.get(); }
 
   virtual std::unique_ptr<ProfileSymbolList> getProfileSymbolList() override {
     return std::move(ProfSymList);
   };
 

 private:
   /// Function name table.
   std::vector<std::string> NameTable;
   /// The table mapping from function name to the offset of its FunctionSample
   /// towards file start.
   DenseMap<StringRef, uint64_t> FuncOffsetTable;
   /// The set containing the functions to use when compiling a module.
   DenseSet<StringRef> FuncsToUse;
-  /// Use all functions from the input profile.
-  bool UseAllFuncs = true;
   virtual std::error_code verifySPMagic(uint64_t Magic) override;
   virtual std::error_code readNameTable() override;
   /// Read a string indirectly via the name table.
   virtual ErrorOr<StringRef> readStringFromTable() override;
   virtual std::error_code readHeader() override;
   std::error_code readFuncOffsetTable();
 
 public:
   SampleProfileReaderCompactBinary(std::unique_ptr<MemoryBuffer> B,
                                    LLVMContext &C)
       : SampleProfileReaderBinary(std::move(B), C, SPF_Compact_Binary) {}
 
   /// \brief Return true if \p Buffer is in the format supported by this class.
   static bool hasFormat(const MemoryBuffer &Buffer);
 
   /// Read samples only for functions to use.
   std::error_code readImpl() override;
 
-  /// Collect functions to be used when compiling Module \p M.
-  void collectFuncsFrom(const Module &M) override;
+  /// Collect functions with definitions in Module M. Return true if
+  /// the reader has been given a module.
+  bool collectFuncsFromModule() override;
 
   /// Return whether names in the profile are all MD5 numbers.
   virtual bool useMD5() override { return true; }
 };
 
 using InlineCallStack = SmallVector<FunctionSamples *, 10>;
 
 // Supported histogram types in GCC.  Currently, we only need support for

llvm/lib/ProfileData/SampleProf.cpp

static cl::opt<uint64_t> ProfileSymbolListCutOff(
     cl::desc("Cutoff value about how many symbols in profile symbol list "
              "will be used. This is very useful for performance debugging"));
 
 namespace llvm {
 namespace sampleprof {
 SampleProfileFormat FunctionSamples::Format;
 bool FunctionSamples::ProfileIsProbeBased = false;
 bool FunctionSamples::ProfileIsCS = false;
-bool FunctionSamples::UseMD5;
+bool FunctionSamples::UseMD5 = false;
+bool FunctionSamples::HasUniqSuffix = true;
 } // namespace sampleprof
 } // namespace llvm
 
 namespace {
 
 // FIXME: This class is only here to support the transition to llvm::Error. It
 // will be removed once this transition is complete. Clients should prefer to
 // deal with the Error value directly, rather than converting to error_code.
for (const auto &NameFS : CS.second) {
       NameFS.second.findAllNames(NameSet);
     }
   }
 }
 
 const FunctionSamples *FunctionSamples::findFunctionSamplesAt(
     const LineLocation &Loc, StringRef CalleeName,
     SampleProfileReaderItaniumRemapper *Remapper) const {
+  CalleeName = getCanonicalFnName(CalleeName);
+
   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

llvm/lib/ProfileData/SampleProfReader.cpp

if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial))
       Summary->setPartialProfile(true);
     break;
   case SecNameTable: {
     FixedLengthMD5 =
         hasSecFlag(Entry, SecNameTableFlags::SecFlagFixedLengthMD5);
     bool UseMD5 = hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name);
     assert((!FixedLengthMD5 || UseMD5) &&
            "If FixedLengthMD5 is true, UseMD5 has to be true");
+    FunctionSamples::HasUniqSuffix =
+        hasSecFlag(Entry, SecNameTableFlags::SecFlagUniqSuffix);
     if (std::error_code EC = readNameTableSec(UseMD5))
       return EC;
     break;
   }
   case SecLBRProfile:
     if (std::error_code EC = readFuncProfiles())
       return EC;
     break;
std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
   default:
     if (std::error_code EC = readCustomSection(Entry))
       return EC;
     break;
   }
   return sampleprof_error::success;
 }
 
-void SampleProfileReaderExtBinaryBase::collectFuncsFrom(const Module &M) {
-  UseAllFuncs = false;
+bool SampleProfileReaderExtBinaryBase::collectFuncsFromModule() {
+  if (!M)
+    return false;
   FuncsToUse.clear();
-  for (auto &F : M)
+  for (auto &F : *M)
     FuncsToUse.insert(FunctionSamples::getCanonicalFnName(F));
+  return true;
 }
 
 std::error_code SampleProfileReaderExtBinaryBase::readFuncOffsetTable() {
   // If there are more than one FuncOffsetTable, the profile read associated
   // with previous FuncOffsetTable has to be done before next FuncOffsetTable
   // is read.
   FuncOffsetTable.clear();
 
if (std::error_code EC = Offset.getError())
       return EC;
 
     FuncOffsetTable[*FName] = *Offset;
   }
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() {
+  // Collect functions used by current module if the Reader has been
+  // given a module.
+  // collectFuncsFromModule uses FunctionSamples::getCanonicalFnName
+  // which will query FunctionSamples::HasUniqSuffix, so it has to be
+  // called after FunctionSamples::HasUniqSuffix is set, i.e. after
+  // NameTable section is read.
+  bool LoadFuncsToBeUsed = collectFuncsFromModule();
+
+  // When LoadFuncsToBeUsed is false, load all the function profiles.
   const uint8_t *Start = Data;
-  if (UseAllFuncs) {
+  if (!LoadFuncsToBeUsed) {
     while (Data < End) {
       if (std::error_code EC = readFuncProfile(Data))
         return EC;
     }
     assert(Data == End && "More data is read than expected");
   } else {
+    // Load function profiles on demand.
     if (Remapper) {
       for (auto Name : FuncsToUse) {
         Remapper->insert(Name);
       }
     }
 
     if (useMD5()) {
       for (auto Name : FuncsToUse) {
if (useMD5()) {
         const uint8_t *FuncProfileAddr = Start + NameOffset.second;
         assert(FuncProfileAddr < End && "out of LBRProfile section");
         if (std::error_code EC = readFuncProfile(FuncProfileAddr))
           return EC;
       }
     }
     Data = End;
   }
-
   assert((CSProfileCount == 0 || CSProfileCount == Profiles.size()) &&
          "Cannot have both context-sensitive and regular profile");
   ProfileIsCS = (CSProfileCount > 0);
   FunctionSamples::ProfileIsCS = ProfileIsCS;
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileReaderExtBinaryBase::readProfileSymbolList() {
if (isCompressed) {
       End = BufStart + Buffer->getBufferSize();
     }
   }
 
   return sampleprof_error::success;
 }
 
 std::error_code SampleProfileReaderCompactBinary::readImpl() {
+  // Collect functions used by current module if the Reader has been
+  // given a module.
+  bool LoadFuncsToBeUsed = collectFuncsFromModule();
+
   std::vector<uint64_t> OffsetsToUse;
-  if (UseAllFuncs) {
+  if (!LoadFuncsToBeUsed) {
+    // load all the function profiles.
     for (auto FuncEntry : FuncOffsetTable) {
       OffsetsToUse.push_back(FuncEntry.second);
     }
-  }
-  else {
+  } else {
+    // load function profiles on demand.
     for (auto Name : FuncsToUse) {
       auto GUID = std::to_string(MD5Hash(Name));
       auto iter = FuncOffsetTable.find(StringRef(GUID));
       if (iter == FuncOffsetTable.end())
         continue;
       OffsetsToUse.push_back(iter->second);
     }
   }
if (hasSecFlag(Entry, SecCommonFlags::SecFlagFlat))
     Flags.append("flat,");
 
   switch (Entry.Type) {
   case SecNameTable:
     if (hasSecFlag(Entry, SecNameTableFlags::SecFlagFixedLengthMD5))
       Flags.append("fixlenmd5,");
     else if (hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name))
       Flags.append("md5,");
+    if (hasSecFlag(Entry, SecNameTableFlags::SecFlagUniqSuffix))
+      Flags.append("uniq,");
     break;
   case SecProfSummary:
     if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial))
       Flags.append("partial,");
     break;
   default:
     break;
   }
for (uint32_t I = 0; I < *Size; ++I) {
 
     FuncOffsetTable[*FName] = *Offset;
   }
   End = TableStart;
   Data = SavedData;
   return sampleprof_error::success;
 }
 
-void SampleProfileReaderCompactBinary::collectFuncsFrom(const Module &M) {
-  UseAllFuncs = false;
+bool SampleProfileReaderCompactBinary::collectFuncsFromModule() {
+  if (!M)
+    return false;
   FuncsToUse.clear();
-  for (auto &F : M)
+  for (auto &F : *M)
     FuncsToUse.insert(FunctionSamples::getCanonicalFnName(F));
+  return true;
 }
 
 std::error_code SampleProfileReaderBinary::readSummaryEntry(
     std::vector<ProfileSummaryEntry> &Entries) {
   auto Cutoff = readNumber<uint64_t>();
   if (std::error_code EC = Cutoff.getError())
     return EC;
 

llvm/lib/ProfileData/SampleProfWriter.cpp

 }
 
 std::error_code SampleProfileWriterExtBinaryBase::writeNameTableSection(
     const StringMap<FunctionSamples> &ProfileMap) {
   for (const auto &I : ProfileMap) {
     addName(I.first());
     addNames(I.second);
   }
+
+  // If NameTable contains ".__uniq." suffix, set SecFlagUniqSuffix flag
+  // so compiler won't strip the suffix during profile matching after
+  // seeing the flag in the profile.
+  for (const auto &I : NameTable) {
+    if (I.first.find(FunctionSamples::UniqSuffix) != StringRef::npos) {
+      addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagUniqSuffix);
+      break;
+    }
+  }
+
   if (auto EC = writeNameTable())
     return EC;
   return sampleprof_error::success;
 }
 
 std::error_code
 SampleProfileWriterExtBinaryBase::writeProfileSymbolListSection() {
   if (ProfSymList && ProfSymList->size() > 0)

llvm/lib/Transforms/IPO/SampleContextTracker.cpp

 FunctionSamples *
 SampleContextTracker::getCalleeContextSamplesFor(const CallBase &Inst,
                                                  StringRef CalleeName) {
   LLVM_DEBUG(dbgs() << "Getting callee context for instr: " << Inst << "\n");
   DILocation *DIL = Inst.getDebugLoc();
   if (!DIL)
     return nullptr;
 
+  CalleeName = FunctionSamples::getCanonicalFnName(CalleeName);
+
   // For indirect call, CalleeName will be empty, in which case the context
   // profile for callee with largest total samples will be returned.
   ContextTrieNode *CalleeContext = getCalleeContextFor(DIL, CalleeName);
   if (CalleeContext) {
     FunctionSamples *FSamples = CalleeContext->getFunctionSamples();
     LLVM_DEBUG(if (FSamples) {
       dbgs() << "  Callee context found: " << FSamples->getContext() << "\n";
     });
     return FSamples;
   }
 
   return nullptr;
 }
 
 std::vector<const FunctionSamples *>
 SampleContextTracker::getIndirectCalleeContextSamplesFor(
     const DILocation *DIL) {
   std::vector<const FunctionSamples *> R;
   if (!DIL)

llvm/lib/Transforms/IPO/SampleProfile.cpp

 SampleProfileLoader::findCalleeFunctionSamples(const CallBase &Inst) const {
   const DILocation *DIL = Inst.getDebugLoc();
   if (!DIL) {
     return nullptr;
   }
 
   StringRef CalleeName;
   if (Function *Callee = Inst.getCalledFunction())
-    CalleeName = FunctionSamples::getCanonicalFnName(*Callee);
+    CalleeName = Callee->getName();
 
   if (ProfileIsCS)
     return ContextTracker->getCalleeContextSamplesFor(Inst, CalleeName);
 
   const FunctionSamples *FS = findFunctionSamples(Inst);
   if (FS == nullptr)
     return nullptr;
 
for (CallBase *I : CIS) {
       // Do not inline recursive calls.
       if (CalledFunction == &F)
         continue;
       if (I->isIndirectCall()) {
         uint64_t Sum;
         for (const auto *FS : findIndirectCallFunctionSamples(*I, Sum)) {
           uint64_t SumOrigin = Sum;
           if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) {
-            FS->findInlinedFunctions(InlinedGUIDs, F.getParent(),
+            FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), SymbolMap,
                                      PSI->getOrCompHotCountThreshold());
             continue;
           }
           if (!callsiteIsHot(FS, PSI, ProfAccForSymsInList))
             continue;
 
           Candidate = {I, FS, FS->getEntrySamples(), 1.0};
           if (tryPromoteAndInlineCandidate(F, Candidate, SumOrigin, Sum)) {
             LocalNotInlinedCallSites.erase(I);
             LocalChanged = true;
           }
         }
       } else if (CalledFunction && CalledFunction->getSubprogram() &&
                  !CalledFunction->isDeclaration()) {
         if (tryInlineCandidate(Candidate)) {
           LocalNotInlinedCallSites.erase(I);
           LocalChanged = true;
         }
       } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) {
         findCalleeFunctionSamples(*I)->findInlinedFunctions(
-            InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold());
+            InlinedGUIDs, F.getParent(), SymbolMap,
+            PSI->getOrCompHotCountThreshold());
       }
     }
     Changed |= LocalChanged;
   }
 
   // For CS profile, profile for not inlined context will be merged when
   // base profile is being trieved
   if (ProfileIsCS)
while (!CQueue.empty() && F.getInstructionCount() < SizeLimit) {
     if (I->isIndirectCall()) {
       uint64_t Sum;
       auto CalleeSamples = findIndirectCallFunctionSamples(*I, Sum);
       uint64_t SumOrigin = Sum;
       Sum *= Candidate.CallsiteDistribution;
       for (const auto *FS : CalleeSamples) {
         // TODO: Consider disable pre-lTO ICP for MonoLTO as well
         if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) {
-          FS->findInlinedFunctions(InlinedGUIDs, F.getParent(),
+          FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), SymbolMap,
                                    PSI->getOrCompHotCountThreshold());
           continue;
         }
         uint64_t EntryCountDistributed =
             FS->getEntrySamples() * Candidate.CallsiteDistribution;
         // In addition to regular inline cost check, we also need to make sure
         // ICP isn't introducing excessive speculative checks even if individual
         // target looks beneficial to promote and inline. That means we should
if (I->isIndirectCall()) {
         for (auto *CB : InlinedCallSites) {
           if (getInlineCandidate(&NewCandidate, CB))
             CQueue.emplace(NewCandidate);
         }
         Changed = true;
       }
     } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) {
       findCalleeFunctionSamples(*I)->findInlinedFunctions(
-          InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold());
+          InlinedGUIDs, F.getParent(), SymbolMap,
+          PSI->getOrCompHotCountThreshold());
     }
   }
 
   if (!CQueue.empty()) {
     if (SizeLimit == (unsigned)ProfileInlineLimitMax)
       ++NumCSInlinedHitMaxLimit;
     else if (SizeLimit == (unsigned)ProfileInlineLimitMin)
       ++NumCSInlinedHitMinLimit;
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());
   Reader->setSkipFlatProf(LTOPhase == ThinOrFullLTOPhase::ThinLTOPostLink);
-  Reader->collectFuncsFrom(M);
+  // set module before reading the profile so reader may be able to only
+  // read the function profiles which are used by the current module.
+  Reader->setModule(&M);
   if (std::error_code EC = Reader->read()) {
     std::string Msg = "profile reading failed: " + EC.message();
     Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg));
     return false;
   }
 
   PSL = Reader->getProfileSymbolList();
 
bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM,
   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)
+    if (F == nullptr || OrigName.empty())
       continue;
     SymbolMap[OrigName] = F;
-    auto pos = OrigName.find('.');
-    if (pos != StringRef::npos) {
-      StringRef NewName = OrigName.substr(0, pos);
+    StringRef NewName = FunctionSamples::getCanonicalFnName(*F);
+    if (OrigName != NewName && !NewName.empty()) {
       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;
+        if (*MapName != OrigName && !MapName->empty())
           SymbolMap.insert(std::make_pair(*MapName, F));
       }
     }
   }
+  assert(SymbolMap.count(StringRef()) == 0 &&
+         "No empty StringRef should be added in SymbolMap");
 
   bool retval = false;
   for (auto F : buildFunctionOrder(M, CG)) {
     assert(!F->isDeclaration());
     clearFunctionData();
     retval |= runOnFunction(*F, AM);
   }
 

llvm/test/Transforms/SampleProfile/Inputs/uniqname.suffix.prof

+_Z3foov:225715:5930
+ 2: 5553
+ 3: 5391
+ 3: _ZL3moov.__uniq.334154460836426447066042049082945760258:5860
+  1: 5279 _Z10moo_calleev:5279
+  2: 5279
+  3: 5000
+  3: _ZL3noov.__uniq.334154460836426447066042049082945760258:5860
+    1: 5000
+    2: 2000 _Z10noo_calleev:2000
+_ZL3goov.__uniq.334154460836426447066042049082945760258:5860:5860
+ 1: 5279
+ 1: _ZL3hoov.__uniq.334154460836426447066042049082945760258:5860
+   1: 5000

llvm/test/Transforms/SampleProfile/uniqname.ll

+; Make sure profile matching is successful if both profile and IR contain
+; ".__uniq." suffix, for text format or extbinary format profile.
+; Make sure profile matching is successful if IR contains ".__uniq." suffix
+; but profile doesn't contain the suffix, for extbinary format profile.
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/uniqname.suffix.prof -S | FileCheck %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/uniqname.suffix.afdo -S | FileCheck %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/uniqname.nosuffix.afdo -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@cond = dso_local global i8 0, align 1
+@p = dso_local global void ()* null, align 8
+
+; Check the callsite in inlined function with uniq suffix is annotated with
+; profile correctly.
+; CHECK-LABEL: @_Z3foov(
+; CHECK: call void @_Z10moo_calleev(), {{.*}} !prof ![[PROF_ID1:[0-9]+]]
+; CHECK: call void @_Z10noo_calleev(), {{.*}} !prof ![[PROF_ID2:[0-9]+]]
+; CHECK: ret void
+
+; Function Attrs: uwtable mustprogress
+define dso_local void @_Z3foov() #0 !dbg !7 {
+entry:
+  store void ()* @_ZL3hoov.__uniq.334154460836426447066042049082945760258, void ()** @p, align 8, !dbg !9, !tbaa !10
+  call void @_ZL3goov.__uniq.334154460836426447066042049082945760258.llvm.4206369970847378271(), !dbg !14
+  call void @_ZL3moov.__uniq.334154460836426447066042049082945760258(), !dbg !15
+  ret void, !dbg !16
+}
+
+; Function Attrs: uwtable mustprogress
+define internal void @_ZL3hoov.__uniq.334154460836426447066042049082945760258() #1 !dbg !17 {
+entry:
+  call void @_Z10hoo_calleev(), !dbg !18
+  ret void, !dbg !19
+}
+
+; Check the indirect call target with uniq suffix is promoted and the inlined
+; body is annotated with profile.
+; CHECK: define internal void @_ZL3goov.__uniq.334154460836426447066042049082945760258.llvm.4206369970847378271{{.*}} !prof ![[PROF_ID3:[0-9]+]]
+; CHECK: icmp eq void ()* {{.*}} @_ZL3hoov.__uniq.334154460836426447066042049082945760258
+; CHECK: call void @_Z10hoo_calleev(), {{.*}} !prof ![[PROF_ID4:[0-9]+]]
+
+; Function Attrs: noinline uwtable mustprogress
+define internal void @_ZL3goov.__uniq.334154460836426447066042049082945760258.llvm.4206369970847378271() #2 !dbg !20 {
+entry:
+  %0 = load void ()*, void ()** @p, align 8, !dbg !21, !tbaa !10
+  call void %0(), !dbg !22
+  ret void, !dbg !23
+}
+
+; Function Attrs: uwtable mustprogress
+define internal void @_ZL3moov.__uniq.334154460836426447066042049082945760258() #1 !dbg !24 {
+entry:
+  call void @_Z10moo_calleev(), !dbg !25
+  %0 = load volatile i8, i8* @cond, align 1, !dbg !26, !tbaa !27, !range !29
+  %tobool.not = icmp eq i8 %0, 0, !dbg !26
+  br i1 %tobool.not, label %if.end, label %if.then, !dbg !26
+
+if.then:                                          ; preds = %entry
+  call void @_ZL3noov.__uniq.334154460836426447066042049082945760258(), !dbg !30
+  br label %if.end, !dbg !30
+
+if.end:                                           ; preds = %if.then, %entry
+  ret void, !dbg !31
+}
+
+declare !dbg !32 dso_local void @_Z10hoo_calleev() #3
+
+declare !dbg !33 dso_local void @_Z10moo_calleev() #3
+
+; Function Attrs: uwtable mustprogress
+define internal void @_ZL3noov.__uniq.334154460836426447066042049082945760258() #1 !dbg !34 {
+entry:
+  %0 = load volatile i8, i8* @cond, align 1, !dbg !35, !tbaa !27, !range !29
+  %tobool.not = icmp eq i8 %0, 0, !dbg !35
+  br i1 %tobool.not, label %if.end, label %if.then, !dbg !35
+
+if.then:                                          ; preds = %entry
+  call void @_Z10noo_calleev(), !dbg !36
+  br label %if.end, !dbg !36
+
+if.end:                                           ; preds = %if.then, %entry
+  ret void, !dbg !37
+}
+
+declare !dbg !38 dso_local void @_Z10noo_calleev() #3
+
+attributes #0 = { uwtable mustprogress "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-sample-profile" "use-soft-float"="false" }
+attributes #1 = { uwtable mustprogress "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "sample-profile-suffix-elision-policy"="selected" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-sample-profile" "use-soft-float"="false" }
+attributes #2 = { noinline uwtable mustprogress "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "sample-profile-suffix-elision-policy"="selected" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-sample-profile" "use-soft-float"="false" }
+attributes #3 = { "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+; CHECK: ![[PROF_ID1]] = !{!"branch_weights", i32 5931}
+; CHECK: ![[PROF_ID2]] = !{!"branch_weights", i32 2000}
+; CHECK: ![[PROF_ID3]] = !{!"function_entry_count", i64 5861}
+; CHECK: ![[PROF_ID4]] = !{!"branch_weights", i32 5000}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 12.0.0 (https://github.com/llvm/llvm-project.git e42c17446a2e5cbf9eebc752beafadad3fac7f63)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "1.cc", directory: "/usr/local/google/home/wmi/workarea/llvm/build/splitprofile")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 12.0.0 (https://github.com/llvm/llvm-project.git e42c17446a2e5cbf9eebc752beafadad3fac7f63)"}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 28, type: !8, scopeLine: 28, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 29, column: 5, scope: !7)
+!10 = !{!11, !11, i64 0}
+!11 = !{!"any pointer", !12, i64 0}
+!12 = !{!"omnipotent char", !13, i64 0}
+!13 = !{!"Simple C++ TBAA"}
+!14 = !DILocation(line: 30, column: 3, scope: !7)
+!15 = !DILocation(line: 31, column: 3, scope: !7)
+!16 = !DILocation(line: 32, column: 3, scope: !7)
+!17 = distinct !DISubprogram(name: "hoo", linkageName: "_ZL3hoov.__uniq.334154460836426447066042049082945760258", scope: !1, file: !1, line: 17, type: !8, scopeLine: 17, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!18 = !DILocation(line: 18, column: 3, scope: !17)
+!19 = !DILocation(line: 19, column: 1, scope: !17)
+!20 = distinct !DISubprogram(name: "goo", linkageName: "_ZL3goov.__uniq.334154460836426447066042049082945760258.llvm.4206369970847378271", scope: !1, file: !1, line: 24, type: !8, scopeLine: 24, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!21 = !DILocation(line: 25, column: 5, scope: !20)
+!22 = !DILocation(line: 25, column: 3, scope: !20)
+!23 = !DILocation(line: 26, column: 1, scope: !20)
+!24 = distinct !DISubprogram(name: "moo", linkageName: "_ZL3moov.__uniq.334154460836426447066042049082945760258", scope: !1, file: !1, line: 11, type: !8, scopeLine: 11, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!25 = !DILocation(line: 12, column: 3, scope: !24)
+!26 = !DILocation(line: 13, column: 7, scope: !24)
+!27 = !{!28, !28, i64 0}
+!28 = !{!"bool", !12, i64 0}
+!29 = !{i8 0, i8 2}
+!30 = !DILocation(line: 14, column: 5, scope: !24)
+!31 = !DILocation(line: 15, column: 1, scope: !24)
+!32 = !DISubprogram(name: "hoo_callee", linkageName: "_Z10hoo_calleev", scope: !1, file: !1, line: 3, type: !8, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!33 = !DISubprogram(name: "moo_callee", linkageName: "_Z10moo_calleev", scope: !1, file: !1, line: 2, type: !8, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!34 = distinct !DISubprogram(name: "noo", linkageName: "_ZL3noov.__uniq.334154460836426447066042049082945760258", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!35 = !DILocation(line: 7, column: 7, scope: !34)
+!36 = !DILocation(line: 8, column: 5, scope: !34)
+!37 = !DILocation(line: 9, column: 1, scope: !34)
+!38 = !DISubprogram(name: "noo_callee", linkageName: "_Z10noo_calleev", scope: !1, file: !1, line: 1, type: !8, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)

llvm/unittests/ProfileData/SampleProfTest.cpp

struct SampleProfTest : ::testing::Test {
   }
 
   void readProfile(const Module &M, StringRef Profile,
                    StringRef RemapFile = "") {
     auto ReaderOrErr = SampleProfileReader::create(
         std::string(Profile), Context, std::string(RemapFile));
     ASSERT_TRUE(NoError(ReaderOrErr.getError()));
     Reader = std::move(ReaderOrErr.get());
-    Reader->collectFuncsFrom(M);
+    Reader->setModule(&M);
   }
 
   TempFile createRemapFile() {
     return TempFile("remapfile", "", R"(
       # Types 'int' and 'long' are equivalent
       type i l
       # Function names 'foo' and 'faux' are equivalent
       name 3foo 4faux