Skip to content

Commit

Permalink
JIT: Clean up BasicBlock interface (#95153)
Browse files Browse the repository at this point in the history 
* Rename bbNewBasicBlock

* Refactor BasicBlock::SetJumpKindAndTarget
  • Loading branch information
@amanasifkhalid
amanasifkhalid committed Nov 23, 2023
1 parent 8c1399d commit 4fa760f
Show file tree
Hide file tree
Showing 20 changed files with 118 additions and 132 deletions.
14 changes: 7 additions & 7 deletions src/coreclr/jit/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1514,7 +1514,7 @@ bool BasicBlock::endsWithTailCallConvertibleToLoop(Compiler* comp, GenTree** tai
* Allocate a basic block but don't append it to the current BB list.
*/

BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler)
BasicBlock* BasicBlock::New(Compiler* compiler)
{
BasicBlock* block;

Expand Down Expand Up @@ -1615,9 +1615,9 @@ BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler)
return block;
}

BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest /* = nullptr */)
BasicBlock* BasicBlock::New(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest /* = nullptr */)
{
BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler);
BasicBlock* block = BasicBlock::New(compiler);

// In some cases, we don't know a block's jump target during initialization, so don't check the jump kind/target
// yet.
Expand All @@ -1633,17 +1633,17 @@ BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind
return block;
}

BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBswtDesc* jumpSwt)
BasicBlock* BasicBlock::New(Compiler* compiler, BBswtDesc* jumpSwt)
{
BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler);
BasicBlock* block = BasicBlock::New(compiler);
block->bbJumpKind = BBJ_SWITCH;
block->bbJumpSwt = jumpSwt;
return block;
}

BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, unsigned jumpOffs)
BasicBlock* BasicBlock::New(Compiler* compiler, BBjumpKinds jumpKind, unsigned jumpOffs)
{
BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler);
BasicBlock* block = BasicBlock::New(compiler);
block->bbJumpKind = jumpKind;
block->bbJumpOffs = jumpOffs;
return block;
Expand Down
24 changes: 5 additions & 19 deletions src/coreclr/jit/block.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,10 +539,10 @@ struct BasicBlock : private LIR::Range
};

public:
static BasicBlock* bbNewBasicBlock(Compiler* compiler);
static BasicBlock* bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr);
static BasicBlock* bbNewBasicBlock(Compiler* compiler, BBswtDesc* jumpSwt);
static BasicBlock* bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, unsigned jumpOffs);
static BasicBlock* New(Compiler* compiler);
static BasicBlock* New(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr);
static BasicBlock* New(Compiler* compiler, BBswtDesc* jumpSwt);
static BasicBlock* New(Compiler* compiler, BBjumpKinds jumpKind, unsigned jumpOffs);

BBjumpKinds GetJumpKind() const
{
Expand Down Expand Up @@ -637,29 +637,15 @@ struct BasicBlock : private LIR::Range
assert(HasJump());
}

void SetJumpKindAndTarget(BBjumpKinds jumpKind, BasicBlock* jumpDest DEBUG_ARG(Compiler* compiler))
void SetJumpKindAndTarget(BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr)
{
#ifdef DEBUG
// BBJ_NONE should only be assigned when optimizing jumps in Compiler::optOptimizeLayout
// TODO: Change assert to check if compiler is in appropriate optimization phase to use BBJ_NONE
//
// (right now, this assertion does the null check to avoid unused variable warnings)
assert((jumpKind != BBJ_NONE) || (compiler != nullptr));
#endif // DEBUG

bbJumpKind = jumpKind;
bbJumpDest = jumpDest;

// If bbJumpKind indicates this block has a jump, bbJumpDest cannot be null
assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE));
}

void SetJumpKindAndTarget(BBjumpKinds jumpKind DEBUG_ARG(Compiler* compiler))
{
BasicBlock* jumpDest = nullptr;
SetJumpKindAndTarget(jumpKind, jumpDest DEBUG_ARG(compiler));
}

bool HasJump() const
{
return (bbJumpDest != nullptr);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegencommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ BasicBlock* CodeGen::genCreateTempLabel()
#endif

// Label doesn't need a jump kind
BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler);
BasicBlock* block = BasicBlock::New(compiler);

#ifdef DEBUG
compiler->fgSafeBasicBlockCreation = false;
Expand Down
34 changes: 17 additions & 17 deletions src/coreclr/jit/fgbasic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ bool Compiler::fgEnsureFirstBBisScratch()

assert(fgFirstBBScratch == nullptr);

BasicBlock* block = BasicBlock::bbNewBasicBlock(this, BBJ_NONE);
BasicBlock* block = BasicBlock::New(this, BBJ_NONE);

