Skip to content

Commit 1bc3570

Browse files
JIT: Set naive likelihoods when initializing preds (#98192)
Part of #93020. Per discussion on #98054, set "naive" edge likelihoods (in other words, assume every successor edge is equally likely to be taken) in fgAddRefPred when initializing preds.
1 parent b47568b commit 1bc3570

File tree

4 files changed

+59
-18
lines changed

4 files changed

+59
-18
lines changed

src/coreclr/jit/fgbasic.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2904,26 +2904,26 @@ void Compiler::fgLinkBasicBlocks()
29042904
//
29052905
fgFirstBB->bbRefs = 1;
29062906

2907-
// Special args to fgAddRefPred so it will use the initialization fast path.
2907+
// Special arg to fgAddRefPred so it will use the initialization fast path.
29082908
//
2909-
FlowEdge* const oldEdge = nullptr;
2910-
bool const initializingPreds = true;
2909+
const bool initializingPreds = true;
29112910

29122911
for (BasicBlock* const curBBdesc : Blocks())
29132912
{
29142913
switch (curBBdesc->GetKind())
29152914
{
29162915
case BBJ_COND:
29172916
{
2918-
BasicBlock* const trueTarget = fgLookupBB(curBBdesc->GetTargetOffs());
2917+
BasicBlock* const trueTarget = fgLookupBB(curBBdesc->GetTargetOffs());
2918+
BasicBlock* const falseTarget = curBBdesc->Next();
29192919
curBBdesc->SetTrueTarget(trueTarget);
2920-
curBBdesc->SetFalseTarget(curBBdesc->Next());
2921-
fgAddRefPred<initializingPreds>(trueTarget, curBBdesc, oldEdge);
2922-
fgAddRefPred<initializingPreds>(curBBdesc->GetFalseTarget(), curBBdesc, oldEdge);
2920+
curBBdesc->SetFalseTarget(falseTarget);
2921+
fgAddRefPred<initializingPreds>(trueTarget, curBBdesc);
2922+
fgAddRefPred<initializingPreds>(falseTarget, curBBdesc);
29232923

2924-
if (curBBdesc->GetTrueTarget()->bbNum <= curBBdesc->bbNum)
2924+
if (trueTarget->bbNum <= curBBdesc->bbNum)
29252925
{
2926-
fgMarkBackwardJump(curBBdesc->GetTrueTarget(), curBBdesc);
2926+
fgMarkBackwardJump(trueTarget, curBBdesc);
29272927
}
29282928

29292929
if (curBBdesc->IsLast())
@@ -2945,7 +2945,7 @@ void Compiler::fgLinkBasicBlocks()
29452945
// Redundantly use SetKindAndTarget() instead of SetTarget() just this once,
29462946
// so we don't break the HasInitializedTarget() invariant of SetTarget().
29472947
curBBdesc->SetKindAndTarget(curBBdesc->GetKind(), jumpDest);
2948-
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc, oldEdge);
2948+
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc);
29492949

29502950
if (curBBdesc->GetTarget()->bbNum <= curBBdesc->bbNum)
29512951
{
@@ -2978,7 +2978,7 @@ void Compiler::fgLinkBasicBlocks()
29782978
{
29792979
BasicBlock* jumpDest = fgLookupBB((unsigned)*(size_t*)jumpPtr);
29802980
*jumpPtr = jumpDest;
2981-
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc, oldEdge);
2981+
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc);
29822982
if ((*jumpPtr)->bbNum <= curBBdesc->bbNum)
29832983
{
29842984
fgMarkBackwardJump(*jumpPtr, curBBdesc);
@@ -3839,7 +3839,8 @@ void Compiler::fgFindBasicBlocks()
38393839
{
38403840
// Mark catch handler as successor.
38413841
block->SetTarget(hndBegBB);
3842-
fgAddRefPred(hndBegBB, block);
3842+
FlowEdge* const newEdge = fgAddRefPred(hndBegBB, block);
3843+
newEdge->setLikelihood(1.0);
38433844
assert(block->GetTarget()->bbCatchTyp == BBCT_FILTER_HANDLER);
38443845
break;
38453846
}

src/coreclr/jit/fgflow.cpp

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,32 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
136136
if (flowLast->getSourceBlock() == blockPred)
137137
{
138138
flow = flowLast;
139+
140+
// This edge should have been given a likelihood when it was created.
141+
// Since we're increasing its duplicate count, update the likelihood.
142+
//
143+
assert(flow->hasLikelihood());
144+
const unsigned numSucc = blockPred->NumSucc();
145+
assert(numSucc > 0);
146+
147+
if (numSucc == 1)
148+
{
149+
// BasicBlock::NumSucc() returns 1 for BBJ_CONDs with the same true/false target.
150+
// For blocks that only ever have one successor (BBJ_ALWAYS, BBJ_LEAVE, etc.),
151+
// their successor edge should never have a duplicate count over 1.
152+
//
153+
assert(blockPred->KindIs(BBJ_COND));
154+
assert(blockPred->TrueTargetIs(blockPred->GetFalseTarget()));
155+
flow->setLikelihood(1.0);
156+
}
157+
else
158+
{
159+
// Duplicate count isn't updated until later, so add 1 for now.
160+
//
161+
const unsigned dupCount = flow->getDupCount() + 1;
162+
assert(dupCount > 1);
163+
flow->setLikelihood((1.0 / numSucc) * dupCount);
164+
}
139165
}
140166
}
141167
}
@@ -185,12 +211,19 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
185211
if (initializingPreds)
186212
{
187213
block->bbLastPred = flow;
188-
}
189214

190-
// Copy likelihood from old edge, if any.
191-
//
192-
if ((oldEdge != nullptr) && oldEdge->hasLikelihood())
215+
// When initializing preds, ensure edge likelihood is set,
216+
// such that this edge is as likely as any other successor edge
217+
//
218+
const unsigned numSucc = blockPred->NumSucc();
219+
assert(numSucc > 0);
220+
assert(flow->getDupCount() == 1);
221+
flow->setLikelihood(1.0 / numSucc);
222+
}
223+
else if ((oldEdge != nullptr) && oldEdge->hasLikelihood())
193224
{
225+
// Copy likelihood from old edge, if any.
226+
//
194227
flow->setLikelihood(oldEdge->getLikelihood());
195228
}
196229

