Index: lib/ProfileData/InstrProfReader.cpp =================================================================== --- lib/ProfileData/InstrProfReader.cpp +++ lib/ProfileData/InstrProfReader.cpp @@ -783,13 +783,13 @@ SummaryData->get(Summary::TotalNumFunctions)); return Cur + SummarySize; } else { - // For older version of profile data, we need to compute on the fly: - using namespace IndexedInstrProf; - + // The older versions do not support a profile summary. This just computes + // an empty summary, which will not result in accurate hot/cold detection. + // We would need to call addRecord for all NamedInstrProfRecords to get the + // correct summary. However, this version is old (prior to early 2016) and + // has not been supporting an accurate summary for several years. InstrProfSummaryBuilder Builder(ProfileSummaryBuilder::DefaultCutoffs); - // FIXME: This only computes an empty summary. Need to call addRecord for - // all NamedInstrProfRecords to get the correct summary. - this->Summary = Builder.getSummary(); + Summary = Builder.getSummary(); return Cur; } } Index: lib/Transforms/Instrumentation/PGOInstrumentation.cpp =================================================================== --- lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -985,9 +985,9 @@ public: PGOUseFunc(Function &Func, Module *Modu, std::unordered_multimap &ComdatMembers, - BranchProbabilityInfo *BPI = nullptr, - BlockFrequencyInfo *BFIin = nullptr, bool IsCS = false) - : F(Func), M(Modu), BFI(BFIin), + BranchProbabilityInfo *BPI, BlockFrequencyInfo *BFIin, + ProfileSummaryInfo *PSI, bool IsCS) + : F(Func), M(Modu), BFI(BFIin), PSI(PSI), FuncInfo(Func, ComdatMembers, false, BPI, BFIin, IsCS), FreqAttr(FFA_Normal), IsCS(IsCS) {} @@ -1042,6 +1042,7 @@ Function &F; Module *M; BlockFrequencyInfo *BFI; + ProfileSummaryInfo *PSI; // This member stores the shared information with class PGOGenFunc. FuncPGOInstrumentation FuncInfo; @@ -1079,15 +1080,9 @@ // FIXME: This function should be removed once the functionality in // the inliner is implemented. void markFunctionAttributes(uint64_t EntryCount, uint64_t MaxCount) { - if (ProgramMaxCount == 0) - return; - // Threshold of the hot functions. - const BranchProbability HotFunctionThreshold(1, 100); - // Threshold of the cold functions. - const BranchProbability ColdFunctionThreshold(2, 10000); - if (EntryCount >= HotFunctionThreshold.scale(ProgramMaxCount)) + if (PSI->isHotCount(EntryCount)) FreqAttr = FFA_Hot; - else if (MaxCount <= ColdFunctionThreshold.scale(ProgramMaxCount)) + else if (PSI->isColdCount(MaxCount)) FreqAttr = FFA_Cold; } }; @@ -1596,7 +1591,8 @@ static bool annotateAllFunctions( Module &M, StringRef ProfileFileName, StringRef ProfileRemappingFileName, function_ref LookupBPI, - function_ref LookupBFI, bool IsCS) { + function_ref LookupBFI, + ProfileSummaryInfo *PSI, bool IsCS) { LLVM_DEBUG(dbgs() << "Read in profile counters: "); auto &Ctx = M.getContext(); // Read the counter array from file. @@ -1627,6 +1623,13 @@ return false; } + // Add the profile summary (read from the header of the indexed summary) here + // so that we can use it below when reading counters (which checks if the + // function should be marked with a cold or inlinehint attribute). + M.setProfileSummary(PGOReader->getSummary(IsCS).getMD(M.getContext()), + IsCS ? ProfileSummary::PSK_CSInstr + : ProfileSummary::PSK_Instr); + std::unordered_multimap ComdatMembers; collectComdatMembers(M, ComdatMembers); std::vector HotFunctions; @@ -1639,7 +1642,7 @@ // Split indirectbr critical edges here before computing the MST rather than // later in getInstrBB() to avoid invalidating it. SplitIndirectBrCriticalEdges(F, BPI, BFI); - PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI, IsCS); + PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI, PSI, IsCS); bool AllZeros = false; if (!Func.readCounters(PGOReader.get(), AllZeros)) continue; @@ -1687,9 +1690,6 @@ } } } - M.setProfileSummary(PGOReader->getSummary(IsCS).getMD(M.getContext()), - IsCS ? ProfileSummary::PSK_CSInstr - : ProfileSummary::PSK_Instr); // Set function hotness attribute from the profile. // We have to apply these attributes at the end because their presence @@ -1731,8 +1731,10 @@ return &FAM.getResult(F); }; + auto *PSI = &AM.getResult(M); + if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName, - LookupBPI, LookupBFI, IsCS)) + LookupBPI, LookupBFI, PSI, IsCS)) return PreservedAnalyses::all(); return PreservedAnalyses::none(); @@ -1749,7 +1751,8 @@ return &this->getAnalysis(F).getBFI(); }; - return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI, + auto *PSI = &getAnalysis().getPSI(); + return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI, PSI, IsCS); } Index: test/Transforms/PGOProfile/Inputs/func_entry.proftext =================================================================== --- test/Transforms/PGOProfile/Inputs/func_entry.proftext +++ test/Transforms/PGOProfile/Inputs/func_entry.proftext @@ -1,17 +1,25 @@ # IR level Instrumentation Flag :ir -foo +hot # Func Hash: 12884901887 # Num Counters: 1 # Counter Values: -9999 +9000 -bar +cold # Func Hash: 12884901887 # Num Counters: 1 # Counter Values: -0 +10 + +med +# Func Hash: +12884901887 +# Num Counters: +1 +# Counter Values: +50 Index: test/Transforms/PGOProfile/func_entry.ll =================================================================== --- test/Transforms/PGOProfile/func_entry.ll +++ test/Transforms/PGOProfile/func_entry.ll @@ -6,8 +6,9 @@ @s = common dso_local local_unnamed_addr global i32 0, align 4 -define void @bar() { -; CHECK-LABEL: @bar +define void @cold() { +; CHECK-LABEL: @cold() +; CHECK-SAME: #[[COLD_ATTR:[0-1]+]] ; CHECK-SAME: !prof ![[FUNC_ENTRY_COUNT_ZERO:[0-9]+]] entry: @@ -15,8 +16,9 @@ ret void } -define void @foo() { -; CHECK-LABEL: @foo +define void @hot() { +; CHECK-LABEL: @hot() +; CHECK-SAME: #[[HOT_ATTR:[0-1]+]] ; CHECK-SAME: !prof ![[FUNC_ENTRY_COUNT_NON_ZERO:[0-9]+]] entry: %0 = load i32, i32* @s, align 4 @@ -25,5 +27,18 @@ ret void } -; CHECK-DAG: ![[FUNC_ENTRY_COUNT_ZERO]] = !{!"function_entry_count", i64 0} -; CHECK-DAG: ![[FUNC_ENTRY_COUNT_NON_ZERO]] = !{!"function_entry_count", i64 9999} +define void @med() { +; CHECK-LABEL: @med +; CHECK-NOT: # +; CHECK-SAME: !prof ![[FUNC_ENTRY_COUNT_MED:[0-9]+]] + +entry: + store i32 1, i32* @s, align 4 + ret void +} + +; CHECK-DAG: attributes #[[COLD_ATTR]] = { cold } +; CHECK-DAG: attributes #[[HOT_ATTR]] = { inlinehint } +; CHECK-DAG: ![[FUNC_ENTRY_COUNT_ZERO]] = !{!"function_entry_count", i64 10} +; CHECK-DAG: ![[FUNC_ENTRY_COUNT_NON_ZERO]] = !{!"function_entry_count", i64 9000} +; CHECK-DAG: ![[FUNC_ENTRY_COUNT_MED]] = !{!"function_entry_count", i64 50}