if (fgFirstBB != nullptr)
{
Expand Down Expand Up @@ -366,7 +366,7 @@ void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
fgRemoveBlockAsPred(block);

// Update jump kind after the scrub.
block->SetJumpKindAndTarget(BBJ_THROW DEBUG_ARG(this));
block->SetJumpKindAndTarget(BBJ_THROW);

// Any block with a throw is rare
block->bbSetRunRarely();
Expand Down Expand Up @@ -2947,7 +2947,7 @@ void Compiler::fgLinkBasicBlocks()
BasicBlock* const jumpDest = fgLookupBB(curBBdesc->GetJumpOffs());
// Redundantly use SetJumpKindAndTarget() instead of SetJumpDest() just this once,
// so we don't break the HasJump() invariant of SetJumpDest().
curBBdesc->SetJumpKindAndTarget(curBBdesc->GetJumpKind(), jumpDest DEBUG_ARG(this));
curBBdesc->SetJumpKindAndTarget(curBBdesc->GetJumpKind(), jumpDest);
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc, oldEdge);

if (curBBdesc->GetJumpDest()->bbNum <= curBBdesc->bbNum)
Expand Down Expand Up @@ -3490,18 +3490,18 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
switch (jmpKind)
{
case BBJ_SWITCH:
curBBdesc = BasicBlock::bbNewBasicBlock(this, swtDsc);
curBBdesc = BasicBlock::New(this, swtDsc);
break;

case BBJ_COND:
case BBJ_ALWAYS:
case BBJ_LEAVE:
noway_assert(jmpAddr != DUMMY_INIT(BAD_IL_OFFSET));
curBBdesc = BasicBlock::bbNewBasicBlock(this, jmpKind, jmpAddr);
curBBdesc = BasicBlock::New(this, jmpKind, jmpAddr);
break;

default:
curBBdesc = BasicBlock::bbNewBasicBlock(this, jmpKind);
curBBdesc = BasicBlock::New(this, jmpKind);
break;
}

Expand Down Expand Up @@ -4195,7 +4195,7 @@ void Compiler::fgFixEntryFlowForOSR()
fgEnsureFirstBBisScratch();
assert(fgFirstBB->KindIs(BBJ_NONE));
fgRemoveRefPred(fgFirstBB->Next(), fgFirstBB);
fgFirstBB->SetJumpKindAndTarget(BBJ_ALWAYS, fgOSREntryBB DEBUG_ARG(this));
fgFirstBB->SetJumpKindAndTarget(BBJ_ALWAYS, fgOSREntryBB);
FlowEdge* const edge = fgAddRefPred(fgOSREntryBB, fgFirstBB);
edge->setLikelihood(1.0);

Expand Down Expand Up @@ -4745,7 +4745,7 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr)
if (!curr->KindIs(BBJ_SWITCH))
{
// Start the new block with no refs. When we set the preds below, this will get updated correctly.
newBlock = BasicBlock::bbNewBasicBlock(this, curr->GetJumpKind(), curr->GetJumpDest());
newBlock = BasicBlock::New(this, curr->GetJumpKind(), curr->GetJumpDest());
newBlock->bbRefs = 0;

for (BasicBlock* const succ : curr->Succs(this))
Expand All @@ -4761,7 +4761,7 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr)
else
{
// Start the new block with no refs. When we set the preds below, this will get updated correctly.
newBlock = BasicBlock::bbNewBasicBlock(this, curr->GetJumpSwt());
newBlock = BasicBlock::New(this, curr->GetJumpSwt());
newBlock->bbRefs = 0;

// In the case of a switch statement there's more complicated logic in order to wire up the predecessor lists
Expand Down Expand Up @@ -4802,7 +4802,7 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr)
curr->bbFlags &= ~(BBF_HAS_JMP | BBF_RETLESS_CALL);

// Default to fallthru, and add the arc for that.
curr->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this));
curr->SetJumpKindAndTarget(BBJ_NONE);
fgAddRefPred(newBlock, curr);

return newBlock;
Expand Down Expand Up @@ -5252,7 +5252,7 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
// Note that we don't do it if bPrev follows a BBJ_CALLFINALLY block (BBF_KEEP_BBJ_ALWAYS),
// because that would violate our invariant that BBJ_CALLFINALLY blocks are followed by
// BBJ_ALWAYS blocks.
bPrev->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this));
bPrev->SetJumpKindAndTarget(BBJ_NONE);
}

// If this is the first Cold basic block update fgFirstColdBlock
Expand Down Expand Up @@ -5451,7 +5451,7 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
if (block->KindIs(BBJ_ALWAYS))
{
/* bPrev now becomes a BBJ_ALWAYS */
bPrev->SetJumpKindAndTarget(BBJ_ALWAYS, succBlock DEBUG_ARG(this));
bPrev->SetJumpKindAndTarget(BBJ_ALWAYS, succBlock);
}
break;

