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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:59
@@ +58,3 @@
+  // Number of blocks
+  uint32_t NumBlocks;
+
----------------
Add simple summary data such as MaxFunctionCount and MaxBlockCount here.

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:63
@@ +62,3 @@
+  ProfileSummary() : TotalCount(0), NumBlocks(0) {}
+  void AddCount(uint64_t Count);
+  std::vector<ProfileSummaryEntry>
----------------
Make this a private method. Instead add a public method:

addCount(const InstrProfRecord &)

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:63
@@ +62,3 @@
+  ProfileSummary() : TotalCount(0), NumBlocks(0) {}
+  void AddCount(uint64_t Count);
+  std::vector<ProfileSummaryEntry>
----------------
davidxl wrote:
> Make this a private method. Instead add a public method:
> 
> addCount(const InstrProfRecord &)
AddCount --> addCount

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:82
@@ +81,3 @@
+// value is a vector of (Cutoff, MinBlockCount, NumBlocks) triplets.
+std::vector<ProfileSummaryEntry>
+ProfileSummary::GetProfileSummary(std::vector<float> Cutoffs) {
----------------
Make this vector a member of the ProfileSummary class.

The use model (and in the future by InstrProfWriter ) should be like this:

ProfileSummary PS;

for each InstrProfRecord R:
     PS.addCount(R);

PS.computeProfileSummary(cutoffs);

vector<..>& sums = PS.getProfSummary(..);
... dump

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:83
@@ +82,3 @@
+std::vector<ProfileSummaryEntry>
+ProfileSummary::GetProfileSummary(std::vector<float> Cutoffs) {
+  std::vector<ProfileSummaryEntry> Summary;
----------------
getProfileSummary

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:83
@@ +82,3 @@
+std::vector<ProfileSummaryEntry>
+ProfileSummary::GetProfileSummary(std::vector<float> Cutoffs) {
+  std::vector<ProfileSummaryEntry> Summary;
----------------
davidxl wrote:
> getProfileSummary
Use a scaled up integer to represent percentage.  Talk to Cong about introducing a common interface for that (similar to BranchProbability)

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:91
@@ +90,3 @@
+  std::sort(Cutoffs.begin(), Cutoffs.end());
+  uint32_t NumBlocks = 0;
+  bool Done = false;
----------------
use a different name for the local variable (conflict with member variable)

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:360
@@ -276,3 +359,3 @@
     assert(Func.Counts.size() > 0 && "function missing entry counter");
     if (Func.Counts[0] > MaxFunctionCount)
       MaxFunctionCount = Func.Counts[0];
----------------
This should be moved to ProfileSummary

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:362
@@ -278,3 +361,3 @@
       MaxFunctionCount = Func.Counts[0];
 
     for (size_t I = 1, E = Func.Counts.size(); I < E; ++I) {
----------------
PS.addCount(Func);

================
Comment at: tools/llvm-profdata/llvm-profdata.cpp:364
@@ -280,2 +363,3 @@
     for (size_t I = 1, E = Func.Counts.size(); I < E; ++I) {
+      PS.AddCount(Func.Counts[I]);
       if (Func.Counts[I] > MaxBlockCount)
----------------
Remove this


http://reviews.llvm.org/D16005