Skip to content

Commit

Permalink
JIT: don't create degenerate flow in fgReplaceJumpTarget (#99509)
Browse files Browse the repository at this point in the history
Detect if the update has created degenerate BBJ_COND flow, and simplify.

Fixes #48609.

Also fixes some issues seen in enhanced likelihood checking.
  • Loading branch information
AndyAyersMS authored Mar 11, 2024
1 parent efa2b78 commit 9d56a4c
Showing 1 changed file with 26 additions and 11 deletions.
37 changes: 26 additions & 11 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas

if (block->FalseEdgeIs(oldEdge))
{
// fgRemoveRefPred returns nullptr for BBJ_COND blocks with two flow edges to target
// Branch was degenerate, simplify it first
//
fgRemoveConditionalJump(block);
assert(block->KindIs(BBJ_ALWAYS));
assert(block->TargetIs(oldTarget));
Expand All @@ -663,10 +664,7 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
// fgRemoveRefPred should have removed the flow edge
fgRemoveRefPred(oldEdge);
assert(oldEdge->getDupCount() == 0);

// TODO-NoFallThrough: Proliferate weight from oldEdge
// (as a quirk, we avoid doing so for the true target to reduce diffs for now)
FlowEdge* const newEdge = fgAddRefPred(newTarget, block);
FlowEdge* const newEdge = fgAddRefPred(newTarget, block, oldEdge);

if (block->KindIs(BBJ_ALWAYS))
{
Expand All @@ -676,24 +674,41 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
{
assert(block->KindIs(BBJ_COND));
block->SetTrueEdge(newEdge);

if (oldEdge->hasLikelihood())
{
newEdge->setLikelihood(oldEdge->getLikelihood());
}
}
}
else
{
assert(block->FalseTargetIs(oldTarget));
FlowEdge* const oldEdge = block->GetFalseEdge();

// Already degenerate cases should have taken the true path above.
assert(!block->TrueEdgeIs(oldEdge));

// fgRemoveRefPred should have removed the flow edge
fgRemoveRefPred(oldEdge);
assert(oldEdge->getDupCount() == 0);
FlowEdge* const newEdge = fgAddRefPred(newTarget, block, oldEdge);
block->SetFalseEdge(newEdge);

if (block->KindIs(BBJ_ALWAYS))
{
block->SetTargetEdge(newEdge);
}
else
{
assert(block->KindIs(BBJ_COND));
block->SetFalseEdge(newEdge);
}
}

if (block->KindIs(BBJ_COND) && (block->GetFalseEdge() == block->GetTrueEdge()))
{
// Block became degenerate, simplify
//
fgRemoveConditionalJump(block);
assert(block->KindIs(BBJ_ALWAYS));
assert(block->TargetIs(newTarget));
}

break;

case BBJ_SWITCH:
Expand Down

0 comments on commit 9d56a4c

Please sign in to comment.