Skip to content

Commit 343b034

Browse files
JIT: Fix edge likelihoods in fgOptimizeBranch (#113235)
Part of #113108. When converting an unconditional jump to a test block into a duplicate test block, fgOptimizeBranch flips the condition (unnecessarily -- I plan to fix this someday), which means we need to derive the new true edge's likelihood from the cloned false edge, and vice versa. Now that late profile repair is enabled, if we get this wrong, the flipped edge likelihoods will flatten loop weights, and inflate non-loop weights. I suspect fixing this will also revert some of the regressions from late profile repair.
1 parent 5a0ea7c commit 343b034

File tree

1 file changed

+9
-8
lines changed

1 file changed

+9
-8
lines changed

src/coreclr/jit/fgopt.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2768,15 +2768,18 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
27682768
FlowEdge* const destFalseEdge = bDest->GetFalseEdge();
27692769
FlowEdge* const destTrueEdge = bDest->GetTrueEdge();
27702770

2771-
// bJump now falls through into the next block
2771+
// bJump now falls through into the next block.
2772+
// Note that we're deriving the false edge's likelihood from 'destTrueEdge',
2773+
// because the comparison in 'bJump' is flipped.
2774+
// Similarly, we will derive the true edge's likelihood from 'destFalseEdge'.
27722775
//
27732776
BasicBlock* const bDestFalseTarget = bJump->Next();
2774-
FlowEdge* const falseEdge = fgAddRefPred(bDestFalseTarget, bJump, destFalseEdge);
2777+
FlowEdge* const falseEdge = fgAddRefPred(bDestFalseTarget, bJump, destTrueEdge);
27752778

27762779
// bJump now jumps to bDest's normal jump target
27772780
//
27782781
fgRedirectTargetEdge(bJump, bDestNormalTarget);
2779-
bJump->GetTargetEdge()->setLikelihood(destTrueEdge->getLikelihood());
2782+
bJump->GetTargetEdge()->setLikelihood(destFalseEdge->getLikelihood());
27802783

27812784
bJump->SetCond(bJump->GetTargetEdge(), falseEdge);
27822785

@@ -2787,18 +2790,16 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
27872790
// bJump no longer flows into bDest
27882791
//
27892792
bDest->decreaseBBProfileWeight(bJump->bbWeight);
2790-
bDestNormalTarget->decreaseBBProfileWeight(bJump->bbWeight * destFalseEdge->getLikelihood());
2791-
bDestFalseTarget->decreaseBBProfileWeight(bJump->bbWeight * destTrueEdge->getLikelihood());
27922793

27932794
// Propagate bJump's weight into its new successors
27942795
//
2795-
bDestNormalTarget->increaseBBProfileWeight(bJump->GetTrueEdge()->getLikelyWeight());
2796-
bDestFalseTarget->increaseBBProfileWeight(falseEdge->getLikelyWeight());
2796+
bDestNormalTarget->setBBProfileWeight(bDestNormalTarget->computeIncomingWeight());
2797+
bDestFalseTarget->setBBProfileWeight(bDestFalseTarget->computeIncomingWeight());
27972798

27982799
if ((bDestNormalTarget->NumSucc() > 0) || (bDestFalseTarget->NumSucc() > 0))
27992800
{
28002801
JITDUMP("fgOptimizeBranch: New flow out of " FMT_BB " needs to be propagated. Data %s inconsistent.\n",
2801-
fgPgoConsistent ? "is now" : "was already");
2802+
bJump->bbNum, fgPgoConsistent ? "is now" : "was already");
28022803
fgPgoConsistent = false;
28032804
}
28042805
}

0 commit comments

Comments
 (0)