Skip to content

Commit 4ffad1f

Browse files
committed
[SampleFDO] Add PromotedInsns to prevent repeated ICP.
In https://reviews.llvm.org/rG5fb65c02ca5e91e7e1a00e0efdb8edc899f3e4b9, We use 0 count value profile to memorize which target has been promoted and prevent repeated ICP for the same target, so we delete PromotedInsns. However, I found the implementation in the patch has some shortcomings to be fixed otherwise there will still be repeated ICP. So I add PromotedInsns back temorarily. Will remove it after I get a thorough fix.
1 parent 1a368ae commit 4ffad1f

File tree

1 file changed

+15
-3
lines changed

1 file changed

+15
-3
lines changed

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,8 @@ class SampleProfileLoader final : public SampleProfileLoaderBaseImpl {
363363
// Attempt to promote indirect call and also inline the promoted call
364364
bool tryPromoteAndInlineCandidate(
365365
Function &F, InlineCandidate &Candidate, uint64_t SumOrigin,
366-
uint64_t &Sum, SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
366+
uint64_t &Sum, DenseSet<Instruction *> &PromotedInsns,
367+
SmallVector<CallBase *, 8> *InlinedCallSites = nullptr);
367368
bool inlineHotFunctions(Function &F,
368369
DenseSet<GlobalValue::GUID> &InlinedGUIDs);
369370
InlineCost shouldInlineCandidate(InlineCandidate &Candidate);
@@ -766,10 +767,12 @@ updateIDTMetaData(Instruction &Inst,
766767
/// \param F Caller function.
767768
/// \param Candidate ICP and inline candidate.
768769
/// \param Sum Sum of target counts for indirect call.
770+
/// \param PromotedInsns Map to keep track of indirect call already processed.
769771
/// \param InlinedCallSite Output vector for new call sites exposed after
770772
/// inlining.
771773
bool SampleProfileLoader::tryPromoteAndInlineCandidate(
772774
Function &F, InlineCandidate &Candidate, uint64_t SumOrigin, uint64_t &Sum,
775+
DenseSet<Instruction *> &PromotedInsns,
773776
SmallVector<CallBase *, 8> *InlinedCallSite) {
774777
auto CalleeFunctionName = Candidate.CalleeSamples->getFuncName();
775778
auto R = SymbolMap.find(CalleeFunctionName);
@@ -808,6 +811,7 @@ bool SampleProfileLoader::tryPromoteAndInlineCandidate(
808811
// be prorated so that the it will reflect the real callsite counts.
809812
setProbeDistributionFactor(CI, Candidate.CallsiteDistribution * Sum /
810813
SumOrigin);
814+
PromotedInsns.insert(Candidate.CallInstr);
811815
Candidate.CallInstr = DI;
812816
if (isa<CallInst>(DI) || isa<InvokeInst>(DI)) {
813817
bool Inlined = tryInlineCandidate(Candidate, InlinedCallSite);
@@ -880,6 +884,8 @@ void SampleProfileLoader::emitOptimizationRemarksForInlineCandidates(
880884
/// \returns True if there is any inline happened.
881885
bool SampleProfileLoader::inlineHotFunctions(
882886
Function &F, DenseSet<GlobalValue::GUID> &InlinedGUIDs) {
887+
DenseSet<Instruction *> PromotedInsns;
888+
883889
// ProfAccForSymsInList is used in callsiteIsHot. The assertion makes sure
884890
// Profile symbol list is ignored when profile-sample-accurate is on.
885891
assert((!ProfAccForSymsInList ||
@@ -933,6 +939,8 @@ bool SampleProfileLoader::inlineHotFunctions(
933939
if (CalledFunction == &F)
934940
continue;
935941
if (I->isIndirectCall()) {
942+
if (PromotedInsns.count(I))
943+
continue;
936944
uint64_t Sum;
937945
for (const auto *FS : findIndirectCallFunctionSamples(*I, Sum)) {
938946
uint64_t SumOrigin = Sum;
@@ -945,7 +953,8 @@ bool SampleProfileLoader::inlineHotFunctions(
945953
continue;
946954

947955
Candidate = {I, FS, FS->getEntrySamples(), 1.0};
948-
if (tryPromoteAndInlineCandidate(F, Candidate, SumOrigin, Sum)) {
956+
if (tryPromoteAndInlineCandidate(F, Candidate, SumOrigin, Sum,
957+
PromotedInsns)) {
949958
LocalNotInlinedCallSites.erase(I);
950959
LocalChanged = true;
951960
}
@@ -1154,6 +1163,7 @@ SampleProfileLoader::shouldInlineCandidate(InlineCandidate &Candidate) {
11541163

11551164
bool SampleProfileLoader::inlineHotFunctionsWithPriority(
11561165
Function &F, DenseSet<GlobalValue::GUID> &InlinedGUIDs) {
1166+
DenseSet<Instruction *> PromotedInsns;
11571167
assert(ProfileIsCS && "Prioritiy based inliner only works with CSSPGO now");
11581168

11591169
// ProfAccForSymsInList is used in callsiteIsHot. The assertion makes sure
@@ -1202,6 +1212,8 @@ bool SampleProfileLoader::inlineHotFunctionsWithPriority(
12021212
if (CalledFunction == &F)
12031213
continue;
12041214
if (I->isIndirectCall()) {
1215+
if (PromotedInsns.count(I))
1216+
continue;
12051217
uint64_t Sum;
12061218
auto CalleeSamples = findIndirectCallFunctionSamples(*I, Sum);
12071219
uint64_t SumOrigin = Sum;
@@ -1236,7 +1248,7 @@ bool SampleProfileLoader::inlineHotFunctionsWithPriority(
12361248
Candidate = {I, FS, EntryCountDistributed,
12371249
Candidate.CallsiteDistribution};
12381250
if (tryPromoteAndInlineCandidate(F, Candidate, SumOrigin, Sum,
1239-
&InlinedCallSites)) {
1251+
PromotedInsns, &InlinedCallSites)) {
12401252
for (auto *CB : InlinedCallSites) {
12411253
if (getInlineCandidate(&NewCandidate, CB))
12421254
CQueue.emplace(NewCandidate);

0 commit comments

Comments
 (0)