[PATCH] D16005: Display detailed profile summary in llvm-profdata tool

[Vedant Kumar via llvm-commits]

vsk added a comment.

Thanks eraman! Comments inline --


================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:45
@@ +44,3 @@
+// counts). The N in the above Top-N is the NumBlocks of the triplet and the
+// smallest count in the Top-N is the MinBlockCount.
+struct ProfileSummaryEntry {
----------------
Nit: I have a hard time parsing this comment. Wdyt of omitting this paragraph and opting for shorter comments on the fields instead? I suggest;

  float Cutoff;  //< The execution count percentile, [0, 100].
  uint64_t MinExecuteCount;  //< Minimum execution count for this percentile.
  uint64_t NumBlocks; //< Number of blocks in this percentile.

Or something along those lines, I don't think my wording there is perfect :).

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:74
@@ +73,3 @@
+  NumBlocks++;
+  if (CountFrequencies.count(Count))
+    CountFrequencies[Count] += 1;
----------------
I believe this entire conditional can be safely replaced with `++CountFrequencies[Count];`.

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:109
@@ +108,3 @@
+      if (Iter == End) {
+        Done = true;
+        break;
----------------
You can avoid this construct by inverting the order of your loops, i.e iterate over the cutoffs then the histogram entries. Briefly;

  PSE NextEntry;
  for (float Cutoff : Cutoffs)
    NextEntry.Cutoff = Cutoff;
    std::tie(NextEntry.MinExecuteCount, NextEntry.NumBlocks) = AccumulateCountsTo(Cutoff);


http://reviews.llvm.org/D16005