Skip to content

Commit 1c05c06

Browse files
AndyAyersMSAndyAyersMS
authored
JIT: profile checking through loop opts (#99367)
Keep profile checks enabled until after we have finished running the loop optmizations (recall this is currently just checking for edge likelihood consistency). Fix various maintenance issues to make this possible. Most are straightforward, but a few are not: * Whenever we create a new BBJ_COND we have to figure out the right likelihoods. If we're copying an existing one (say loop inversion) we currently duplicate the likelihoods. This is a choice, and it may not accurately represent what happends, but we have no better information. * If we invent new branching structures we need to put in reasonable likelihoods. For cloning we assume flowing to the cold loop is unlikely but can happen. Block weights and edge likelihoods are not yet consistent. The plan is to get all the edge likelihoods "correct" and self-consistent, and then start rectifying edge likelihoods and block weights. Contributes to #93020.
1 parent 9ab8047 commit 1c05c06

File tree

8 files changed

+197
-49
lines changed

8 files changed

+197
-49
lines changed

‎src/coreclr/jit/block.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ unsigned SsaStressHashHelper()
6969
#endif
7070

7171
//------------------------------------------------------------------------
72-
// setLikelihood: set the likelihood of aflow edge
72+
// setLikelihood: set the likelihood of a flow edge
7373
//
7474
// Arguments:
7575
// likelihood -- value in range [0.0, 1.0] indicating how likely
@@ -86,6 +86,40 @@ void FlowEdge::setLikelihood(weight_t likelihood)
8686
m_likelihood);
8787
}
8888

89+
//------------------------------------------------------------------------
90+
// addLikelihood: adjust the likelihood of a flow edge
91+
//
92+
// Arguments:
93+
// addedLikelihood -- value in range [-likelihood, 1.0 - likelihood]
94+
// to add to current likelihood.
95+
//
96+
void FlowEdge::addLikelihood(weight_t addedLikelihood)
97+
{
98+
assert(m_likelihoodSet);
99+
100+
weight_t newLikelihood = m_likelihood + addedLikelihood;
101+
102+
// Tolerate slight overflow or underflow
103+
//
104+
const weight_t eps = 0.0001;
105+
106+
if ((newLikelihood < 0) && (newLikelihood > -eps))
107+
{
108+
newLikelihood = 0.0;
109+
}
110+
else if ((newLikelihood > 1) && (newLikelihood < 1 + eps))
111+
{
112+
newLikelihood = 1.0;
113+
}
114+
115+
assert(newLikelihood >= 0.0);
116+
assert(newLikelihood <= 1.0);
117+
m_likelihood = newLikelihood;
118+
119+
JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
120+
m_likelihood);
121+
}
122+
89123
//------------------------------------------------------------------------
90124
// AllSuccessorEnumerator: Construct an instance of the enumerator.
91125
//

‎src/coreclr/jit/block.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,7 @@ struct FlowEdge
671671
}
672672

673673
void setLikelihood(weight_t likelihood);
674+
void addLikelihood(weight_t addedLikelihod);
674675

675676
void clearLikelihood()
676677
{

‎src/coreclr/jit/compiler.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4610,11 +4610,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
46104610
//
46114611
DoPhase(this, PHASE_MORPH_ADD_INTERNAL, &Compiler::fgAddInternal);
46124612

4613-
// Disable profile checks now.
4614-
// Over time we will move this further and further back in the phase list, as we fix issues.
4615-
//
4616-
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
4617-
46184613
// Remove empty try regions
46194614
//
46204615
DoPhase(this, PHASE_EMPTY_TRY, &Compiler::fgRemoveEmptyTry);
@@ -4855,6 +4850,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
48554850
DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators);
48564851
}
48574852

4853+
// Disable profile checks now.
4854+
// Over time we will move this further and further back in the phase list, as we fix issues.
4855+
//
4856+
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
4857+
48584858
#ifdef DEBUG
48594859
fgDebugCheckLinks();
48604860
#endif

‎src/coreclr/jit/fgflow.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
157157
if (flow != nullptr)
158158
{
159159
// The predecessor block already exists in the flow list; simply add to its duplicate count.
160+
//
160161
noway_assert(flow->getDupCount());
161162
flow->incrementDupCount();
162163
}

