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

79

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

98

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

77

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

154–155

s/:hot/:hot entry/

155–158

cold entry

[eraman]

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

lib/Analysis/ProfileSummaryInfo.cpp

147–148

Remove this FIXME since isFunctionEntryHot calls isHotCount.

[danielcdh]

rebase

[davidxl]

lgtm

lib/Analysis/ProfileSummaryInfo.cpp

 bool ProfileSummaryInfo::isFunctionEntryHot(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()));
+  return FunctionCount && isHotCount(FunctionCount.getValue());
 }
 
 /// Returns true if the function's entry is a cold. If it returns false, it
 /// either means it is not cold or it is unknown whether it is cold or not (for
 /// example, no profile data is available).
 bool ProfileSummaryInfo::isFunctionEntryCold(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()));
+  return FunctionCount && isColdCount(FunctionCount.getValue());
 }
 
 /// Compute the hot and cold thresholds.
 void ProfileSummaryInfo::computeThresholds() {
   if (!Summary)
     computeSummary();
   if (!Summary)
     return;
   auto &DetailedSummary = Summary->getDetailedSummary();
   HotCountThreshold =
bool ProfileSummaryInfoWrapperPass::doFinalization(Module &M) {
   return false;
 }
 
 char ProfileSummaryAnalysis::PassID;
 ProfileSummaryInfo ProfileSummaryAnalysis::run(Module &M,
                                                ModuleAnalysisManager &) {
   return ProfileSummaryInfo(M);
 }
 
 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.isFunctionEntryHot(&F))
       OS << " :hot entry ";
     else if (PSI.isFunctionEntryCold(&F))
       OS << " :cold entry ";
     OS << "\n";
   }
   return PreservedAnalyses::all();
 }
 
 char ProfileSummaryInfoWrapperPass::ID = 0;