[llvm] r372189 - [PGO] Change hardcoded thresholds for cold/inlinehint to use summary
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 17 16:12:14 PDT 2019
Author: tejohnson
Date: Tue Sep 17 16:12:13 2019
New Revision: 372189
URL: http://llvm.org/viewvc/llvm-project?rev=372189&view=rev
Log:
[PGO] Change hardcoded thresholds for cold/inlinehint to use summary
Summary:
The PGO counter reading will add cold and inlinehint (hot) attributes
to functions that are very cold or hot. This was using hardcoded
thresholds, instead of the profile summary cutoffs which are used in
other hot/cold detection and are more dynamic and adaptable. Switch
to using the summary-based cold/hot detection.
The hardcoded limits were causing some code that had a medium level of
hotness (per the summary) to be incorrectly marked with a cold
attribute, blocking inlining.
Reviewers: davidxl
Subscribers: llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D67673
Modified:
llvm/trunk/lib/ProfileData/InstrProfReader.cpp
llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/trunk/test/Transforms/PGOProfile/Inputs/func_entry.proftext
llvm/trunk/test/Transforms/PGOProfile/func_entry.ll
Modified: llvm/trunk/lib/ProfileData/InstrProfReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProfReader.cpp?rev=372189&r1=372188&r2=372189&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/InstrProfReader.cpp (original)
+++ llvm/trunk/lib/ProfileData/InstrProfReader.cpp Tue Sep 17 16:12:13 2019
@@ -783,13 +783,13 @@ IndexedInstrProfReader::readSummary(Inde
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;
}
}
Modified: llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp?rev=372189&r1=372188&r2=372189&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp Tue Sep 17 16:12:13 2019
@@ -985,9 +985,9 @@ class PGOUseFunc {
public:
PGOUseFunc(Function &Func, Module *Modu,
std::unordered_multimap<Comdat *, GlobalValue *> &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 @@ private:
Function &F;
Module *M;
BlockFrequencyInfo *BFI;
+ ProfileSummaryInfo *PSI;
// This member stores the shared information with class PGOGenFunc.
FuncPGOInstrumentation<PGOUseEdge, UseBBInfo> FuncInfo;
@@ -1079,15 +1080,9 @@ private:
// 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 @@ PreservedAnalyses PGOInstrumentationGen:
static bool annotateAllFunctions(
Module &M, StringRef ProfileFileName, StringRef ProfileRemappingFileName,
function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
- function_ref<BlockFrequencyInfo *(Function &)> LookupBFI, bool IsCS) {
+ function_ref<BlockFrequencyInfo *(Function &)> 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 @@ static bool annotateAllFunctions(
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<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
std::vector<Function *> HotFunctions;
@@ -1639,7 +1642,7 @@ static bool annotateAllFunctions(
// 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 @@ static bool annotateAllFunctions(
}
}
}
- 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 @@ PreservedAnalyses PGOInstrumentationUse:
return &FAM.getResult<BlockFrequencyAnalysis>(F);
};
+ auto *PSI = &AM.getResult<ProfileSummaryAnalysis>(M);
+
if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName,
- LookupBPI, LookupBFI, IsCS))
+ LookupBPI, LookupBFI, PSI, IsCS))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
@@ -1749,7 +1751,8 @@ bool PGOInstrumentationUseLegacyPass::ru
return &this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
};
- return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI,
+ auto *PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+ return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI, PSI,
IsCS);
}
Modified: llvm/trunk/test/Transforms/PGOProfile/Inputs/func_entry.proftext
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/Inputs/func_entry.proftext?rev=372189&r1=372188&r2=372189&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/Inputs/func_entry.proftext (original)
+++ llvm/trunk/test/Transforms/PGOProfile/Inputs/func_entry.proftext Tue Sep 17 16:12:13 2019
@@ -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
Modified: llvm/trunk/test/Transforms/PGOProfile/func_entry.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/func_entry.ll?rev=372189&r1=372188&r2=372189&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/func_entry.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/func_entry.ll Tue Sep 17 16:12:13 2019
@@ -6,8 +6,9 @@ target triple = "x86_64-unknown-linux-gn
@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 @@ entry:
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 @@ entry:
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}
More information about the llvm-commits
mailing list