Expand Down Expand Up @@ -5521,7 +5521,7 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
if ((bPrev == fgFirstBB) || !bPrev->isBBCallAlwaysPairTail())
{
// It's safe to change the jump type
bPrev->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this));
bPrev->SetJumpKindAndTarget(BBJ_NONE);
}
}
break;
Expand Down Expand Up @@ -5569,7 +5569,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
switch (bSrc->GetJumpKind())
{
case BBJ_NONE:
bSrc->SetJumpKindAndTarget(BBJ_ALWAYS, bDst DEBUG_ARG(this));
bSrc->SetJumpKindAndTarget(BBJ_ALWAYS, bDst);
JITDUMP("Block " FMT_BB " ended with a BBJ_NONE, Changed to an unconditional jump to " FMT_BB "\n",
bSrc->bbNum, bSrc->GetJumpDest()->bbNum);
break;
Expand Down Expand Up @@ -5648,7 +5648,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
if (bSrc->KindIs(BBJ_ALWAYS) && !(bSrc->bbFlags & BBF_KEEP_BBJ_ALWAYS) && bSrc->HasJump() &&
bSrc->JumpsToNext())
{
bSrc->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this));
bSrc->SetJumpKindAndTarget(BBJ_NONE);
JITDUMP("Changed an unconditional jump from " FMT_BB " to the next block " FMT_BB
" into a BBJ_NONE block\n",
bSrc->bbNum, bSrc->Next()->bbNum);
Expand Down Expand Up @@ -6252,7 +6252,7 @@ BasicBlock* Compiler::fgNewBBbefore(BBjumpKinds jumpKind,
{
// Create a new BasicBlock and chain it in

BasicBlock* newBlk = BasicBlock::bbNewBasicBlock(this, jumpKind, jumpDest);
BasicBlock* newBlk = BasicBlock::New(this, jumpKind, jumpDest);
newBlk->bbFlags |= BBF_INTERNAL;

fgInsertBBbefore(block, newBlk);
Expand Down Expand Up @@ -6294,7 +6294,7 @@ BasicBlock* Compiler::fgNewBBafter(BBjumpKinds jumpKind,
{
// Create a new BasicBlock and chain it in

BasicBlock* newBlk = BasicBlock::bbNewBasicBlock(this, jumpKind, jumpDest);
BasicBlock* newBlk = BasicBlock::New(this, jumpKind, jumpDest);
newBlk->bbFlags |= BBF_INTERNAL;

fgInsertBBafter(block, newBlk);
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/fgehopt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()

noway_assert(leaveBlock->KindIs(BBJ_ALWAYS));

currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, postTryFinallyBlock DEBUG_ARG(this));
currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, postTryFinallyBlock);

// Ref count updates.
fgAddRefPred(postTryFinallyBlock, currentBlock);
Expand Down Expand Up @@ -528,7 +528,7 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
GenTree* finallyRetExpr = finallyRet->GetRootNode();
assert(finallyRetExpr->gtOper == GT_RETFILT);
fgRemoveStmt(block, finallyRet);
block->SetJumpKindAndTarget(BBJ_ALWAYS, continuation DEBUG_ARG(this));
block->SetJumpKindAndTarget(BBJ_ALWAYS, continuation);
fgAddRefPred(continuation, block);
fgRemoveRefPred(leave, block);
}
Expand Down Expand Up @@ -1112,7 +1112,7 @@ PhaseStatus Compiler::fgCloneFinally()
GenTree* finallyRetExpr = finallyRet->GetRootNode();
assert(finallyRetExpr->gtOper == GT_RETFILT);
fgRemoveStmt(newBlock, finallyRet);
newBlock->SetJumpKindAndTarget(BBJ_ALWAYS, normalCallFinallyReturn DEBUG_ARG(this));
newBlock->SetJumpKindAndTarget(BBJ_ALWAYS, normalCallFinallyReturn);

fgAddRefPred(normalCallFinallyReturn, newBlock);
}
Expand Down Expand Up @@ -1152,7 +1152,7 @@ PhaseStatus Compiler::fgCloneFinally()

// This call returns to the expected spot, so
// retarget it to branch to the clone.
currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, firstCloneBlock DEBUG_ARG(this));
currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, firstCloneBlock);

// Ref count updates.
fgAddRefPred(firstCloneBlock, currentBlock);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/fginline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
else
{
m_compiler->fgRemoveRefPred(block->GetJumpDest(), block);
block->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(m_compiler));
block->SetJumpKindAndTarget(BBJ_NONE);
}
}
}
Expand Down Expand Up @@ -1532,13 +1532,13 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
if (block->IsLast())
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_NONE\n", block->bbNum);
block->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this));
block->SetJumpKindAndTarget(BBJ_NONE);
}
else
{
JITDUMP("\nConvert bbJumpKind of " FMT_BB " to BBJ_ALWAYS to bottomBlock " FMT_BB "\n",
block->bbNum, bottomBlock->bbNum);
block->SetJumpKindAndTarget(BBJ_ALWAYS, bottomBlock DEBUG_ARG(this));
block->SetJumpKindAndTarget(BBJ_ALWAYS, bottomBlock);
}

fgAddRefPred(bottomBlock, block);
Expand Down
Loading

0 comments on commit 4fa760f

Please sign in to comment.