Skip to content

JIT: Add fgRedirectTrueEdge, fgRedirectFalseEdge #99561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 12, 2024
Merged

JIT: Add fgRedirectTrueEdge, fgRedirectFalseEdge #99561

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a06198c
Add fgRedirectTrueEdge, fgRedirectFalseEdge
amanasifkhalid Mar 11, 2024
0918241
Merge from main
amanasifkhalid Mar 11, 2024
6e444dc
Fix build
amanasifkhalid Mar 11, 2024
a37b3a4
Merge from main
amanasifkhalid Mar 12, 2024
b8a40c4
Feedback
amanasifkhalid Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5228,14 +5228,6 @@ class Compiler
void fgCleanupContinuation(BasicBlock* continuation);

PhaseStatus fgTailMergeThrows();
void fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock,
BasicBlock* nonCanonicalBlock,
BasicBlock* canonicalBlock,
FlowEdge* predEdge);
void fgTailMergeThrowsJumpToHelper(BasicBlock* predBlock,
BasicBlock* nonCanonicalBlock,
BasicBlock* canonicalBlock,
FlowEdge* predEdge);

bool fgRetargetBranchesToCanonicalCallFinally(BasicBlock* block,
BasicBlock* handler,
Expand Down Expand Up @@ -5915,6 +5907,10 @@ class Compiler
public:
void fgRedirectTargetEdge(BasicBlock* block, BasicBlock* newTarget);

void fgRedirectTrueEdge(BasicBlock* block, BasicBlock* newTarget);

void fgRedirectFalseEdge(BasicBlock* block, BasicBlock* newTarget);

void fgFindBasicBlocks();

bool fgCheckEHCanInsertAfterBlock(BasicBlock* blk, unsigned regionIndex, bool putInTryRegion);
Expand Down
43 changes: 8 additions & 35 deletions src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,57 +650,30 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
case BBJ_COND:
if (block->TrueTargetIs(oldTarget))
{
FlowEdge* const oldEdge = block->GetTrueEdge();

if (block->FalseEdgeIs(oldEdge))
if (block->FalseEdgeIs(block->GetTrueEdge()))
{
// Branch was degenerate, simplify it first
//
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);

if (block->KindIs(BBJ_ALWAYS))
{
block->SetTargetEdge(newEdge);
fgRedirectTargetEdge(block, newTarget);
}
else
{
assert(block->KindIs(BBJ_COND));
block->SetTrueEdge(newEdge);
fgRedirectTrueEdge(block, newTarget);
}
}
else
{
// Already degenerate cases should have taken the true path above
//
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);

if (block->KindIs(BBJ_ALWAYS))
{
block->SetTargetEdge(newEdge);
}
else
{
assert(block->KindIs(BBJ_COND));
block->SetFalseEdge(newEdge);
}
assert(!block->TrueEdgeIs(block->GetFalseEdge()));
fgRedirectFalseEdge(block, newTarget);
}