‎src/coreclr/jit/fgopt.cpp

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,6 +1891,27 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
18911891
FlowEdge* const newEdge = fgAddRefPred(bNewDest, block, oldEdge);
18921892
*jmpTab = newEdge;
18931893

1894+
// Update edge likelihoods
1895+
// Note old edge may still be "in use" so we decrease its likelihood.
1896+
//
1897+
if (oldEdge->hasLikelihood())
1898+
{
1899+
// We want to move this much likelihood from old->new
1900+
//
1901+
const weight_t likelihoodFraction = oldEdge->getLikelihood() / (oldEdge->getDupCount() + 1);
1902+
1903+
if (newEdge->getDupCount() == 1)
1904+
{
1905+
newEdge->setLikelihood(likelihoodFraction);
1906+
}
1907+
else
1908+
{
1909+
newEdge->addLikelihood(likelihoodFraction);
1910+
}
1911+
1912+
oldEdge->addLikelihood(-likelihoodFraction);
1913+
}
1914+
18941915
// we optimized a Switch label - goto REPEAT_SWITCH to follow this new jump
18951916
modified = true;
18961917

@@ -2903,19 +2924,32 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
29032924
// We need to update the following flags of the bJump block if they were set in the bDest block
29042925
bJump->CopyFlags(bDest, BBF_COPY_PROPAGATE);
29052926

2906-
/* Update bbRefs and bbPreds */
2927+
// Update bbRefs and bbPreds
2928+
//
2929+
// For now we set the likelihood of the new branch to match
2930+
// the likelihood of the old branch.
2931+
//
2932+
// This may or may not match the block weight adjustments we're
2933+
// making. All this becomes easier to reconcile once we rely on
2934+
// edge likelihoods more and have synthesis running.
2935+
//
2936+
// Until then we won't worry that edges and blocks are potentially
2937+
// out of sync.
2938+
//
2939+
FlowEdge* const destFalseEdge = bDest->GetFalseEdge();
2940+
FlowEdge* const destTrueEdge = bDest->GetTrueEdge();
29072941

29082942
// bJump now falls through into the next block
29092943
//
2910-
FlowEdge* const falseEdge = fgAddRefPred(bJump->Next(), bJump);
2944+
FlowEdge* const falseEdge = fgAddRefPred(bJump->Next(), bJump, destFalseEdge);
29112945

29122946
// bJump no longer jumps to bDest
29132947
//
29142948
fgRemoveRefPred(bJump->GetTargetEdge());
29152949

29162950
// bJump now jumps to bDest's normal jump target
29172951
//
2918-
FlowEdge* const trueEdge = fgAddRefPred(bDestNormalTarget, bJump);
2952+
FlowEdge* const trueEdge = fgAddRefPred(bDestNormalTarget, bJump, destTrueEdge);
29192953

29202954
bJump->SetCond(trueEdge, falseEdge);
29212955

@@ -3076,7 +3110,9 @@ bool Compiler::fgOptimizeSwitchJumps()
30763110
newBlock->setBBProfileWeight(blockToNewBlockWeight);
30773111

30783112
blockToTargetEdge->setEdgeWeights(blockToTargetWeight, blockToTargetWeight, dominantTarget);
3113+
blockToTargetEdge->setLikelihood(fraction);
30793114
blockToNewBlockEdge->setEdgeWeights(blockToNewBlockWeight, blockToNewBlockWeight, block);
3115+
blockToNewBlockEdge->setLikelihood(max(0, 1.0 - fraction));
30803116

30813117
// There may be other switch cases that lead to this same block, but there's just
30823118
// one edge in the flowgraph. So we need to subtract off the profile data that now flows
@@ -5020,11 +5056,20 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
50205056
}
50215057