@@ -235,6 +268,10 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
235268
//
236269
assert(block->checkPredListOrder());
237270

271+
// When initializing preds, edge likelihood should always be set.
272+
//
273+
assert(!initializingPreds || flow->hasLikelihood());
274+
238275
return flow;
239276
}
240277

src/coreclr/jit/fgprofile.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3916,7 +3916,10 @@ void EfficientEdgeCountReconstructor::PropagateOSREntryEdges(BasicBlock* block,
39163916
FlowEdge* const flowEdge = m_comp->fgGetPredForBlock(edge->m_targetBlock, block);
39173917

39183918
assert(flowEdge != nullptr);
3919-
assert(!flowEdge->hasLikelihood());
3919+
3920+
// Naive likelihood should have been set during pred initialization in fgAddRefPred
3921+
//
3922+
assert(flowEdge->hasLikelihood());
39203923
weight_t likelihood = 0;
39213924

39223925
if (nEdges == 1)

src/coreclr/jit/importer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12280,7 +12280,7 @@ void Compiler::impFixPredLists()
1228012280

1228112281
BasicBlock* const continuation = predBlock->Next();
1228212282
FlowEdge* const newEdge = fgAddRefPred(continuation, finallyBlock);
12283-
newEdge->setLikelihood(1.0);
12283+
newEdge->setLikelihood(1.0 / predCount);
1228412284

1228512285
jumpEhf->bbeSuccs[predNum] = continuation;
1228612286
++predNum;

0 commit comments

Comments
 (0)