Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7075361
Introduce bbNormalJumpDest
amanasifkhalid Dec 7, 2023
edcd975
Replace Next() with GetNormalJumpDest()
amanasifkhalid Dec 7, 2023
bdc886d
Replace NextIs() with HasNormalJumpTo()
amanasifkhalid Dec 7, 2023
4c71330
Style
amanasifkhalid Dec 7, 2023
bdcf9ea
Update TODO comments
amanasifkhalid Dec 8, 2023
4e94533
Style
amanasifkhalid Dec 8, 2023
3855094
Add comment explaining assert
amanasifkhalid Dec 8, 2023
f630b5d
Rename bbNormalJumpDest to bbFalseTarget
amanasifkhalid Dec 8, 2023
5be6ed2
Rename bbJumpDest to bbTarget
amanasifkhalid Dec 8, 2023
a286d86
Rename SetJumpKindAndTarget variants
amanasifkhalid Dec 8, 2023
9ff966b
Rename bbJumpKind
amanasifkhalid Dec 8, 2023
e632217
Rename bbJumpOffs
amanasifkhalid Dec 8, 2023
dcf7ce9
Rename bbJumpSwt
amanasifkhalid Dec 8, 2023
07ff2dd
Rename bbJumpEhf
amanasifkhalid Dec 8, 2023
e2ab378
Rename HasJumpTo
amanasifkhalid Dec 8, 2023
92139e3
Rename method args, dspJumpKind
amanasifkhalid Dec 8, 2023
6802c17
Introduce getters/setters/checkers for bbTrueTarget
amanasifkhalid Dec 8, 2023
2bddac2
Replace GetTarget with GetTrueTarget for BBJ_COND
amanasifkhalid Dec 8, 2023
32174ab
Replace SetTarget with SetTrueTarget for BBJ_COND
amanasifkhalid Dec 8, 2023
5cbd2c4
Replace TargetIs with TrueTargetIs for BBJ_COND
amanasifkhalid Dec 8, 2023
69cbbc7
Style
amanasifkhalid Dec 8, 2023
a35bd40
Fix comment
amanasifkhalid Dec 8, 2023
f45a824
Fix bbNext/bbTarget access for BBJ_COND in BBSuccList
amanasifkhalid Dec 8, 2023
2e5fc28
Split out SetSwtKindAndTarget and SetEhfKindAndTarget
amanasifkhalid Dec 9, 2023
a8f3ffd
Add SetCondKindAndTarget
amanasifkhalid Dec 9, 2023
b1de3c6
Rename to SetCond
amanasifkhalid Dec 9, 2023
a038073
Rename to SetSwitch
amanasifkhalid Dec 9, 2023
3625c39
Rename to SetEhf
amanasifkhalid Dec 9, 2023
fbd3943
Rename to GetSwitchTarget (for consistency)
amanasifkhalid Dec 9, 2023
ed0b149
Rename GetSwitchTarget > GetSwitchTargets
amanasifkhalid Dec 11, 2023
7f1fbb7
Rename GetEhfTarget > GetEhfTargets
amanasifkhalid Dec 11, 2023
e453580
Rename bbSwtTarget > bbSwtTargets
amanasifkhalid Dec 11, 2023
c78e766
Rename bbEhfTarget/SetEhfTarget
amanasifkhalid Dec 11, 2023
f445e29
Add asserts
amanasifkhalid Dec 11, 2023
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
Prev Previous commit
Next Next commit
Add SetCondKindAndTarget
  • Loading branch information
amanasifkhalid committed Dec 9, 2023
commit a8f3ffd8b813d26d4e42e1963b68bf22a858d754
10 changes: 10 additions & 0 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -704,8 +704,18 @@ struct BasicBlock : private LIR::Range
return (bbFalseTarget == target);
}

void SetCondKindAndTarget(BasicBlock* target)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't exactly roll off the tongue -- why not SetCond? Eventually we would hopefully end up with just SetCond(BasicBlock* falseTarget, BasicBlock* trueTarget), SetSwitch(BBswtDesc* swtTarget) and so on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can shorten the names of all the SetKind... variants. Would you be ok with SetKindAndTarget being left as-is? I'm not sure how we can intuitively shorten that one while still conveying it's setting the jump kind and target.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think SetKindAndTarget makes sense -- it's sort of a low-level operation, while these other ones are more high-level.

