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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: include/llvm/Analysis/ProfileSummaryInfo.h:85
@@ +84,3 @@
+  ProfileSummaryAnalysis(ProfileSummaryAnalysis &&Arg) {}
+  ProfileSummaryAnalysis &operator=(const ProfileSummaryAnalysis &RHS) {
+    return *this;
----------------
Are those ctors/ assignment operators needed? The result type needs the rvalue ctors for data moving.

================
Comment at: include/llvm/Analysis/ProfileSummaryInfo.h:92
@@ +91,3 @@
+
+  ProfileSummaryInfo run(Module &M);
+
----------------
Return 'Result'

================
Comment at: include/llvm/Analysis/ProfileSummaryInfo.h:100
@@ +99,3 @@
+/// \brief Printer pass that uses \c ProfileSummaryAnalysis.
+class FunctionHotnessPrinterPass
+    : public PassInfoMixin<FunctionHotnessPrinterPass> {
----------------
Nit : ProfileSummaryPrinterPass?

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:45
@@ +44,3 @@
+  auto It = std::find_if(DS.begin(), DS.end(), FindPercentile);
+  // The reqquired percentile has to match one of the percentiles in the
+  // detailed summary.
----------------
typo.



================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:65
@@ +64,3 @@
+// Returns true if the function is a hot function.
+bool ProfileSummaryInfo::isHotFunction(const Function *F) {
+  computeSummary();
----------------
Should define a enum : SUMMARY_hot, _cold, _unknown (or noprofile)

We probably also need another interface in the future to check body hotness.

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:79
@@ +78,3 @@
+// Returns true if the function is a cold function.
+bool ProfileSummaryInfo::isColdFunction(const Function *F) {
+  computeSummary();
----------------
Merge with above and make it 'getFunctionHotness' ?

================
Comment at: lib/Analysis/ProfileSummaryInfo.cpp:106
@@ +105,3 @@
+
+bool ProfileSummaryInfo::isHotCount(uint64_t C) {
+  if (!HotCountThreshold)
----------------
Document with FIXME that the hotness determination here is different from function hotness logic above.


http://reviews.llvm.org/D20648