[PATCH] D20648: Analysis pass to access profile summary info

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri May 27 15:21:34 PDT 2016


vsk added a comment.

Sorry for the delay. I have some preliminary comments, I'll take a closer look in a second.


================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:34
@@ +33,3 @@
+static cl::opt<int> ColdCountPercentile(
+    "cold-count-percentile", cl::Hidden, cl::init(999999), cl::ZeroOrMore,
+    cl::desc("A count is cold if it is below the minimum count"
----------------
I'm a bit confused as to why this is higher than the percentile for hot functions. Sorry if this is a basic question! Could you explain this a bit?

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:44
@@ +43,3 @@
+  };
+  auto It = std::find_if(DS.begin(), DS.end(), FindPercentile);
+  // The required percentile has to match one of the percentiles in the
----------------
Why not a lower_bound? That should be faster, and it adds a bit of flexibility, I think.

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:47
@@ +46,3 @@
+  // detailed summary.
+  assert(It != DS.end());
+  return It->MinCount;
----------------
Could you add an error string to this assert?


http://reviews.llvm.org/D20648





More information about the llvm-commits mailing list