Skip to content
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.

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

JIT: Explicitly set bbFalseTarget in BasicBlock::SetCond #96406

Merged
merged 1 commit into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -694,12 +694,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 @@ -6204,8 +6204,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 @@ -14904,7 +14904,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 @@ -14926,7 +14926,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 @@ -14942,7 +14942,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