Skip to content

Commit

Permalink
JIT: Explicitly set bbFalseTarget in BasicBlock::SetCond (#96406)
Browse files Browse the repository at this point in the history
  • Loading branch information
amanasifkhalid authored Jan 2, 2024
1 parent cb2e382 commit 4b19d67
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 26 deletions.
6 changes: 4 additions & 2 deletions src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,8 @@ void BasicBlock::CopyTarget(Compiler* compiler, const BasicBlock* from)
SetEhf(new (compiler, CMK_BasicBlock) BBehfDesc(compiler, from->GetEhfTargets()));
break;
case BBJ_COND:
SetCond(from->GetTrueTarget());
// TODO-NoFallThrough: Copy false target, too?
SetCond(from->GetTrueTarget(), Next());
break;
case BBJ_ALWAYS:
SetKindAndTarget(from->GetKind(), from->GetTarget());
Expand Down Expand Up @@ -876,7 +877,8 @@ void BasicBlock::TransferTarget(BasicBlock* from)
from->bbEhfTargets = nullptr; // Make sure nobody uses the descriptor after this.
break;
case BBJ_COND:
SetCond(from->GetTrueTarget());
// TODO-NoFallThrough: Copy false target, too?
SetCond(from->GetTrueTarget(), Next());
break;
case BBJ_ALWAYS:
SetKindAndTarget(from->GetKind(), from->GetTarget());
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,14 @@ struct BasicBlock : private LIR::Range
return (bbFalseTarget == target);
}

void SetCond(BasicBlock* trueTarget)
void SetCond(BasicBlock* trueTarget, BasicBlock* falseTarget)
{
assert(trueTarget != nullptr);
// TODO-NoFallThrough: Allow falseTarget to diverge from bbNext
assert(falseTarget == bbNext);
bbKind = BBJ_COND;
bbTrueTarget = trueTarget;
bbFalseTarget = bbNext;
bbFalseTarget = falseTarget;
}

// Set both the block kind and target. This can clear `bbTarget` when setting
Expand Down
18 changes: 8 additions & 10 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
GenTree* const jumpIfEntryStateZero = gtNewOperNode(GT_JTRUE, TYP_VOID, compareEntryStateToZero);
fgNewStmtAtBeg(fromBlock, jumpIfEntryStateZero);

fromBlock->SetCond(toBlock);
fromBlock->SetCond(toBlock, newBlock);
fgAddRefPred(toBlock, fromBlock);
newBlock->inheritWeight(fromBlock);

Expand Down Expand Up @@ -2307,12 +2307,12 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
break;

case BBJ_COND:
block->SetCond(bNext->GetTrueTarget());
block->SetCond(bNext->GetTrueTarget(), bNext->GetFalseTarget());

/* Update the predecessor list for 'bNext->bbTarget' */
/* Update the predecessor list for 'bNext->bbTrueTarget' */
fgReplacePred(bNext->GetTrueTarget(), bNext, block);

/* Update the predecessor list for 'bNext->bbFalseTarget' if it is different than 'bNext->bbTarget' */
/* Update the predecessor list for 'bNext->bbFalseTarget' if it is different than 'bNext->bbTrueTarget' */
if (!bNext->TrueTargetIs(bNext->GetFalseTarget()))
{
fgReplacePred(bNext->GetFalseTarget(), bNext, block);
Expand Down Expand Up @@ -3295,7 +3295,7 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
fgSetStmtSeq(switchStmt);
}

block->SetCond(block->GetSwitchTargets()->bbsDstTab[0]);
block->SetCond(block->GetSwitchTargets()->bbsDstTab[0], block->GetSwitchTargets()->bbsDstTab[1]);

JITDUMP("After:\n");
DISPNODE(switchTree);
Expand Down Expand Up @@ -3710,7 +3710,7 @@ bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock*

// Fix up block's flow
//
block->SetCond(target->GetTrueTarget());
block->SetCond(target->GetTrueTarget(), next);
fgAddRefPred(block->GetTrueTarget(), block);
fgRemoveRefPred(target, block);

Expand Down Expand Up @@ -4117,7 +4117,7 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
// We need to update the following flags of the bJump block if they were set in the bDest block
bJump->CopyFlags(bDest, BBF_COPY_PROPAGATE);

bJump->SetCond(bDestNormalTarget);
bJump->SetCond(bDestNormalTarget, bJump->Next());

/* Update bbRefs and bbPreds */

Expand Down Expand Up @@ -4277,7 +4277,7 @@ bool Compiler::fgOptimizeSwitchJumps()

// Wire up the new control flow.
//
block->SetCond(dominantTarget);
block->SetCond(dominantTarget, newBlock);
FlowEdge* const blockToTargetEdge = fgAddRefPred(dominantTarget, block);
FlowEdge* const blockToNewBlockEdge = newBlock->bbPreds;
assert(blockToNewBlockEdge->getSourceBlock() == block);
Expand Down Expand Up @@ -6205,8 +6205,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase)
fgRemoveRefPred(bDestNext, bDest);
fgAddRefPred(bFixup, bDest);
fgAddRefPred(bDestNext, bFixup);

