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

[Easwaran Raman via llvm-commits]

eraman added inline comments.

================
Comment at: include/llvm/ProfileData/ProfileCommon.h:32
@@ -30,1 +31,3 @@
 struct InstrProfRecord;
+inline const char *getHotSectionPrefix() { return ".hot"; }
+inline const char *getUnlikelySectionPrefix() { return ".unlikely"; }
----------------
These two functions should be elsewhere. Perhaps in TargetLowering ?

================
Comment at: lib/ProfileData/ProfileSummary.cpp:91
@@ +90,3 @@
+
+bool ProfileSummary::isFunctionUnlikely(const Function *F) {
+  if (F->hasFnAttribute(Attribute::Cold)) {
----------------
davidxl wrote:
> 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.
This and isFunctionHot should be made as a member functions of  Function. 

================
Comment at: lib/ProfileData/ProfileSummary.cpp:95
@@ +94,3 @@
+  }
+  if (!hasProfile() || !F->getEntryCount()) {
+    return false;
----------------
davidxl wrote:
> the check of hasProfile is probably redundant.
I think what David says is true (!F->genEntryCount() implies !hasProfile()). 


http://reviews.llvm.org/D17460