Tune isHotFunction/isColdFunction

[danielcdh]

Summary

This patch sets function as hot if function's entry count is hot/cold.

[eraman]

Long term we shouldn't be using the current logic (>= 0.3 * MaxFuncCount). Could you just use only isHotCount/isColdCount and run Spec? If the performance doesn't degrade, then we can just kill the 30% or 1% of MaxFunctionCount logic. If there is a regression, we should either live with the current state of things (ie discard this patch), or move the 30%/1% logic into the inliner (and not call isHotFunction/isColdFunction.

[davidxl]

lib/Analysis/ProfileSummaryInfo.cpp

80

It is very likely that isHotCount(...) is a superset of the old predicate which means the old predicate is likely to be redundant here.

102

It is likely that isColdCount(..) predicate is a subset of the old predicate -- thus this change likely does not make any behavior difference. To make the new check effective, Either && should be used or remove the old predicate.

[danielcdh]

Rename functions.

[danielcdh]

ping...

[davidxl]

lib/Analysis/ProfileSummaryInfo.cpp

78–80

Can you remove this change and make the first patch NFC?

164

s/:hot/:hot entry/

166

cold entry

[eraman]

LGTM (after addressing David's comment on separating into two patches)

lib/Analysis/ProfileSummaryInfo.cpp

154

Remove this FIXME since isFunctionEntryHot calls isHotCount.

[danielcdh]

rebase

[davidxl]

lgtm

lib/Analysis/ProfileSummaryInfo.cpp

bool ProfileSummaryInfo::isHotFunction(const Function *F) {
   computeSummary();
   if (!F || !Summary)
     return false;
   auto FunctionCount = F->getEntryCount();
   // FIXME: The heuristic used below for determining hotness is based on
   // preliminary SPEC tuning for inliner. This will eventually be a
   // convenience method that calls isHotCount.
   return (FunctionCount &&
-          FunctionCount.getValue() >=
-              (uint64_t)(0.3 * (double)Summary->getMaxFunctionCount()));
+          (FunctionCount.getValue() >=
+              (uint64_t)(0.3 * (double)Summary->getMaxFunctionCount()) ||
+           isHotCount(FunctionCount.getValue())));
 }
 
 // Returns true if the function is a cold function. If it returns false, it
 // either means it is not cold or it is unknown whether F is cold or not (for
 // example, no profile data is available).
 bool ProfileSummaryInfo::isColdFunction(const Function *F) {
   computeSummary();
   if (!F)
     return false;
   if (F->hasFnAttribute(Attribute::Cold)) {
     return true;
   }
   if (!Summary)
     return false;
   auto FunctionCount = F->getEntryCount();
   // FIXME: The heuristic used below for determining coldness is based on
   // preliminary SPEC tuning for inliner. This will eventually be a
   // convenience method that calls isHotCount.
   return (FunctionCount &&
-          FunctionCount.getValue() <=
-              (uint64_t)(0.01 * (double)Summary->getMaxFunctionCount()));
+          (FunctionCount.getValue() <=
+              (uint64_t)(0.01 * (double)Summary->getMaxFunctionCount()) ||
+           isColdCount(FunctionCount.getValue())));
 }
 
 // Compute the hot and cold thresholds.
 void ProfileSummaryInfo::computeThresholds() {
   if (!Summary)
     computeSummary();
   if (!Summary)
     return;

 }
 
 char ProfileSummaryAnalysis::PassID;
 ProfileSummaryInfo ProfileSummaryAnalysis::run(Module &M,
                                                ModuleAnalysisManager &) {
   return ProfileSummaryInfo(M);
 }
 
 // FIXME: This only tests isHotFunction and isColdFunction and not the
 // isHotCount and isColdCount calls.
 PreservedAnalyses ProfileSummaryPrinterPass::run(Module &M,
                                                  ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);
 
   OS << "Functions in " << M.getName() << " with hot/cold annotations: \n";
   for (auto &F : M) {
     OS << F.getName();
     if (PSI.isHotFunction(&F))
       OS << " :hot ";
     else if (PSI.isColdFunction(&F))
       OS << " :cold ";
     OS << "\n";
   }
   return PreservedAnalyses::all();
 }
 
 char ProfileSummaryInfoWrapperPass::ID = 0;