bDest->SetFalseTarget(bDest->Next());
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ BasicBlock* Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block)
}
#endif

top->SetCond(bottom);
top->SetCond(bottom, poll);
// Bottom has Top and Poll as its predecessors. Poll has just Top as a predecessor.
fgAddRefPred(bottom, poll);
fgAddRefPred(bottom, top);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ class IndirectCallTransformer

// checkBlock
assert(checkBlock->KindIs(BBJ_ALWAYS));
checkBlock->SetCond(elseBlock);
checkBlock->SetCond(elseBlock, thenBlock);
compiler->fgAddRefPred(elseBlock, checkBlock);
compiler->fgAddRefPred(thenBlock, checkBlock);

Expand Down Expand Up @@ -591,7 +591,7 @@ class IndirectCallTransformer
checkFallsThrough = false;

// prevCheckBlock is expected to jump to this new check (if its type check doesn't succeed)
prevCheckBlock->SetCond(checkBlock);
prevCheckBlock->SetCond(checkBlock, prevCheckBlock->Next());
compiler->fgAddRefPred(checkBlock, prevCheckBlock);

// Calculate the total likelihood for this check as a sum of likelihoods
Expand Down Expand Up @@ -1031,7 +1031,7 @@ class IndirectCallTransformer
// where we know the last check is always true (in case of "exact" GDV)
if (!checkFallsThrough)
{
checkBlock->SetCond(elseBlock);
checkBlock->SetCond(elseBlock, checkBlock->Next());
compiler->fgAddRefPred(elseBlock, checkBlock);
}
else
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ GenTree* Lowering::LowerSwitch(GenTree* node)
// The GT_SWITCH code is still in originalSwitchBB (it will be removed later).

// Turn originalSwitchBB into a BBJ_COND.
originalSwitchBB->SetCond(jumpTab[jumpCnt - 1]);
originalSwitchBB->SetCond(jumpTab[jumpCnt - 1], afterDefaultCondBlock);

// Fix the pred for the default case: the default block target still has originalSwitchBB
// as a predecessor, but the fgSplitBlockAfterStatement() moved all predecessors to point
Expand Down Expand Up @@ -1101,7 +1101,7 @@ GenTree* Lowering::LowerSwitch(GenTree* node)
{
// Otherwise, it's a conditional branch. Set the branch kind, then add the
// condition statement.
currentBlock->SetCond(jumpTab[i]);
currentBlock->SetCond(jumpTab[i], currentBlock->Next());

// Now, build the conditional statement for the current case that is
// being evaluated:
Expand Down Expand Up @@ -1313,15 +1313,15 @@ bool Lowering::TryLowerSwitchToBitTest(
{
// GenCondition::C generates JC so we jump to bbCase1 when the bit is set
bbSwitchCondition = GenCondition::C;
bbSwitch->SetCond(bbCase1);
bbSwitch->SetCond(bbCase1, bbCase0);
}
else
{
assert(bbSwitch->NextIs(bbCase1));

// GenCondition::NC generates JNC so we jump to bbCase0 when the bit is not set
bbSwitchCondition = GenCondition::NC;
bbSwitch->SetCond(bbCase0);
bbSwitch->SetCond(bbCase0, bbCase1);
}

comp->fgAddRefPred(bbCase0, bbSwitch);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14910,7 +14910,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)

thenBlock = fgNewBBafter(BBJ_ALWAYS, condBlock, true, remainderBlock);
thenBlock->SetFlags(propagateFlagsToAll);
condBlock->SetCond(elseBlock);
condBlock->SetCond(elseBlock, thenBlock);
if (!block->HasFlag(BBF_INTERNAL))
{
thenBlock->RemoveFlags(BBF_INTERNAL);
Expand All @@ -14932,7 +14932,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
// bbj_cond(true)
//
gtReverseCond(condExpr);
condBlock->SetCond(remainderBlock);
condBlock->SetCond(remainderBlock, elseBlock);
fgAddRefPred(remainderBlock, condBlock);
// Since we have no false expr, use the one we'd already created.
thenBlock = elseBlock;
Expand All @@ -14948,7 +14948,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
// +-->------------+
// bbj_cond(true)
//
condBlock->SetCond(remainderBlock);
condBlock->SetCond(remainderBlock, elseBlock);
fgAddRefPred(remainderBlock, condBlock);

elseBlock->inheritWeightPercentage(condBlock, 50);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/patchpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class PatchpointTransformer
BasicBlock* helperBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, block, block->Next());

// Update flow and flags
block->SetCond(remainderBlock);
block->SetCond(remainderBlock, helperBlock);
block->SetFlags(BBF_INTERNAL);

helperBlock->SetFlags(BBF_BACKWARD_JUMP | BBF_NONE_QUIRK);
Expand Down

0 comments on commit 4b19d67

Please sign in to comment.