[PATCH] D17460: Add prefix based function layout when profile is available.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: include/llvm/ProfileData/ProfileCommon.h:70
@@ -68,1 +69,3 @@
   static const int Scale = 1000000;
+  static bool hasProfile();
+  static bool isFunctionHot(const Function *F);
----------------
Document this method -- what does it do?

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:248
@@ -246,1 +247,3 @@
 
+static StringRef getHotSectionPrefix() { return ".hot"; }
+
----------------
Move these two functions into ProfileSummary.h (as a standalone function in llvm:: namespace)

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:252
@@ +251,3 @@
+
+static cl::opt<bool> UseFunctionHotnessPrefix(
+    "use-function-hotness-prefix",
----------------
Better name it as: GroupFunctionsByHotness

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:255
@@ +254,3 @@
+    llvm::cl::desc("Partition hot/cold functions by sections prefix"),
+    cl::init(true));
+
----------------
I think the initial value should be false. To turn it on by default, more data is needed (and announced more widely).

================
Comment at: lib/ProfileData/ProfileSummary.cpp:81
@@ +80,3 @@
+bool ProfileSummary::hasProfile() {
+  // TODO: update when summary data is stored in module's metadata.
+  return false;
----------------
use FIXME

================
Comment at: lib/ProfileData/ProfileSummary.cpp:87
@@ +86,3 @@
+bool ProfileSummary::isFunctionHot(const Function *F) {
+  // TODO: update when summary data is stored in module's metadata.
+  return false;
----------------
FIXME

================
Comment at: lib/ProfileData/ProfileSummary.cpp:98
@@ +97,3 @@
+  }
+  // TODO: update when summary data is stored in module's metadata.
+  return (*F->getEntryCount()) == 0;
----------------
FIXME


http://reviews.llvm.org/D17460