[llvm-profdata]Fix llvm-profdata crash on compact binary profile

[wlei]

Summary

llvm-profdata show and overlap will crash in getFuncName on compact binary profile. This change fixed this by switching to use getName.

getFuncName is misused in llvm-profdata. As showed below, GUIDToFuncNameMap is only supported in compilation mode, there is no initialization in llvm-profdata. Compact profile whose MD5 is true would try to query GUIDToFuncNameMap then caused the crash. So fix this by switching to getName

[wlei]

fix lint

[wlei]

fix lint

[hoy]

Thanks for the fix! LGTM.

I'm also seeing InstrProfSymtab is there to handle MD5-based names for PGO instrumentation. Not sure how it is used exactly. @wmi Have we thought about using that for AutoFDO?

[MaskRay]

llvm/test/tools/llvm-profdata/compact-sample.proftext

3

| -> |

10

| -> |

(Nit: for FileCheck options, the two-dash --check-prefix is preferred.)

18

Indent this line so that the indentation difference with the next line matches the actual output.

[wlei]

fix lint in compact-sample.proftext

[MaskRay]

LGTM. Worth waiting a bit in case there are other opinions.

[weihe]

Thank you for fixing it!

[wenlei]

LGTM. A small nit: the code snippet in the description could be removed as it's indeed in the codebase already. :)

[wmi]

In D87740#2277446, @hoy wrote:

Thanks for the fix! LGTM.

I'm also seeing InstrProfSymtab is there to handle MD5-based names for PGO instrumentation. Not sure how it is used exactly. @wmi Have we thought about using that for AutoFDO?

Looks like there are some functionality overlap between InstrProfSymtab and AutoFDO MD5 lookup. It is worth noting that for Instrumentation PGO profile, function names present in the profile, and for compact format profile in AutoFDO, there are only MD5 and no names are present in the profile -- in order to keep the profile compact, so it is not that trivial to extract the common part but it is definitely possible.

[wlei]

In D87740#2278197, @wenlei wrote:

LGTM. A small nit: the code snippet in the description could be removed as it's indeed in the codebase already. :)

Yes, thanks for the suggestion

llvm/test/tools/llvm-profdata/compact-sample.proftext

3

Fixed, thanks for your feedback!

10

Fixed, thanks for your feedback!

18

Fixed, thanks for your feedback!

[wenlei]

Landed the change on behalf of @wlei.

llvm/test/tools/llvm-profdata/compact-sample.proftext

 # Make sure "llvm-profdata show" works for sample profile in binary compact format
 
-# RUN: llvm-profdata show -sample %S/Inputs/compat-sample.profdata  | FileCheck %s 
+# RUN: llvm-profdata show -sample %S/Inputs/compat-sample.profdata | FileCheck %s
 
-# CHECK: Function: 15822663052811949562: 17, 0, 6 sampled lines 
+# CHECK: Function: 15822663052811949562: 17, 0, 6 sampled lines
 # CHECK-NEXT: Samples collected in the function's body {
 # CHECK: Samples collected in inlined callsites {
 # CHECK-NEXT: 1: inlined callee: 6309742469962978389: 17, 0, 1
+
+# RUN: llvm-profdata show -hot-func-list -sample %S/Inputs/compat-sample.profdata | FileCheck %s --check-prefix=HOTFUNC
+# HOTFUNC:      24753993 out of 24754010 profile counts (100.00%) are from hot functions.
+# HOTFUNC:       Total sample (%)       Max sample        Entry sample    Function name
+# HOTFUNC-NEXT:  24753993 (100.00%)     1284671           1284671         16429767378996342100
+
+# Make sure "llvm-profdata overlap" works for sample profile in binary compact format
+# RUN: llvm-profdata overlap -sample %S/Inputs/compat-sample.profdata %S/Inputs/compat-sample.profdata | FileCheck %s --check-prefix=OVERLAP
+
+# OVERLAP:      Program level:
+# OVERLAP-NEXT:   Whole program profile similarity: 100.000%
+# OVERLAP-NEXT:   Whole program sample overlap: 100.000%