{
assert(target != nullptr);
bbKind = BBJ_COND;
bbTrueTarget = target;
}

void SetKindAndTarget(BBKinds kind, BasicBlock* target = nullptr)
{
// To set BBJ_COND, use SetCondKindAndTarget
assert(kind != BBJ_COND);

bbKind = kind;
bbTarget = target;

Expand Down
45 changes: 21 additions & 24 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2939,6 +2939,25 @@ void Compiler::fgLinkBasicBlocks()
switch (curBBdesc->GetKind())
{
case BBJ_COND:
{
BasicBlock* const trueTarget = fgLookupBB(curBBdesc->GetTargetOffs());
curBBdesc->SetTrueTarget(trueTarget);
curBBdesc->SetFalseTarget(curBBdesc->Next());
fgAddRefPred<initializingPreds>(trueTarget, curBBdesc, oldEdge);
fgAddRefPred<initializingPreds>(curBBdesc->GetFalseTarget(), curBBdesc, oldEdge);

if (curBBdesc->GetTrueTarget()->bbNum <= curBBdesc->bbNum)
{
fgMarkBackwardJump(curBBdesc->GetTrueTarget(), curBBdesc);
}

if (curBBdesc->IsLast())
{
BADCODE("Fall thru the end of a method");
}

break;
}
case BBJ_ALWAYS:
case BBJ_LEAVE:
{
Expand All @@ -2953,32 +2972,10 @@ void Compiler::fgLinkBasicBlocks()
curBBdesc->SetKindAndTarget(curBBdesc->GetKind(), jumpDest);
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc, oldEdge);

// Can this block fall through into the next block?
//
if (curBBdesc->KindIs(BBJ_ALWAYS, BBJ_LEAVE))
{
if (curBBdesc->GetTarget()->bbNum <= curBBdesc->bbNum)
{
fgMarkBackwardJump(curBBdesc->GetTarget(), curBBdesc);
}
break;
}

// The fall-through block is also reachable
assert(curBBdesc->KindIs(BBJ_COND));

if (curBBdesc->GetTrueTarget()->bbNum <= curBBdesc->bbNum)
if (curBBdesc->GetTarget()->bbNum <= curBBdesc->bbNum)
{
fgMarkBackwardJump(curBBdesc->GetTrueTarget(), curBBdesc);
fgMarkBackwardJump(curBBdesc->GetTarget(), curBBdesc);
}

if (curBBdesc->IsLast())
{
BADCODE("Fall thru the end of a method");
}

curBBdesc->SetFalseTarget(curBBdesc->Next());
fgAddRefPred<initializingPreds>(curBBdesc->GetFalseTarget(), curBBdesc, oldEdge);
break;
}

Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1759,7 +1759,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
GenTree* const jumpIfEntryStateZero = gtNewOperNode(GT_JTRUE, TYP_VOID, compareEntryStateToZero);
fgNewStmtAtBeg(fromBlock, jumpIfEntryStateZero);

fromBlock->SetKindAndTarget(BBJ_COND, toBlock);
fromBlock->SetCondKindAndTarget(toBlock);
fgAddRefPred(toBlock, fromBlock);
newBlock->inheritWeight(fromBlock);

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

case BBJ_COND:
block->SetKindAndTarget(BBJ_COND, bNext->GetTrueTarget());
block->SetCondKindAndTarget(bNext->GetTrueTarget());

/* Update the predecessor list for 'bNext->bbTarget' */
fgReplacePred(bNext->GetTrueTarget(), bNext, block);
Expand Down Expand Up @@ -3310,7 +3310,7 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
fgSetStmtSeq(switchStmt);
}

block->SetKindAndTarget(BBJ_COND, block->GetSwtTarget()->bbsDstTab[0]);
block->SetCondKindAndTarget(block->GetSwtTarget()->bbsDstTab[0]);

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

// Fix up block's flow
//
block->SetKindAndTarget(BBJ_COND, target->GetTrueTarget());
block->SetCondKindAndTarget(target->GetTrueTarget());
fgAddRefPred(block->GetTrueTarget(), block);
fgRemoveRefPred(target, block);

Expand Down Expand Up @@ -4132,7 +4132,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->SetKindAndTarget(BBJ_COND, bDestNormalTarget);
bJump->SetCondKindAndTarget(bDestNormalTarget);
bJump->SetFalseTarget(bJump->Next());

