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

David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 15:42:49 PST 2016


davidxl 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"; }
----------------
eraman wrote:
> These two functions should be elsewhere. Perhaps in TargetLowering ?
I think it is fine to be here -- it is generic enough and it is profile related.

================
Comment at: lib/ProfileData/ProfileSummary.cpp:91
@@ +90,3 @@
+
+bool ProfileSummary::isFunctionUnlikely(const Function *F) {
+  if (F->hasFnAttribute(Attribute::Cold)) {
----------------
eraman wrote:
> 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. 
I think it is better to keep all/most of the profile related query APIs here in this file.


http://reviews.llvm.org/D17460





More information about the llvm-commits mailing list