llvm/tools/llvm-profdata/llvm-profdata.cpp

 }
 
 void SampleOverlapAggregator::computeSampleProfileOverlap(raw_fd_ostream &OS) {
   using namespace sampleprof;
 
   StringMap<const FunctionSamples *> BaseFuncProf;
   const auto &BaseProfiles = BaseReader->getProfiles();
   for (const auto &BaseFunc : BaseProfiles) {
-    BaseFuncProf.try_emplace(BaseFunc.second.getFuncName(), &(BaseFunc.second));
+    BaseFuncProf.try_emplace(BaseFunc.second.getName(), &(BaseFunc.second));
   }
   ProfOverlap.UnionCount = BaseFuncProf.size();
 
   const auto &TestProfiles = TestReader->getProfiles();
   for (const auto &TestFunc : TestProfiles) {
     SampleOverlapStats FuncOverlap;
-    FuncOverlap.TestName = TestFunc.second.getFuncName();
+    FuncOverlap.TestName = TestFunc.second.getName();
     assert(TestStats.count(FuncOverlap.TestName) &&
            "TestStats should have records for all functions in test profile "
            "except inlinees");
     FuncOverlap.TestSample = TestStats[FuncOverlap.TestName].SampleSum;
 
     const auto Match = BaseFuncProf.find(FuncOverlap.TestName);
     if (Match == BaseFuncProf.end()) {
       const FuncSampleStats &FuncStats = TestStats[FuncOverlap.TestName];
if (Match == BaseFuncProf.end()) {
 
       ++ProfOverlap.UnionCount;
       ProfOverlap.UnionSample += FuncStats.SampleSum;
     } else {
       ++ProfOverlap.OverlapCount;
 
       // Two functions match with each other. Compute function-level overlap and
       // aggregate them into profile-level overlap.
-      FuncOverlap.BaseName = Match->second->getFuncName();
+      FuncOverlap.BaseName = Match->second->getName();
       assert(BaseStats.count(FuncOverlap.BaseName) &&
              "BaseStats should have records for all functions in base profile "
              "except inlinees");
       FuncOverlap.BaseSample = BaseStats[FuncOverlap.BaseName].SampleSum;
 
       FuncOverlap.Similarity = computeSampleFunctionOverlap(
           Match->second, &TestFunc.second, &FuncOverlap, FuncOverlap.BaseSample,
           FuncOverlap.TestSample);
if (TestStats[FuncOverlap.TestName].MaxSample >= FuncFilter.ValueCutoff ||
       FuncOverlap.TestWeight =
           static_cast<double>(FuncOverlap.TestSample) / ProfOverlap.TestSample;
       FuncSimilarityDump.emplace(FuncOverlap.BaseWeight, FuncOverlap);
     }
   }
 
   // Traverse through functions in base profile but not in test profile.
   for (const auto &F : BaseFuncProf) {
-    assert(BaseStats.count(F.second->getFuncName()) &&
+    assert(BaseStats.count(F.second->getName()) &&
            "BaseStats should have records for all functions in base profile "
            "except inlinees");
-    const FuncSampleStats &FuncStats = BaseStats[F.second->getFuncName()];
+    const FuncSampleStats &FuncStats = BaseStats[F.second->getName()];
     ++ProfOverlap.BaseUniqueCount;
     ProfOverlap.BaseUniqueSample += FuncStats.SampleSum;
 
     updateHotBlockOverlap(FuncStats.SampleSum, 0, FuncStats.HotBlockCount);
 
     double FuncSimilarity = computeSampleFunctionOverlap(
         nullptr, nullptr, nullptr, FuncStats.SampleSum, 0);
     ProfOverlap.Similarity +=

 
 void SampleOverlapAggregator::initializeSampleProfileOverlap() {
   const auto &BaseProf = BaseReader->getProfiles();
   for (const auto &I : BaseProf) {
     ++ProfOverlap.BaseCount;
     FuncSampleStats FuncStats;
     getFuncSampleStats(I.second, FuncStats, BaseHotThreshold);
     ProfOverlap.BaseSample += FuncStats.SampleSum;
-    BaseStats.try_emplace(I.second.getFuncName(), FuncStats);
+    BaseStats.try_emplace(I.second.getName(), FuncStats);
   }
 
   const auto &TestProf = TestReader->getProfiles();
   for (const auto &I : TestProf) {
     ++ProfOverlap.TestCount;
     FuncSampleStats FuncStats;
     getFuncSampleStats(I.second, FuncStats, TestHotThreshold);
     ProfOverlap.TestSample += FuncStats.SampleSum;
-    TestStats.try_emplace(I.second.getFuncName(), FuncStats);
+    TestStats.try_emplace(I.second.getName(), FuncStats);
   }
 
   ProfOverlap.BaseName = StringRef(BaseFilename);
   ProfOverlap.TestName = StringRef(TestFilename);
 }
 
 void SampleOverlapAggregator::dumpFuncSimilarity(raw_fd_ostream &OS) const {
   using namespace sampleprof;
std::string Metric =
       std::string("max sample >= ") + std::to_string(MinCountThreshold);
   std::vector<HotFuncInfo> PrintValues;
   for (const auto &FuncPair : HotFunc) {
     const FunctionSamples &Func = *FuncPair.second.first;
     double TotalSamplePercent =
         (ProfileTotalSample > 0)
             ? (Func.getTotalSamples() * 100.0) / ProfileTotalSample
             : 0;
-    PrintValues.emplace_back(HotFuncInfo(
-        Func.getFuncName(), Func.getTotalSamples(), TotalSamplePercent,
+    PrintValues.emplace_back(
+        HotFuncInfo(Func.getName(), Func.getTotalSamples(), TotalSamplePercent,
                     FuncPair.second.second, Func.getEntrySamples()));
   }
   dumpHotFunctionList(ColumnTitle, ColumnOffset, PrintValues, HotFuncCount,
                       Profiles.size(), HotFuncSample, ProfileTotalSample,
                       Metric, OS);
 
   return 0;
 }