Skip to content

Commit

Permalink
JIT: don't create degenerate flow in fgReplaceJumpTarget
Browse files Browse the repository at this point in the history
Detect if the update has created degenerate BBJ_COND flow, and simplify.
Also symmetrize the true/false cases.

Fixes dotnet#48609.

Also fixes some issues seen in enhanced likelhood checking.
  • Loading branch information
AndyAyersMS committed Mar 11, 2024
1 parent da781b3 commit 313654e
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,8 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas

if (block->FalseEdgeIs(oldEdge))
{
// Branch was degenerate, simplify it first
//
// fgRemoveRefPred returns nullptr for BBJ_COND blocks with two flow edges to target
fgRemoveConditionalJump(block);
assert(block->KindIs(BBJ_ALWAYS));
Expand All @@ -671,10 +673,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 @@ -684,24 +683,48 @@ 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();

if (block->TrueEdgeIs(oldEdge))
{
// Branch was degenerate
//
// fgRemoveRefPred returns nullptr for BBJ_COND blocks with two flow edges to target
fgRemoveConditionalJump(block);
assert(block->KindIs(BBJ_ALWAYS));
assert(block->TargetIs(oldTarget));
}

// 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->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 313654e

Please sign in to comment.