50225058
// Optimize the Conditional JUMP to go to the new target
5023-
fgRemoveRefPred(block->GetFalseEdge());
5024-
fgRemoveRefPred(bNext->GetTargetEdge());
5025-
block->SetFalseEdge(block->GetTrueEdge());
5026-
FlowEdge* const newEdge = fgAddRefPred(bNext->GetTarget(), block, bNext->GetTargetEdge());
5027-
block->SetTrueEdge(newEdge);
5059+
//
5060+
FlowEdge* const oldFalseEdge = block->GetFalseEdge();
5061+
FlowEdge* const oldTrueEdge = block->GetTrueEdge();
5062+
FlowEdge* const oldNextEdge = bNext->GetTargetEdge();
5063+
5064+
// bNext no longer flows to target
5065+
//
5066+
fgRemoveRefPred(oldNextEdge);
5067+
5068+
// Rewire flow from block
5069+
//
5070+
block->SetFalseEdge(oldTrueEdge);
5071+
FlowEdge* const newTrueEdge = fgAddRefPred(bNext->GetTarget(), block, oldFalseEdge);
5072+
block->SetTrueEdge(newTrueEdge);
50285073

50295074
/*
50305075
Unlink bNext from the BasicBlock list; note that we can

‎src/coreclr/jit/loopcloning.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,11 +853,27 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
853853
noway_assert(conds.Size() > 0);
854854
assert(slowPreheader != nullptr);
855855

856+
// For now assume high likelihood for the fast path,
857+
// uniformly spread across the gating branches.
858+
//
859+
// For "normal" cloning this is probably ok. For GDV cloning this
860+
// may be inaccurate. We should key off the type test likelihood(s).
861+
//
862+
// TODO: this is a bit of out sync with what we do for block weights.
863+
// Reconcile.
864+
//
865+
const weight_t fastLikelihood = 0.999;
866+
856867
// Choose how to generate the conditions
857868
const bool generateOneConditionPerBlock = true;
858869

859870
if (generateOneConditionPerBlock)
860871
{
872+
// N = conds.Size() branches must all be true to execute the fast loop.
873+
// Use the N'th root....
874+
//
875+
const weight_t fastLikelihoodPerBlock = exp(log(fastLikelihood) / (weight_t)conds.Size());
876+
861877
for (unsigned i = 0; i < conds.Size(); ++i)
862878
{
863879
BasicBlock* newBlk = comp->fgNewBBafter(BBJ_COND, insertAfter, /*extendRegion*/ true);
@@ -866,12 +882,14 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
866882
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", newBlk->bbNum, slowPreheader->bbNum);
867883
FlowEdge* const trueEdge = comp->fgAddRefPred(slowPreheader, newBlk);
868884
newBlk->SetTrueEdge(trueEdge);
885+
trueEdge->setLikelihood(1 - fastLikelihoodPerBlock);
869886

870887
if (insertAfter->KindIs(BBJ_COND))
871888
{
872889
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", insertAfter->bbNum, newBlk->bbNum);
873890
FlowEdge* const falseEdge = comp->fgAddRefPred(newBlk, insertAfter);
874891
insertAfter->SetFalseEdge(falseEdge);
892+
falseEdge->setLikelihood(fastLikelihoodPerBlock);
875893
}
876894

877895
JITDUMP("Adding conditions %u to " FMT_BB "\n", i, newBlk->bbNum);
@@ -901,12 +919,14 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
901919
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", newBlk->bbNum, slowPreheader->bbNum);
902920
FlowEdge* const trueEdge = comp->fgAddRefPred(slowPreheader, newBlk);
903921
newBlk->SetTrueEdge(trueEdge);
922+
trueEdge->setLikelihood(1.0 - fastLikelihood);
904923

905924
if (insertAfter->KindIs(BBJ_COND))
906925
{
907926
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", insertAfter->bbNum, newBlk->bbNum);
908927
FlowEdge* const falseEdge = comp->fgAddRefPred(newBlk, insertAfter);
909928
insertAfter->SetFalseEdge(falseEdge);
929+
falseEdge->setLikelihood(fastLikelihood);
910930
}
911931

912932
JITDUMP("Adding conditions to " FMT_BB "\n", newBlk->bbNum);
@@ -2069,6 +2089,8 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
20692089
assert(condLast->NextIs(fastPreheader));
20702090
FlowEdge* const falseEdge = fgAddRefPred(fastPreheader, condLast);
20712091
condLast->SetFalseEdge(falseEdge);
2092+
FlowEdge* const trueEdge = condLast->GetTrueEdge();
2093+
falseEdge->setLikelihood(max(0, 1.0 - trueEdge->getLikelihood()));
20722094
}
20732095

