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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/TargetLoweringObjectFileImpl.cpp:297
@@ -296,1 +296,3 @@
 
+  // TODO: update when API is available to check if a function is hot.
+  if (const Function *F = dyn_cast<Function>(GV)) {
----------------
1) please introduce  placeholder query APIs in include/llvm/ProfileData/ProfileCommon.h for this purpose (can be a static method of ProfileSummary class:

 bool ProfileSummary::isFunctionHot(Function *);
 bool ProfileSummary::isFunctionUnlikely(Function *)

2) using the existence of entry profile count as the criteria is wrong -- the entry count can be 0 or a very low value. The right tuning will need to look at the max Block count of the function and program summary -- so I think we need to leave that part out and wait for Easwaran's tuning work is in

3) Before the summary based work is in, for now, implement a simpler heuristic similar to what is used in the current inliner: 

 a)  check attribute 'hot' of the function (in practice, this may not work well -- after ininling, the out of line copy of the original hot function may become cold)
 b) check attribute 'cold' for the function -- this is more reliable
 c) if the function's has zero bb count/sample, mark it as cold (Inliner's herustics is 0.01* maxFunctionCount) -- but I think we can be more conservative here by checking zero count ones)

In short, the initial heuristic should be conservative in the sense we only filter out those we are really confident with -- mainly cold ones never executed which can go a long way.

4) Define two helper functions to return .hot and .unlikely suffixes instead of using hard coded strings here.

5) We can also introduce an internal option to turn this layout optimization on |off.  By default it can be off. When it is off, the query functions above will simply return 'false'.




http://reviews.llvm.org/D17460