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 all 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);
}
112 changes: 112 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,118 @@ void Compiler::fgRedirectTargetEdge(BasicBlock* block, BasicBlock* newTarget)
newTarget->bbRefs++;
}

//------------------------------------------------------------------------
// fgRedirectTrueEdge: Sets block->bbTrueEdge's target block to newTarget,
// updating pred lists as necessary.
//
// Arguments:
// block -- The block we want to make a predecessor of newTarget.
// It could be one already, in which case nothing changes.
// newTarget -- The new successor of block.
//
// Notes:
// This assumes block's true and false targets are different.
// If setting block's true target to its false target,
// fgRedirectTrueEdge increments the false edge's dup count,
// and ensures block->bbTrueEdge == block->bbFalseEdge.
// We don't update newTarget->bbPreds in this case,
// as we don't want to have more than one edge from the same predecessor.
//
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
//
assert(newTarget->checkPredListOrder());
}

//------------------------------------------------------------------------
// fgRedirectFalseEdge: Sets block->bbFalseEdge's target block to newTarget,
// updating pred lists as necessary.
//
// Arguments:
// block -- The block we want to make a predecessor of newTarget.
// It could be one already, in which case nothing changes.
// newTarget -- The new successor of block.
//
// Notes:
// This assumes block's true and false targets are different.
// If setting block's false target to its true target,
// fgRedirectFalseEdge increments the true edge's dup count,
// and ensures block->bbTrueEdge == block->bbFalseEdge.
// We don't update newTarget->bbPreds in this case,
// as we don't want to have more than one edge from the same predecessor.
//
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
//
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
Loading