20742096
//-------------------------------------------------------------------------

‎src/coreclr/jit/morph.cpp

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14679,15 +14679,20 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
1467914679
thenBlock->SetFlags(BBF_IMPORTED);
1468014680
}
1468114681

14682+
const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
14683+
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();
14684+
1468214685
FlowEdge* const newEdge = fgAddRefPred(remainderBlock, thenBlock);
1468314686
thenBlock->SetTargetEdge(newEdge);
1468414687

1468514688
assert(condBlock->TargetIs(elseBlock));
14686-
FlowEdge* const falseEdge = fgAddRefPred(thenBlock, condBlock);
14687-
condBlock->SetCond(condBlock->GetTargetEdge(), falseEdge);
14688-
14689-
thenBlock->inheritWeightPercentage(condBlock, qmark->ThenNodeLikelihood());
14690-
elseBlock->inheritWeightPercentage(condBlock, qmark->ElseNodeLikelihood());
14689+
FlowEdge* const elseEdge = fgAddRefPred(thenBlock, condBlock);
14690+
FlowEdge* const thenEdge = condBlock->GetTargetEdge();
14691+
condBlock->SetCond(thenEdge, elseEdge);
14692+
thenBlock->inheritWeightPercentage(condBlock, thenLikelihood);
14693+
elseBlock->inheritWeightPercentage(condBlock, elseLikelihood);
14694+
thenEdge->setLikelihood(thenLikelihood / 100.0);
14695+
elseEdge->setLikelihood(elseLikelihood / 100.0);
1469114696
}
1469214697
else if (hasTrueExpr)
1469314698
{
@@ -14699,15 +14704,21 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
1469914704
//
1470014705
gtReverseCond(condExpr);
1470114706

14707+
const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
14708+
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();
14709+
1470214710
assert(condBlock->TargetIs(elseBlock));
14703-
FlowEdge* const trueEdge = fgAddRefPred(remainderBlock, condBlock);
14704-
condBlock->SetCond(trueEdge, condBlock->GetTargetEdge());
14711+
FlowEdge* const thenEdge = fgAddRefPred(remainderBlock, condBlock);
14712+
FlowEdge* const elseEdge = condBlock->GetTargetEdge();
14713+
condBlock->SetCond(thenEdge, elseEdge);
1470514714

1470614715
// Since we have no false expr, use the one we'd already created.
1470714716
thenBlock = elseBlock;
1470814717
elseBlock = nullptr;
1470914718

14710-
thenBlock->inheritWeightPercentage(condBlock, qmark->ThenNodeLikelihood());
14719+
thenBlock->inheritWeightPercentage(condBlock, thenLikelihood);
14720+
thenEdge->setLikelihood(thenLikelihood / 100.0);
14721+
elseEdge->setLikelihood(elseLikelihood / 100.0);
1471114722
}
1471214723
else if (hasFalseExpr)
1471314724
{
@@ -14717,11 +14728,17 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
1471714728
// +-->------------+
1471814729
// bbj_cond(true)
1471914730
//
14731+
const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
14732+
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();
14733+
1472014734
assert(condBlock->TargetIs(elseBlock));
14721-
FlowEdge* const trueEdge = fgAddRefPred(remainderBlock, condBlock);
14722-
condBlock->SetCond(trueEdge, condBlock->GetTargetEdge());
14735+
FlowEdge* const thenEdge = fgAddRefPred(remainderBlock, condBlock);
14736+
FlowEdge* const elseEdge = condBlock->GetTargetEdge();
14737+
condBlock->SetCond(thenEdge, elseEdge);
1472314738

14724-
elseBlock->inheritWeightPercentage(condBlock, qmark->ElseNodeLikelihood());
14739+
elseBlock->inheritWeightPercentage(condBlock, elseLikelihood);
14740+
thenEdge->setLikelihood(thenLikelihood / 100.0);
14741+
elseEdge->setLikelihood(elseLikelihood / 100.0);
1472514742
}
1472614743

1472714744
assert(condBlock->KindIs(BBJ_COND));

0 commit comments

Comments
 (0)