Skip to content

Commit

Permalink
JIT: Make BasicBlock::bbJumpKind private (dotnet#92908)
Browse files Browse the repository at this point in the history
This is the beginning of a larger effort to disallow the use of BBJ_NONE (reserved for basic blocks that fall through) before the current method's block layout is finalized.
  • Loading branch information
amanasifkhalid authored Oct 4, 2023
1 parent 76b63a3 commit 1d352cb
Show file tree
Hide file tree
Showing 46 changed files with 503 additions and 491 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5260,7 +5260,7 @@ class AssertionPropFlowCallback
{
ASSERT_TP pAssertionOut;

if (predBlock->bbJumpKind == BBJ_COND && (predBlock->bbJumpDest == block))
if (predBlock->KindIs(BBJ_COND) && (predBlock->bbJumpDest == block))
{
pAssertionOut = mJumpDestOut[predBlock->bbNum];

Expand Down Expand Up @@ -5460,7 +5460,7 @@ ASSERT_TP* Compiler::optComputeAssertionGen()

printf(FMT_BB " valueGen = ", block->bbNum);
optPrintAssertionIndices(block->bbAssertionGen);
if (block->bbJumpKind == BBJ_COND)
if (block->KindIs(BBJ_COND))
{
printf(" => " FMT_BB " valueGen = ", block->bbJumpDest->bbNum);
optPrintAssertionIndices(jumpDestGen[block->bbNum]);
Expand Down Expand Up @@ -6020,7 +6020,7 @@ PhaseStatus Compiler::optAssertionPropMain()
printf(FMT_BB ":\n", block->bbNum);
optDumpAssertionIndices(" in = ", block->bbAssertionIn, "\n");
optDumpAssertionIndices(" out = ", block->bbAssertionOut, "\n");
if (block->bbJumpKind == BBJ_COND)
if (block->KindIs(BBJ_COND))
{
printf(" " FMT_BB " = ", block->bbJumpDest->bbNum);
optDumpAssertionIndices(bbJtrueAssertionOut[block->bbNum], "\n");
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,7 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)

/* Record the jump kind in the block */

block->bbJumpKind = jumpKind;
block->SetBBJumpKind(jumpKind DEBUG_ARG(this));

if (jumpKind == BBJ_THROW)
{
Expand Down Expand Up @@ -1499,9 +1499,9 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)
bool BasicBlock::isBBCallAlwaysPair() const
{
#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
if (this->bbJumpKind == BBJ_CALLFINALLY)
if (this->KindIs(BBJ_CALLFINALLY))
#else
if ((this->bbJumpKind == BBJ_CALLFINALLY) && !(this->bbFlags & BBF_RETLESS_CALL))
if (this->KindIs(BBJ_CALLFINALLY) && !(this->bbFlags & BBF_RETLESS_CALL))
#endif
{
#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
Expand All @@ -1510,7 +1510,7 @@ bool BasicBlock::isBBCallAlwaysPair() const
#endif
// Some asserts that the next block is a BBJ_ALWAYS of the proper form.
assert(this->bbNext != nullptr);
assert(this->bbNext->bbJumpKind == BBJ_ALWAYS);
assert(this->bbNext->KindIs(BBJ_ALWAYS));
assert(this->bbNext->bbFlags & BBF_KEEP_BBJ_ALWAYS);
assert(this->bbNext->isEmpty());

Expand Down
20 changes: 19 additions & 1 deletion src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,26 @@ struct BasicBlock : private LIR::Range
// a block corresponding to an exit from the try of a try/finally.
bool isBBCallAlwaysPairTail() const;

private:
BBjumpKinds bbJumpKind; // jump (if any) at the end of this block

public:
BBjumpKinds GetBBJumpKind() const
{
return bbJumpKind;
}

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

/* The following union describes the jump target(s) of this block */
union {
unsigned bbJumpOffs; // PC offset (temporary only)
Expand Down Expand Up @@ -1556,7 +1574,7 @@ inline BBArrayIterator BBSwitchTargetList::end() const
inline BasicBlock::BBSuccList::BBSuccList(const BasicBlock* block)
{
assert(block != nullptr);
switch (block->bbJumpKind)
switch (block->GetBBJumpKind())
{
case BBJ_THROW:
case BBJ_RETURN:
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegenarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
assert(block->isBBCallAlwaysPair());

assert(block->bbNext != NULL);
assert(block->bbNext->bbJumpKind == BBJ_ALWAYS);
assert(block->bbNext->KindIs(BBJ_ALWAYS));
assert(block->bbNext->bbJumpDest != NULL);
assert(block->bbNext->bbJumpDest->bbFlags & BBF_FINALLY_TARGET);

Expand Down Expand Up @@ -630,7 +630,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode)
//
void CodeGen::genJumpTable(GenTree* treeNode)
{
noway_assert(compiler->compCurBB->bbJumpKind == BBJ_SWITCH);
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
assert(treeNode->OperGet() == GT_JMPTABLE);

unsigned jumpCount = compiler->compCurBB->bbJumpSwt->bbsCount;
Expand Down Expand Up @@ -1294,7 +1294,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3749,7 +3749,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode)
// emits the table and an instruction to get the address of the first element
void CodeGen::genJumpTable(GenTree* treeNode)
{
noway_assert(compiler->compCurBB->bbJumpKind == BBJ_SWITCH);
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
assert(treeNode->OperGet() == GT_JMPTABLE);

unsigned jumpCount = compiler->compCurBB->bbJumpSwt->bbsCount;
Expand Down Expand Up @@ -4650,7 +4650,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
Expand Down Expand Up @@ -4841,7 +4841,7 @@ void CodeGen::genCodeForSelect(GenTreeOp* tree)
//
void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

GenTree* op1 = tree->gtGetOp1();
GenTree* op2 = tree->gtGetOp2();
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5515,7 +5515,7 @@ void CodeGen::genFnEpilog(BasicBlock* block)
{
SetHasTailCalls(true);

noway_assert(block->bbJumpKind == BBJ_RETURN);
noway_assert(block->KindIs(BBJ_RETURN));
noway_assert(block->GetFirstLIRNode() != nullptr);

/* figure out what jump we have */
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ void CodeGen::genMarkLabelsForCodegen()

for (BasicBlock* const block : compiler->Blocks())
{
switch (block->bbJumpKind)
switch (block->GetBBJumpKind())
{
case BBJ_ALWAYS: // This will also handle the BBJ_ALWAYS of a BBJ_CALLFINALLY/BBJ_ALWAYS pair.
case BBJ_COND:
Expand Down Expand Up @@ -2256,7 +2256,7 @@ void CodeGen::genReportEH()
{
for (BasicBlock* const block : compiler->Blocks())
{
if (block->bbJumpKind == BBJ_CALLFINALLY)
if (block->KindIs(BBJ_CALLFINALLY))
{
++clonedFinallyCount;
}
Expand Down Expand Up @@ -2582,7 +2582,7 @@ void CodeGen::genReportEH()
unsigned reportedClonedFinallyCount = 0;
for (BasicBlock* const block : compiler->Blocks())
{
if (block->bbJumpKind == BBJ_CALLFINALLY)
if (block->KindIs(BBJ_CALLFINALLY))
{
UNATIVE_OFFSET hndBeg, hndEnd;

Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ void CodeGen::genCodeForBBlist()
//
// Note: We need to have set compCurBB before calling emitAddLabel
//
if ((block->bbPrev != nullptr) && (block->bbPrev->bbJumpKind == BBJ_COND) &&
if ((block->bbPrev != nullptr) && block->bbPrev->KindIs(BBJ_COND) &&
(block->bbWeight != block->bbPrev->bbWeight))
{
JITDUMP("Adding label due to BB weight difference: BBJ_COND " FMT_BB " with weight " FMT_WT
Expand Down Expand Up @@ -619,7 +619,7 @@ void CodeGen::genCodeForBBlist()
{
// We only need the NOP if we're not going to generate any more code as part of the block end.

switch (block->bbJumpKind)
switch (block->GetBBJumpKind())
{
case BBJ_ALWAYS:
case BBJ_THROW:
Expand Down Expand Up @@ -662,7 +662,7 @@ void CodeGen::genCodeForBBlist()

/* Do we need to generate a jump or return? */

switch (block->bbJumpKind)
switch (block->GetBBJumpKind())
{
case BBJ_RETURN:
genExitCode(block);
Expand Down Expand Up @@ -812,10 +812,10 @@ void CodeGen::genCodeForBBlist()
assert(ShouldAlignLoops());
assert(!block->isBBCallAlwaysPairTail());
#if FEATURE_EH_CALLFINALLY_THUNKS
assert(block->bbJumpKind != BBJ_CALLFINALLY);
assert(!block->KindIs(BBJ_CALLFINALLY));
#endif // FEATURE_EH_CALLFINALLY_THUNKS

GetEmitter()->emitLoopAlignment(DEBUG_ARG1(block->bbJumpKind == BBJ_ALWAYS));
GetEmitter()->emitLoopAlignment(DEBUG_ARG1(block->KindIs(BBJ_ALWAYS)));
}

if ((block->bbNext != nullptr) && (block->bbNext->isLoopAlign()))
Expand Down Expand Up @@ -2615,7 +2615,7 @@ void CodeGen::genStoreLongLclVar(GenTree* treeNode)
//
void CodeGen::genCodeForJcc(GenTreeCC* jcc)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));
assert(jcc->OperIs(GT_JCC));

inst_JCC(jcc->gtCondition, compiler->compCurBB->bbJumpDest);
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ void CodeGen::genFnEpilog(BasicBlock* block)
{
SetHasTailCalls(true);

noway_assert(block->bbJumpKind == BBJ_RETURN);
noway_assert(block->KindIs(BBJ_RETURN));
noway_assert(block->GetFirstLIRNode() != nullptr);

/* figure out what jump we have */
Expand Down Expand Up @@ -2932,7 +2932,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode)
// emits the table and an instruction to get the address of the first element
void CodeGen::genJumpTable(GenTree* treeNode)
{
noway_assert(compiler->compCurBB->bbJumpKind == BBJ_SWITCH);
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
assert(treeNode->OperGet() == GT_JMPTABLE);

unsigned jumpCount = compiler->compCurBB->bbJumpSwt->bbsCount;
Expand Down Expand Up @@ -4140,7 +4140,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
// A GT_JCMP node is created for an integer-comparison's conditional branch.
void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

assert(tree->OperIs(GT_JCMP));
assert(!varTypeIsFloating(tree));
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegenriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ void CodeGen::genFnEpilog(BasicBlock* block)
{
SetHasTailCalls(true);

noway_assert(block->bbJumpKind == BBJ_RETURN);
noway_assert(block->KindIs(BBJ_RETURN));
noway_assert(block->GetFirstLIRNode() != nullptr);

/* figure out what jump we have */
Expand Down Expand Up @@ -2578,7 +2578,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode)
// emits the table and an instruction to get the address of the first element
void CodeGen::genJumpTable(GenTree* treeNode)
{
noway_assert(compiler->compCurBB->bbJumpKind == BBJ_SWITCH);
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
assert(treeNode->OperGet() == GT_JMPTABLE);

unsigned jumpCount = compiler->compCurBB->bbJumpSwt->bbsCount;
Expand Down Expand Up @@ -3784,7 +3784,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
//
void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

assert(tree->OperIs(GT_JCMP));
assert(!varTypeIsFloating(tree));
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ void CodeGen::genEHFinallyOrFilterRet(BasicBlock* block)
}
else
{
assert(block->bbJumpKind == BBJ_EHFILTERRET);
assert(block->KindIs(BBJ_EHFILTERRET));

// The return value has already been computed.
instGen_Return(0);
Expand Down Expand Up @@ -1445,7 +1445,7 @@ void CodeGen::genCodeForCompare(GenTreeOp* tree)
//
void CodeGen::genCodeForJTrue(GenTreeOp* jtrue)
{
assert(compiler->compCurBB->bbJumpKind == BBJ_COND);
assert(compiler->compCurBB->KindIs(BBJ_COND));

GenTree* op = jtrue->gtGetOp1();
regNumber reg = genConsumeReg(op);
Expand Down Expand Up @@ -4267,7 +4267,7 @@ void CodeGen::genTableBasedSwitch(GenTree* treeNode)
// emits the table and an instruction to get the address of the first element
void CodeGen::genJumpTable(GenTree* treeNode)
{
noway_assert(compiler->compCurBB->bbJumpKind == BBJ_SWITCH);
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
assert(treeNode->OperGet() == GT_JMPTABLE);

unsigned jumpCount = compiler->compCurBB->bbJumpSwt->bbsCount;
Expand Down Expand Up @@ -10245,7 +10245,7 @@ void CodeGen::genFnEpilog(BasicBlock* block)

if (jmpEpilog)
{
noway_assert(block->bbJumpKind == BBJ_RETURN);
noway_assert(block->KindIs(BBJ_RETURN));
noway_assert(block->GetFirstLIRNode());

// figure out what jump we have
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5275,7 +5275,7 @@ PhaseStatus Compiler::placeLoopAlignInstructions()
}

// If there is an unconditional jump (which is not part of callf/always pair)
if (opts.compJitHideAlignBehindJmp && (block->bbJumpKind == BBJ_ALWAYS) && !block->isBBCallAlwaysPairTail())
if (opts.compJitHideAlignBehindJmp && block->KindIs(BBJ_ALWAYS) && !block->isBBCallAlwaysPairTail())
{
// Track the lower weight blocks
if (block->bbWeight < minBlockSoFar)
Expand All @@ -5300,7 +5300,7 @@ PhaseStatus Compiler::placeLoopAlignInstructions()
bool unmarkedLoopAlign = false;

#if FEATURE_EH_CALLFINALLY_THUNKS
if (block->bbJumpKind == BBJ_CALLFINALLY)
if (block->KindIs(BBJ_CALLFINALLY))
{
// It must be a retless BBJ_CALLFINALLY if we get here.
assert(!block->isBBCallAlwaysPair());
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)

for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->bbNext)
{
if ((bcall->bbJumpKind != BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
if (!bcall->KindIs(BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
{
continue;
}
Expand All @@ -649,7 +649,7 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)

for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->bbNext)
{
if ((bcall->bbJumpKind != BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
if (!bcall->KindIs(BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
{
continue;
}
Expand Down Expand Up @@ -769,7 +769,7 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func)

for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->bbNext)
{
if ((bcall->bbJumpKind != BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
if (!bcall->KindIs(BBJ_CALLFINALLY) || (bcall->bbJumpDest != finBeg))
{
continue;
}
Expand Down Expand Up @@ -3125,7 +3125,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block)
return false;
}

if (!(block->bbFlags & BBF_INTERNAL) || block->bbJumpKind != BBJ_THROW)
if (!(block->bbFlags & BBF_INTERNAL) || !block->KindIs(BBJ_THROW))
{
return false;
}
Expand Down Expand Up @@ -3224,7 +3224,7 @@ inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
fgRemoveBlockAsPred(block);

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

// Any block with a throw is rare
block->bbSetRunRarely();
Expand All @@ -3236,7 +3236,7 @@ inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
if (isCallAlwaysPair)
{
BasicBlock* leaveBlk = block->bbNext;
noway_assert(leaveBlk->bbJumpKind == BBJ_ALWAYS);
noway_assert(leaveBlk->KindIs(BBJ_ALWAYS));

// leaveBlk is now unreachable, so scrub the pred lists.
leaveBlk->bbFlags &= ~BBF_DONT_REMOVE;
Expand Down
Loading

0 comments on commit 1d352cb

Please sign in to comment.