[llvm-profdata] --hot-func-list: fix some style issues in D81800

[hoyFB]

Thanks for fixing these issues!

[wenlei]

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

[MaskRay]

In D82500#2112631, @wenlei wrote:

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

I feel so. For longer type names like std::pair<..., ...> or std::map<...>::iterator people don't typically write the underlying type. I agree with such practice. However, in this file, when we can write a single identifier instead auto , I think we should stick with the rule.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

Thanks. Corrected the title.

[wenlei]

In D82500#2112752, @MaskRay wrote:

In D82500#2112631, @wenlei wrote:

For the use of auto, is there extra convention except what's said here? https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It feels the style convention is a bit loose though, except those enforced by linter.

I feel so. For longer type names like std::pair<..., ...> or std::map<...>::iterator people don't typically write the underlying type. I agree with such practice. However, in this file, when we can write a single identifier instead auto , I think we should stick with the rule.

Ok, I'm not aware of the single identifier rule, but makes sense to me. Thanks for the touch up.

(FYI this is the original diff: https://reviews.llvm.org/D81800)

Thanks. Corrected the title.

llvm/include/llvm/ProfileData/SampleProf.h

public:
   const CallsiteSampleMap &getCallsiteSamples() const {
     return CallsiteSamples;
   }
 
   /// Return the maximum of sample counts in a function body including functions
   /// inlined in it.
   uint64_t getMaxCountInside() const {
     uint64_t MaxCount = 0;
-    for (const auto &L : getBodySamples()) {
+    for (const auto &L : getBodySamples())
       MaxCount = std::max(MaxCount, L.second.getSamples());
-    }
-    for (const auto &C : getCallsiteSamples()) {
-      for (const auto &F : C.second) {
+    for (const auto &C : getCallsiteSamples())
+      for (const FunctionSamplesMap::value_type &F : C.second)
         MaxCount = std::max(MaxCount, F.second.getMaxCountInside());
-      }
-    }
     return MaxCount;
   }
 
   /// Merge the samples in \p Other into this one.
   /// Optionally scale samples by \p Weight.
   sampleprof_error merge(const FunctionSamples &Other, uint64_t Weight = 1) {
     sampleprof_error Result = sampleprof_error::success;
     Name = Other.getName();

llvm/test/tools/llvm-profdata/sample-hot-func-list.test

-; RUN: llvm-profdata show --sample --hot-func-list %S/Inputs/sample-hot-func-list.proftext | FileCheck %s
-; CHECK: 8 out of 10 functions with profile (80.00%) are considered hot functions (max sample >= 470).
-; CHECK-NEXT: 355251 out of 356026 profile counts (99.78%) are from hot functions.
-; CHECK-NEXT: Total sample (%)        Max sample        Entry sample    Function name
-; CHECK-NEXT: 184019 (51.69%)         2300              534             main
-; CHECK-NEXT: 97401 (27.36%)          10640             3035            Func3
-; CHECK-NEXT: 20305 (5.70%)           1000              1000            _Z3bazi
-; CHECK-NEXT: 20301 (5.70%)           1437              1437            _Z3bari
-; CHECK-NEXT: 17043 (4.79%)           3105              1594            Func2
-; CHECK-NEXT: 7711 (2.17%)            610               610             _Z3fooi
-; CHECK-NEXT: 6948 (1.95%)            3507              470             Func5
-; CHECK-NEXT: 1523 (0.43%)            563               169             Func1
+; RUN: llvm-profdata show --sample --hot-func-list %S/Inputs/sample-hot-func-list.proftext | FileCheck %s --match-full-lines --strict-whitespace
+
+;      CHECK:8 out of 10 functions with profile (80.00%) are considered hot functions (max sample >= 470).
+; CHECK-NEXT:355251 out of 356026 profile counts (99.78%) are from hot functions.
+; CHECK-NEXT: Total sample (%)       Max sample        Entry sample    Function name
+; CHECK-NEXT: 184019 (51.69%)        2300              534             main
+; CHECK-NEXT: 97401 (27.36%)         10640             3035            Func3
+; CHECK-NEXT: 20305 (5.70%)          1000              1000            _Z3bazi
+; CHECK-NEXT: 20301 (5.70%)          1437              1437            _Z3bari
+; CHECK-NEXT: 17043 (4.79%)          3105              1594            Func2
+; CHECK-NEXT: 7711 (2.17%)           610               610             _Z3fooi
+; CHECK-NEXT: 6948 (1.95%)           3507              470             Func5
+; CHECK-NEXT: 1523 (0.43%)           563               169             Func1

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

static void showSectionInfo(sampleprof::SampleProfileReader *Reader,
   if (!Reader->dumpSectionInfo(OS)) {
     WithColor::warning() << "-show-sec-info-only is only supported for "
                          << "sample profile in extbinary format and is "
                          << "ignored for other formats.\n";
     return;
   }
 }
 
+namespace {
 struct HotFuncInfo {
   StringRef FuncName;
   uint64_t TotalCount;
   double TotalCountPercent;
   uint64_t MaxCount;
   uint64_t EntryCount;
 
   HotFuncInfo()
       : FuncName(), TotalCount(0), TotalCountPercent(0.0f), MaxCount(0),
         EntryCount(0) {}
 
   HotFuncInfo(StringRef FN, uint64_t TS, double TSP, uint64_t MS, uint64_t ES)
       : FuncName(FN), TotalCount(TS), TotalCountPercent(TSP), MaxCount(MS),
         EntryCount(ES) {}
 };
+} // namespace
 
 // Print out detailed information about hot functions in PrintValues vector.
 // Users specify titles and offset of every columns through ColumnTitle and
 // ColumnOffset. The size of ColumnTitle and ColumnOffset need to be the same
 // and at least 4. Besides, users can optionally give a HotFuncMetric string to
 // print out or let it be an empty string.
 static void dumpHotFunctionList(const std::vector<std::string> &ColumnTitle,
                                 const std::vector<int> &ColumnOffset,
FOS << HotProfCount << " out of " << TotalProfCount << " profile counts ("
       << format("%.2f%%", TotalProfPercent) << ") are from hot functions.\n";
 
   for (size_t I = 0; I < ColumnTitle.size(); ++I) {
     FOS.PadToColumn(ColumnOffset[I]);
     FOS << ColumnTitle[I];
   }
   FOS << "\n";
 
-  for (const auto &R : PrintValues) {
+  for (const HotFuncInfo &R : PrintValues) {
     FOS.PadToColumn(ColumnOffset[0]);
     FOS << R.TotalCount << " (" << format("%.2f%%", R.TotalCountPercent) << ")";
     FOS.PadToColumn(ColumnOffset[1]);
     FOS << R.MaxCount;
     FOS.PadToColumn(ColumnOffset[2]);
     FOS << R.EntryCount;
     FOS.PadToColumn(ColumnOffset[3]);
     FOS << R.FuncName << "\n";
   }
   return;
 }
 
 static int
 showHotFunctionList(const StringMap<sampleprof::FunctionSamples> &Profiles,
                     ProfileSummary &PS, raw_fd_ostream &OS) {
   using namespace sampleprof;
 
   const uint32_t HotFuncCutoff = 990000;
   auto &SummaryVector = PS.getDetailedSummary();
   uint64_t MinCountThreshold = 0;
-  for (const auto &SummaryEntry : SummaryVector) {
+  for (const ProfileSummaryEntry &SummaryEntry : SummaryVector) {
     if (SummaryEntry.Cutoff == HotFuncCutoff) {
       MinCountThreshold = SummaryEntry.MinCount;
       break;
     }
   }
   assert(MinCountThreshold != 0);
 
   // Traverse all functions in the profile and keep only hot functions.
   // The following loop also calculates the sum of total samples of all
   // functions.
   std::multimap<uint64_t, std::pair<const FunctionSamples *, const uint64_t>,
                 std::greater<uint64_t>>
       HotFunc;
   uint64_t ProfileTotalSample = 0;
   uint64_t HotFuncSample = 0;
   uint64_t HotFuncCount = 0;
   uint64_t MaxCount = 0;
   for (const auto &I : Profiles) {
-    const auto &FuncProf = I.second;
+    const FunctionSamples &FuncProf = I.second;
     ProfileTotalSample += FuncProf.getTotalSamples();
     MaxCount = FuncProf.getMaxCountInside();
 
     // MinCountThreshold is a block/line threshold computed for a given cutoff.
     // We intentionally compare the maximum sample count in a function with this
     // threshold to get an approximate threshold for hot functions.
     if (MaxCount >= MinCountThreshold) {
       HotFunc.emplace(FuncProf.getTotalSamples(),
                       std::make_pair(&(I.second), MaxCount));
       HotFuncSample += FuncProf.getTotalSamples();
       ++HotFuncCount;
     }
   }
 
   std::vector<std::string> ColumnTitle{"Total sample (%)", "Max sample",
                                        "Entry sample", "Function name"};
   std::vector<int> ColumnOffset{0, 24, 42, 58};
   std::string Metric =
       std::string("max sample >= ") + std::to_string(MinCountThreshold);
   std::vector<HotFuncInfo> PrintValues;
   for (const auto &FuncPair : HotFunc) {
-    const auto &FuncPtr = FuncPair.second.first;
+    const FunctionSamples &Func = *FuncPair.second.first;
     double TotalSamplePercent =
         (ProfileTotalSample > 0)
-            ? (FuncPtr->getTotalSamples() * 100.0) / ProfileTotalSample
+            ? (Func.getTotalSamples() * 100.0) / ProfileTotalSample
             : 0;
     PrintValues.emplace_back(HotFuncInfo(
-        FuncPtr->getFuncName(), FuncPtr->getTotalSamples(), TotalSamplePercent,
-        FuncPair.second.second, FuncPtr->getEntrySamples()));
+        Func.getFuncName(), Func.getTotalSamples(), TotalSamplePercent,
+        FuncPair.second.second, Func.getEntrySamples()));
   }
   dumpHotFunctionList(ColumnTitle, ColumnOffset, PrintValues, HotFuncCount,
                       Profiles.size(), HotFuncSample, ProfileTotalSample,
                       Metric, OS);
 
   return 0;
 }