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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:80
@@ -77,1 +79,3 @@
 
+bool ProfileSummary::hasProfile() {
+  // FIXME: update when summary data is stored in module's metadata.
----------------
If this takes Function * as an argument, it can be a wrapper of getEntryCount call

================
Comment at: lib/ProfileData/ProfileSummary.cpp:91
@@ +90,3 @@
+
+bool ProfileSummary::isFunctionUnlikely(const Function *F) {
+  if (F->hasFnAttribute(Attribute::Cold)) {
----------------
I suggest changing this interface's name to be 'isFunctionRarelyCalled'. It is different from 'isFunctionCold' which should be checking the internal block count. These two interfaces are for different purposes. The later can be used to guide decisions such as optimize for size etc, while the former can be used for layout decision to reduce itlb misses.

Similarly, isFunctionHot does not mean it is frequently called either -- so the interface need to be split up too.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:95
@@ +94,3 @@
+  }
+  if (!hasProfile() || !F->getEntryCount()) {
+    return false;
----------------
the check of hasProfile is probably redundant.


http://reviews.llvm.org/D17460