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

Easwaran Raman via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 2 11:03:35 PDT 2016


eraman marked 8 inline comments as done and an inline comment as not done.

================
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
----------------
vsk wrote:
> eraman wrote:
> > vsk wrote:
> > > Why not a lower_bound? That should be faster, and it adds a bit of flexibility, I think.
> > If we want the flexibility of not specifying an exact percentile, won't nearest entry (minimum difference) be better than lower_bound? 
> ISTM that breaks the function's contract. It would no longer always return the minimum execution count for a given percentile: it could return something greater.
You're right. I've changed the code to use lower_bound

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:47
@@ +46,3 @@
+  // detailed summary.
+  assert(It != DS.end());
+  return It->MinCount;
----------------
vsk wrote:
> Could you add an error string to this assert?
I've changed to report_fatal_error as suggested by Sean

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:139
@@ +138,3 @@
+  return ProfileSummaryInfo(M);
+}
+
----------------
Sorry, I don't understand this comment. Irrespective of how we set the function entry count, the issue is we are not testing the isHotCount and isColdCount code.


http://reviews.llvm.org/D20648





More information about the llvm-commits mailing list