/* Update bbRefs and bbPreds */
Expand Down Expand Up @@ -4293,7 +4293,7 @@ bool Compiler::fgOptimizeSwitchJumps()

// Wire up the new control flow.
//
block->SetKindAndTarget(BBJ_COND, dominantTarget);
block->SetCondKindAndTarget(dominantTarget);
FlowEdge* const blockToTargetEdge = fgAddRefPred(dominantTarget, block);
FlowEdge* const blockToNewBlockEdge = newBlock->bbPreds;
assert(blockToNewBlockEdge->getSourceBlock() == block);
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->SetKindAndTarget(BBJ_COND, bottom);
top->SetCondKindAndTarget(bottom);
// 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->SetKindAndTarget(BBJ_COND, elseBlock);
checkBlock->SetCondKindAndTarget(elseBlock);
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->SetKindAndTarget(BBJ_COND, checkBlock);
prevCheckBlock->SetCondKindAndTarget(checkBlock);
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->SetKindAndTarget(BBJ_COND, elseBlock);
checkBlock->SetCondKindAndTarget(elseBlock);
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->SetKindAndTarget(BBJ_COND, jumpTab[jumpCnt - 1]);
originalSwitchBB->SetCondKindAndTarget(jumpTab[jumpCnt - 1]);

// 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 @@ -1100,7 +1100,7 @@ GenTree* Lowering::LowerSwitch(GenTree* node)
{
// Otherwise, it's a conditional branch. Set the branch kind, then add the
// condition statement.
currentBlock->SetKindAndTarget(BBJ_COND, jumpTab[i]);
currentBlock->SetCondKindAndTarget(jumpTab[i]);

// Now, build the conditional statement for the current case that is
// being evaluated:
Expand Down Expand Up @@ -1312,7 +1312,7 @@ bool Lowering::TryLowerSwitchToBitTest(
{
// GenCondition::C generates JC so we jump to bbCase1 when the bit is set
bbSwitchCondition = GenCondition::C;
bbSwitch->SetKindAndTarget(BBJ_COND, bbCase1);
bbSwitch->SetCondKindAndTarget(bbCase1);

comp->fgAddRefPred(bbCase0, bbSwitch);
comp->fgAddRefPred(bbCase1, bbSwitch);
Expand All @@ -1323,7 +1323,7 @@ bool Lowering::TryLowerSwitchToBitTest(

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

comp->fgAddRefPred(bbCase0, bbSwitch);
comp->fgAddRefPred(bbCase1, 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)
// bbj_cond(true)
//
gtReverseCond(condExpr);
condBlock->SetKindAndTarget(BBJ_COND, elseBlock);
condBlock->SetCondKindAndTarget(elseBlock);

thenBlock = fgNewBBafter(BBJ_ALWAYS, condBlock, true, remainderBlock);
thenBlock->SetFlags(propagateFlagsToAll);
Expand All @@ -14929,7 +14929,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
// bbj_cond(true)
//
gtReverseCond(condExpr);
condBlock->SetKindAndTarget(BBJ_COND, remainderBlock);
condBlock->SetCondKindAndTarget(remainderBlock);
fgAddRefPred(remainderBlock, condBlock);
// Since we have no false expr, use the one we'd already created.
thenBlock = elseBlock;
Expand All @@ -14945,7 +14945,7 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
// +-->------------+
// bbj_cond(true)
//
condBlock->SetKindAndTarget(BBJ_COND, remainderBlock);
condBlock->SetCondKindAndTarget(remainderBlock);
fgAddRefPred(remainderBlock, condBlock);

elseBlock->inheritWeightPercentage(condBlock, 50);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2846,7 +2846,7 @@ void Compiler::optCopyBlkDest(BasicBlock* from, BasicBlock* to)
to->SetEhfKindAndTarget(new (this, CMK_BasicBlock) BBehfDesc(this, from->GetEhfTarget()));
break;
case BBJ_COND:
to->SetKindAndTarget(BBJ_COND, from->GetTrueTarget());
to->SetCondKindAndTarget(from->GetTrueTarget());
break;
default:
to->SetKindAndTarget(from->GetKind(), from->GetTarget());
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->SetKindAndTarget(BBJ_COND, remainderBlock);
block->SetCondKindAndTarget(remainderBlock);
block->SetFlags(BBF_INTERNAL);

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