if (block->KindIs(BBJ_COND) && (block->GetFalseEdge() == block->GetTrueEdge()))
if (block->KindIs(BBJ_COND) && block->TrueEdgeIs(block->GetFalseEdge()))
{
// Block became degenerate, simplify
//
Expand Down
82 changes: 1 addition & 81 deletions src/coreclr/jit/fgehopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2006,26 +2006,8 @@ PhaseStatus Compiler::fgTailMergeThrows()
{
switch (predBlock->GetKind())
{
// TODO: Use fgReplaceJumpTarget once it preserves edge weights for BBJ_COND blocks
case BBJ_COND:
{
// Flow to non canonical block could be via true or false target.
if (predBlock->FalseTargetIs(nonCanonicalBlock))
{
fgTailMergeThrowsFallThroughHelper(predBlock, nonCanonicalBlock, canonicalBlock,
predBlock->GetFalseEdge());
}

if (predBlock->TrueTargetIs(nonCanonicalBlock))
{
fgTailMergeThrowsJumpToHelper(predBlock, nonCanonicalBlock, canonicalBlock,
predBlock->GetTrueEdge());
}
updated = true;
}
break;

case BBJ_ALWAYS:
case BBJ_COND:
case BBJ_SWITCH:
{
JITDUMP("*** " FMT_BB " now branching to " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);
Expand Down Expand Up @@ -2068,65 +2050,3 @@ PhaseStatus Compiler::fgTailMergeThrows()
fgModified = false;
return PhaseStatus::MODIFIED_EVERYTHING;
}

//------------------------------------------------------------------------
// fgTailMergeThrowsFallThroughHelper: fixup flow for fall throughs to mergeable throws
//
// Arguments:
// predBlock - block falling through to the throw helper
// nonCanonicalBlock - original fall through target
// canonicalBlock - new (jump) target
// predEdge - original flow edge
//
// Notes:
// Alters fall through flow of predBlock so it jumps to the
// canonicalBlock via a new basic block. Does not try and fix
// jump-around flow; we leave that to optOptimizeFlow which runs
// just afterwards.
//
void Compiler::fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock,
BasicBlock* nonCanonicalBlock,
BasicBlock* canonicalBlock,
FlowEdge* predEdge)
{
assert(predBlock->KindIs(BBJ_COND));
assert(predBlock->FalseTargetIs(nonCanonicalBlock));

JITDUMP("*** " FMT_BB " false target is now " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);

// Remove the old flow
fgRemoveRefPred(predEdge);

// Wire up the new flow
FlowEdge* const falseEdge = fgAddRefPred(canonicalBlock, predBlock, predEdge);
predBlock->SetFalseEdge(falseEdge);
}

//------------------------------------------------------------------------
// fgTailMergeThrowsJumpToHelper: fixup flow for jumps to mergeable throws
//
// Arguments:
// predBlock - block jumping to the throw helper
// nonCanonicalBlock - original jump target
// canonicalBlock - new jump target
// predEdge - original flow edge
//
// Notes:
// Alters jumpDest of predBlock so it jumps to the canonicalBlock.
//
void Compiler::fgTailMergeThrowsJumpToHelper(BasicBlock* predBlock,
BasicBlock* nonCanonicalBlock,
BasicBlock* canonicalBlock,
FlowEdge* predEdge)
{
JITDUMP("*** " FMT_BB " now branching to " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);

// Remove the old flow
assert(predBlock->KindIs(BBJ_COND));
assert(predBlock->TrueTargetIs(nonCanonicalBlock));
fgRemoveRefPred(predBlock->GetTrueEdge());

// Wire up the new flow
FlowEdge* const trueEdge = fgAddRefPred(canonicalBlock, predBlock, predEdge);
predBlock->SetTrueEdge(trueEdge);
}
78 changes: 78 additions & 0 deletions src/coreclr/jit/fgflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,84 @@ void Compiler::fgRedirectTargetEdge(BasicBlock* block, BasicBlock* newTarget)
newTarget->bbRefs++;
}

void Compiler::fgRedirectTrueEdge(BasicBlock* block, BasicBlock* newTarget)
Copy link
Member

@AndyAyersMS AndyAyersMS Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a header comment block for this method

{
assert(block != nullptr);
assert(newTarget != nullptr);
assert(block->KindIs(BBJ_COND));
assert(!block->TrueEdgeIs(block->GetFalseEdge()));

// Update oldTarget's pred list.
// We could call fgRemoveRefPred, but since we're removing the one and only ref from block to oldTarget,
// fgRemoveAllRefPreds is slightly more efficient (one fewer branch, doesn't update edge's dup count, etc).
//
FlowEdge* trueEdge = block->GetTrueEdge();
BasicBlock* oldTarget = trueEdge->getDestinationBlock();
fgRemoveAllRefPreds(oldTarget, block);

// Splice edge into new target block's pred list
//
FlowEdge** predListPtr = fgGetPredInsertPoint(block, newTarget);
FlowEdge* predEdge = *predListPtr;

if (block->FalseEdgeIs(predEdge))
{
block->SetTrueEdge(predEdge);
predEdge->incrementDupCount();
}
else
{
trueEdge->setNextPredEdge(predEdge);
trueEdge->setDestinationBlock(newTarget);
*predListPtr = trueEdge;
}

newTarget->bbRefs++;

// Pred list of target should (stilL) be ordered
Copy link
Member

@AndyAyersMS AndyAyersMS Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Pred list of target should (stilL) be ordered
// Pred list of target should (still) be ordered

//
assert(newTarget->checkPredListOrder());
}

void Compiler::fgRedirectFalseEdge(BasicBlock* block, BasicBlock* newTarget)
Copy link
Member

@AndyAyersMS AndyAyersMS Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a header comment block for this method

{
assert(block != nullptr);
assert(newTarget != nullptr);
assert(block->KindIs(BBJ_COND));
assert(!block->TrueEdgeIs(block->GetFalseEdge()));

// Update oldTarget's pred list.
// We could call fgRemoveRefPred, but since we're removing the one and only ref from block to oldTarget,
// fgRemoveAllRefPreds is slightly more efficient (one fewer branch, doesn't update edge's dup count, etc).
//
FlowEdge* falseEdge = block->GetFalseEdge();
BasicBlock* oldTarget = falseEdge->getDestinationBlock();
fgRemoveAllRefPreds(oldTarget, block);

// Splice edge into new target block's pred list
//
FlowEdge** predListPtr = fgGetPredInsertPoint(block, newTarget);
FlowEdge* predEdge = *predListPtr;

if (block->TrueEdgeIs(predEdge))
{
block->SetFalseEdge(predEdge);
predEdge->incrementDupCount();
}
else
{
falseEdge->setNextPredEdge(predEdge);
falseEdge->setDestinationBlock(newTarget);
*predListPtr = falseEdge;
}

newTarget->bbRefs++;

// Pred list of target should (stilL) be ordered
Copy link
Member

@AndyAyersMS AndyAyersMS Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Pred list of target should (stilL) be ordered
// Pred list of target should (still) be ordered

//
assert(newTarget->checkPredListOrder());
}

Compiler::SwitchUniqueSuccSet Compiler::GetDescriptorForSwitch(BasicBlock* switchBlk)
{
assert(switchBlk->KindIs(BBJ_SWITCH));
Expand Down
38 changes: 17 additions & 21 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,16 +1548,12 @@ bool Compiler::fgOptimizeBranchToEmptyUnconditional(BasicBlock* block, BasicBloc
if (block->TrueTargetIs(bDest))
{
assert(!block->FalseTargetIs(bDest));
fgRemoveRefPred(block->GetTrueEdge());
FlowEdge* const trueEdge = fgAddRefPred(bDest->GetTarget(), block, block->GetTrueEdge());
block->SetTrueEdge(trueEdge);
fgRedirectTrueEdge(block, bDest->GetTarget());
}
else
{
assert(block->FalseTargetIs(bDest));
fgRemoveRefPred(block->GetFalseEdge());
FlowEdge* const falseEdge = fgAddRefPred(bDest->GetTarget(), block, block->GetFalseEdge());
block->SetFalseEdge(falseEdge);
fgRedirectFalseEdge(block, bDest->GetTarget());
}
break;

Expand Down Expand Up @@ -2478,11 +2474,11 @@ bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock*
// Fix up block's flow.
// Assume edge likelihoods transfer over.
//
fgRemoveRefPred(block->GetTargetEdge());
fgRedirectTargetEdge(block, target->GetTrueTarget());
block->GetTargetEdge()->setLikelihood(target->GetTrueEdge()->getLikelihood());

FlowEdge* const trueEdge = fgAddRefPred(target->GetTrueTarget(), block, target->GetTrueEdge());
FlowEdge* const falseEdge = fgAddRefPred(target->GetFalseTarget(), block, target->GetFalseEdge());
block->SetCond(trueEdge, falseEdge);
block->SetCond(block->GetTargetEdge(), falseEdge);

JITDUMP("fgOptimizeUncondBranchToSimpleCond(from " FMT_BB " to cond " FMT_BB "), modified " FMT_BB "\n",
block->bbNum, target->bbNum, block->bbNum);
Expand Down Expand Up @@ -2911,15 +2907,12 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
//
FlowEdge* const falseEdge = fgAddRefPred(bJump->Next(), bJump, destFalseEdge);

// bJump no longer jumps to bDest
//
fgRemoveRefPred(bJump->GetTargetEdge());

// bJump now jumps to bDest's normal jump target
//
FlowEdge* const trueEdge = fgAddRefPred(bDestNormalTarget, bJump, destTrueEdge);
fgRedirectTargetEdge(bJump, bDestNormalTarget);
bJump->GetTargetEdge()->setLikelihood(destTrueEdge->getLikelihood());

bJump->SetCond(trueEdge, falseEdge);
bJump->SetCond(bJump->GetTargetEdge(), falseEdge);

if (weightJump > 0)
{
Expand Down Expand Up @@ -5036,8 +5029,8 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
// Rewire flow from block
//
block->SetFalseEdge(oldTrueEdge);
FlowEdge* const newTrueEdge = fgAddRefPred(bNext->GetTarget(), block, oldFalseEdge);
block->SetTrueEdge(newTrueEdge);
block->SetTrueEdge(oldFalseEdge);
fgRedirectTrueEdge(block, bNext->GetTarget());

/*
Unlink bNext from the BasicBlock list; note that we can
Expand Down Expand Up @@ -5683,11 +5676,14 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
//
if (commSucc != nullptr)
{
fgRemoveRefPred(predBlock->GetTargetEdge());
assert(predBlock->KindIs(BBJ_ALWAYS));
fgRedirectTargetEdge(predBlock, crossJumpTarget);
}
else
{
FlowEdge* const newEdge = fgAddRefPred(crossJumpTarget, predBlock);
predBlock->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
}

FlowEdge* const newEdge = fgAddRefPred(crossJumpTarget, predBlock);
predBlock->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
}

// We changed things
Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2774,7 +2774,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block)
// the handler go to the prolog. Edges coming from with the handler are back-edges, and
// go to the existing 'block'.

for (BasicBlock* const predBlock : block->PredBlocks())
for (BasicBlock* const predBlock : block->PredBlocksEditing())
{
if (!fgIsIntraHandlerPred(predBlock, block))
{
Expand All @@ -2786,9 +2786,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block)
case BBJ_CALLFINALLY:
{
noway_assert(predBlock->TargetIs(block));
fgRemoveRefPred(predBlock->GetTargetEdge());
FlowEdge* const newEdge = fgAddRefPred(newHead, predBlock);
predBlock->SetTargetEdge(newEdge);
fgRedirectTargetEdge(predBlock, newHead);
break;
}

Expand Down
Loading