From fe043b489f69a888ad2efd97fbc177c1f25e541d Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Tue, 24 Oct 2023 18:13:32 -0400 Subject: [PATCH] JIT: Allow initializing blocks without jump targets (#93928) ...and remove BasicBlock::bbTempJumpDest, per discussion in #93415. We still assert the jump target is set appropriately whenever it is read/written, and in the majority of cases, we still initialize blocks with their jump kind and target set simultaneously. This change improves usability for the few edge cases (like in Compiler::impImportLeave) where a block's jump target isn't known at initialization. --- src/coreclr/jit/block.cpp | 69 +++++++++++++++-------------- src/coreclr/jit/block.h | 15 ++----- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/compiler.h | 9 ++-- src/coreclr/jit/fgbasic.cpp | 32 +++++++++---- src/coreclr/jit/fgdiagnostic.cpp | 3 +- src/coreclr/jit/fgopt.cpp | 2 +- src/coreclr/jit/flowgraph.cpp | 2 +- src/coreclr/jit/helperexpansion.cpp | 17 +++---- src/coreclr/jit/importer.cpp | 33 +++++++------- src/coreclr/jit/jiteh.cpp | 8 ++-- src/coreclr/jit/optimizer.cpp | 4 +- src/coreclr/jit/ssabuilder.cpp | 2 +- 13 files changed, 100 insertions(+), 98 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index 1984d2b7a6a99..efa5be945ec7a 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -28,10 +28,6 @@ size_t BasicBlock::s_Count; // The max # of tree nodes in any BB /* static */ unsigned BasicBlock::s_nMaxTrees; - -// Temporary target to initialize blocks with jumps -/* static */ -BasicBlock BasicBlock::bbTempJumpDest; #endif // DEBUG #ifdef DEBUG @@ -1449,14 +1445,14 @@ bool BasicBlock::endsWithTailCallConvertibleToLoop(Compiler* comp, GenTree** tai * Allocate a basic block but don't append it to the current BB list. */ -BasicBlock* Compiler::bbNewBasicBlock() +BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler) { BasicBlock* block; /* Allocate the block descriptor and zero it out */ - assert(fgSafeBasicBlockCreation); + assert(compiler->fgSafeBasicBlockCreation); - block = new (this, CMK_BasicBlock) BasicBlock; + block = new (compiler, CMK_BasicBlock) BasicBlock; #if MEASURE_BLOCK_SIZE BasicBlock::s_Count += 1; @@ -1465,7 +1461,7 @@ BasicBlock* Compiler::bbNewBasicBlock() #ifdef DEBUG // fgLookupBB() is invalid until fgInitBBLookup() is called again. - fgBBs = (BasicBlock**)0xCDCD; + compiler->fgInvalidateBBLookup(); #endif // TODO-Throughput: The following memset is pretty expensive - do something else? @@ -1479,15 +1475,15 @@ BasicBlock* Compiler::bbNewBasicBlock() block->bbCodeOffsEnd = BAD_IL_OFFSET; #ifdef DEBUG - block->bbID = compBasicBlockID++; + block->bbID = compiler->compBasicBlockID++; #endif /* Give the block a number, set the ancestor count and weight */ - ++fgBBcount; - block->bbNum = ++fgBBNumMax; + ++compiler->fgBBcount; + block->bbNum = ++compiler->fgBBNumMax; - if (compRationalIRForm) + if (compiler->compRationalIRForm) { block->bbFlags |= BBF_IS_LIR; } @@ -1501,7 +1497,7 @@ BasicBlock* Compiler::bbNewBasicBlock() block->bbEntryState = nullptr; #ifdef DEBUG - if (verbose) + if (compiler->verbose) { printf("New Basic Block %s created.\n", block->dspToString()); } @@ -1510,21 +1506,21 @@ BasicBlock* Compiler::bbNewBasicBlock() // We will give all the blocks var sets after the number of tracked variables // is determined and frozen. After that, if we dynamically create a basic block, // we will initialize its var sets. - if (fgBBVarSetsInited) + if (compiler->fgBBVarSetsInited) { - VarSetOps::AssignNoCopy(this, block->bbVarUse, VarSetOps::MakeEmpty(this)); - VarSetOps::AssignNoCopy(this, block->bbVarDef, VarSetOps::MakeEmpty(this)); - VarSetOps::AssignNoCopy(this, block->bbLiveIn, VarSetOps::MakeEmpty(this)); - VarSetOps::AssignNoCopy(this, block->bbLiveOut, VarSetOps::MakeEmpty(this)); - VarSetOps::AssignNoCopy(this, block->bbScope, VarSetOps::MakeEmpty(this)); + VarSetOps::AssignNoCopy(compiler, block->bbVarUse, VarSetOps::MakeEmpty(compiler)); + VarSetOps::AssignNoCopy(compiler, block->bbVarDef, VarSetOps::MakeEmpty(compiler)); + VarSetOps::AssignNoCopy(compiler, block->bbLiveIn, VarSetOps::MakeEmpty(compiler)); + VarSetOps::AssignNoCopy(compiler, block->bbLiveOut, VarSetOps::MakeEmpty(compiler)); + VarSetOps::AssignNoCopy(compiler, block->bbScope, VarSetOps::MakeEmpty(compiler)); } else { - VarSetOps::AssignNoCopy(this, block->bbVarUse, VarSetOps::UninitVal()); - VarSetOps::AssignNoCopy(this, block->bbVarDef, VarSetOps::UninitVal()); - VarSetOps::AssignNoCopy(this, block->bbLiveIn, VarSetOps::UninitVal()); - VarSetOps::AssignNoCopy(this, block->bbLiveOut, VarSetOps::UninitVal()); - VarSetOps::AssignNoCopy(this, block->bbScope, VarSetOps::UninitVal()); + VarSetOps::AssignNoCopy(compiler, block->bbVarUse, VarSetOps::UninitVal()); + VarSetOps::AssignNoCopy(compiler, block->bbVarDef, VarSetOps::UninitVal()); + VarSetOps::AssignNoCopy(compiler, block->bbLiveIn, VarSetOps::UninitVal()); + VarSetOps::AssignNoCopy(compiler, block->bbLiveOut, VarSetOps::UninitVal()); + VarSetOps::AssignNoCopy(compiler, block->bbScope, VarSetOps::UninitVal()); } block->bbMemoryUse = emptyMemoryKindSet; @@ -1550,10 +1546,15 @@ BasicBlock* Compiler::bbNewBasicBlock() return block; } -BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind, BasicBlock* jumpDest /* = nullptr */) +BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest /* = nullptr */) { - BasicBlock* block = bbNewBasicBlock(); - block->SetJumpKindAndTarget(jumpKind, jumpDest DEBUG_ARG(this)); + BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler); + + // In some cases, we don't know a block's jump target during initialization, so don't check the jump kind/target + // yet. + // The checks will be done any time the jump kind/target is read or written to after initialization. + block->bbJumpKind = jumpKind; + block->bbJumpDest = jumpDest; if (jumpKind == BBJ_THROW) { @@ -1563,17 +1564,19 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind, BasicBlock* jumpDest return block; } -BasicBlock* Compiler::bbNewBasicBlock(BBswtDesc* jumpSwt) +BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBswtDesc* jumpSwt) { - BasicBlock* block = bbNewBasicBlock(); - block->SetSwitchKindAndTarget(jumpSwt); + BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler); + block->bbJumpKind = BBJ_SWITCH; + block->bbJumpSwt = jumpSwt; return block; } -BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind, unsigned jumpOffs) +BasicBlock* BasicBlock::bbNewBasicBlock(Compiler* compiler, BBjumpKinds jumpKind, unsigned jumpOffs) { - BasicBlock* block = bbNewBasicBlock(); - block->SetJumpKindAndTarget(jumpKind, jumpOffs); + BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler); + block->bbJumpKind = jumpKind; + block->bbJumpOffs = jumpOffs; return block; } diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index f3936955300ac..7d9e30971ce0a 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -538,13 +538,10 @@ struct BasicBlock : private LIR::Range }; public: -#ifdef DEBUG - // When creating a block with a jump, we require its jump kind and target be initialized simultaneously. - // In a few edge cases (for example, in Compiler::impImportLeave), we don't know the jump target at block creation. - // In these cases, temporarily set the jump target to bbTempJumpDest, and update the jump target later. - // We won't check jump targets against bbTempJumpDest in Release builds. - static BasicBlock bbTempJumpDest; -#endif // DEBUG + 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); BBjumpKinds GetJumpKind() const { @@ -633,10 +630,6 @@ struct BasicBlock : private LIR::Range void SetJumpDest(BasicBlock* jumpDest) { - // If bbJumpKind indicates this block has a jump, - // bbJumpDest should have previously been set in SetJumpKindAndTarget(). - assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE)); - // SetJumpKindAndTarget() nulls jumpDest for non-jump kinds, // so don't use SetJumpDest() to null bbJumpDest without updating bbJumpKind. bbJumpDest = jumpDest; diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 83270d95b2564..cc0afca6a7974 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -857,7 +857,7 @@ BasicBlock* CodeGen::genCreateTempLabel() #endif // Label doesn't need a jump kind - BasicBlock* block = compiler->bbNewBasicBlock(); + BasicBlock* block = BasicBlock::bbNewBasicBlock(compiler); #ifdef DEBUG compiler->fgSafeBasicBlockCreation = false; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 12d8476d0aac2..1d387fbcfbe68 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3221,11 +3221,6 @@ class Compiler bool fgSafeBasicBlockCreation; #endif - BasicBlock* bbNewBasicBlock(); - BasicBlock* bbNewBasicBlock(BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr); - BasicBlock* bbNewBasicBlock(BBswtDesc* jumpSwt); - BasicBlock* bbNewBasicBlock(BBjumpKinds jumpKind, unsigned jumpOffs); - /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -5718,6 +5713,10 @@ class Compiler void* pCallBackData = nullptr, bool computeStack = false); +#ifdef DEBUG + void fgInvalidateBBLookup(); +#endif // DEBUG + /************************************************************************** * PROTECTED *************************************************************************/ diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index eb55ef60042b7..e3f7f4f61521b 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -221,7 +221,7 @@ bool Compiler::fgEnsureFirstBBisScratch() assert(fgFirstBBScratch == nullptr); - BasicBlock* block = bbNewBasicBlock(BBJ_NONE); + BasicBlock* block = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); if (fgFirstBB != nullptr) { @@ -802,6 +802,17 @@ BasicBlock* Compiler::fgFirstBlockOfHandler(BasicBlock* block) return ehGetDsc(block->getHndIndex())->ebdHndBeg; } +#ifdef DEBUG +//------------------------------------------------------------------------ +// fgInvalidateBBLookup: In non-Release builds, set fgBBs to a dummy value. +// After calling this, fgInitBBLookup must be called before using fgBBs again. +// +void Compiler::fgInvalidateBBLookup() +{ + fgBBs = (BasicBlock**)0xCDCD; +} +#endif // DEBUG + /***************************************************************************** * * The following helps find a basic block given its PC offset. @@ -3478,18 +3489,18 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F switch (jmpKind) { case BBJ_SWITCH: - curBBdesc = bbNewBasicBlock(swtDsc); + curBBdesc = BasicBlock::bbNewBasicBlock(this, swtDsc); break; case BBJ_COND: case BBJ_ALWAYS: case BBJ_LEAVE: noway_assert(jmpAddr != DUMMY_INIT(BAD_IL_OFFSET)); - curBBdesc = bbNewBasicBlock(jmpKind, jmpAddr); + curBBdesc = BasicBlock::bbNewBasicBlock(this, jmpKind, jmpAddr); break; default: - curBBdesc = bbNewBasicBlock(jmpKind); + curBBdesc = BasicBlock::bbNewBasicBlock(this, jmpKind); break; } @@ -4733,7 +4744,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 = bbNewBasicBlock(curr->GetJumpKind(), curr->GetJumpDest()); + newBlock = BasicBlock::bbNewBasicBlock(this, curr->GetJumpKind(), curr->GetJumpDest()); newBlock->bbRefs = 0; for (BasicBlock* const succ : curr->Succs(this)) @@ -4749,7 +4760,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 = bbNewBasicBlock(curr->GetJumpSwt()); + newBlock = BasicBlock::bbNewBasicBlock(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 @@ -5618,8 +5629,11 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) { // If bSrc is an unconditional branch to the next block // then change it to a BBJ_NONE block + // (It's possible for bSrc's jump destination to not be set yet, + // so check with BasicBlock::HasJump before calling BasicBlock::JumpsToNext) // - if (bSrc->KindIs(BBJ_ALWAYS) && !(bSrc->bbFlags & BBF_KEEP_BBJ_ALWAYS) && bSrc->JumpsToNext()) + if (bSrc->KindIs(BBJ_ALWAYS) && !(bSrc->bbFlags & BBF_KEEP_BBJ_ALWAYS) && bSrc->HasJump() && + bSrc->JumpsToNext()) { bSrc->SetJumpKindAndTarget(BBJ_NONE DEBUG_ARG(this)); JITDUMP("Changed an unconditional jump from " FMT_BB " to the next block " FMT_BB @@ -6226,7 +6240,7 @@ BasicBlock* Compiler::fgNewBBbefore(BBjumpKinds jumpKind, { // Create a new BasicBlock and chain it in - BasicBlock* newBlk = bbNewBasicBlock(jumpKind, jumpDest); + BasicBlock* newBlk = BasicBlock::bbNewBasicBlock(this, jumpKind, jumpDest); newBlk->bbFlags |= BBF_INTERNAL; fgInsertBBbefore(block, newBlk); @@ -6268,7 +6282,7 @@ BasicBlock* Compiler::fgNewBBafter(BBjumpKinds jumpKind, { // Create a new BasicBlock and chain it in - BasicBlock* newBlk = bbNewBasicBlock(jumpKind, jumpDest); + BasicBlock* newBlk = BasicBlock::bbNewBasicBlock(this, jumpKind, jumpDest); newBlk->bbFlags |= BBF_INTERNAL; fgInsertBBafter(block, newBlk); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 65e3cc406eca7..8e8331490043f 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3039,11 +3039,10 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef assert(block->getTryIndex() < compHndBBtabCount); } - // Blocks with these jump kinds must have valid (non-temporary) jump targets + // Blocks with these jump kinds must have non-null jump targets if (block->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE)) { assert(block->HasJump()); - assert(!block->HasJumpTo(&BasicBlock::bbTempJumpDest)); } // A branch or fall-through to a BBJ_CALLFINALLY block must come from the `try` region associated diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 79b098547523b..3efeaf4411d5a 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1661,7 +1661,7 @@ PhaseStatus Compiler::fgPostImportationCleanup() // What follows is similar to fgNewBBInRegion, but we can't call that // here as the oldTryEntry is no longer in the main bb list. - newTryEntry = bbNewBasicBlock(BBJ_NONE); + newTryEntry = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); newTryEntry->bbFlags |= (BBF_IMPORTED | BBF_INTERNAL); newTryEntry->bbRefs = 0; diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index a4e7670ca8ff5..32fb8f457bf51 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2985,7 +2985,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block) /* Allocate a new basic block */ - BasicBlock* newHead = bbNewBasicBlock(BBJ_NONE); + BasicBlock* newHead = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); newHead->bbFlags |= BBF_INTERNAL; newHead->inheritWeight(block); newHead->bbRefs = 0; diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index e00b9f6044b84..4b5809d069552 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -279,9 +279,9 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm nullcheckOp->gtFlags |= GTF_RELOP_JMP_USED; // nullcheckBb conditionally jumps to fallbackBb, but we need to initialize fallbackBb last - // so we can place it after nullcheckBb. So use a temporary jump target for now. - BasicBlock* nullcheckBb = fgNewBBFromTreeAfter(BBJ_COND, prevBb, gtNewOperNode(GT_JTRUE, TYP_VOID, nullcheckOp), - debugInfo DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + // so we can place it after nullcheckBb. So set the jump target later. + BasicBlock* nullcheckBb = + fgNewBBFromTreeAfter(BBJ_COND, prevBb, gtNewOperNode(GT_JTRUE, TYP_VOID, nullcheckOp), debugInfo); // Fallback basic block GenTree* fallbackValueDef = gtNewStoreLclVarNode(rtLookupLcl->GetLclNum(), call); @@ -741,18 +741,15 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement* // maxThreadStaticBlocksCondBB // maxThreadStaticBlocksCondBB conditionally jumps to fallbackBb, but fallbackBb must be initialized last - // so it can be placed after it. So use a temporary jump target for now. - BasicBlock* maxThreadStaticBlocksCondBB = - fgNewBBFromTreeAfter(BBJ_COND, prevBb, tlsValueDef, debugInfo DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + // so it can be placed after it. So set the jump target later. + BasicBlock* maxThreadStaticBlocksCondBB = fgNewBBFromTreeAfter(BBJ_COND, prevBb, tlsValueDef, debugInfo); fgInsertStmtAfter(maxThreadStaticBlocksCondBB, maxThreadStaticBlocksCondBB->firstStmt(), fgNewStmtFromTree(maxThreadStaticBlocksCond)); - // Similarly, give threadStaticBlockNulLCondBB a temporary jump target for now, - // and update it to jump to its real target (fastPathBb) after it is initialized. + // Similarly, set threadStaticBlockNulLCondBB to jump to fastPathBb once the latter exists. BasicBlock* threadStaticBlockNullCondBB = - fgNewBBFromTreeAfter(BBJ_COND, maxThreadStaticBlocksCondBB, threadStaticBlockBaseDef, - debugInfo DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + fgNewBBFromTreeAfter(BBJ_COND, maxThreadStaticBlocksCondBB, threadStaticBlockBaseDef, debugInfo); fgInsertStmtAfter(threadStaticBlockNullCondBB, threadStaticBlockNullCondBB->firstStmt(), fgNewStmtFromTree(threadStaticBlockNullCond)); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 9689ec202a824..5bc0d12cbd650 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4352,8 +4352,8 @@ void Compiler::impImportLeave(BasicBlock* block) // callBlock will call the finally handler callBlock = fgNewBBinRegion(BBJ_CALLFINALLY, XTnum + 1, 0, step, HBtab->ebdHndBeg); - // Jump target should be set to block as a dummy value - assert(step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + // step's jump target shouldn't be set yet + assert(!step->HasJump()); // the previous call to a finally returns to this call (to the next finally in the chain) step->SetJumpDest(callBlock); @@ -4388,7 +4388,7 @@ void Compiler::impImportLeave(BasicBlock* block) } // Note: we don't know the jump target yet - step = fgNewBBafter(BBJ_ALWAYS, callBlock, true DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + step = fgNewBBafter(BBJ_ALWAYS, callBlock, true); /* The new block will inherit this block's weight */ step->inheritWeight(block); step->bbFlags |= BBF_IMPORTED | BBF_KEEP_BBJ_ALWAYS; @@ -4452,8 +4452,8 @@ void Compiler::impImportLeave(BasicBlock* block) BasicBlock* finalStep = fgNewBBinRegion(BBJ_ALWAYS, tryIndex, leaveTarget->bbHndIndex, step, leaveTarget); finalStep->bbFlags |= BBF_KEEP_BBJ_ALWAYS; - // Jump target should be set to block as a dummy value - assert(step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + // step's jump target shouldn't be set yet + assert(!step->HasJump()); step->SetJumpDest(finalStep); fgAddRefPred(finalStep, step); @@ -4595,11 +4595,10 @@ void Compiler::impImportLeave(BasicBlock* block) /* Create a new catch exit block in the catch region for the existing step block to jump to in this * scope */ // Note: we don't know the jump target yet - BasicBlock* exitBlock = - fgNewBBinRegion(BBJ_EHCATCHRET, 0, XTnum + 1, step DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + BasicBlock* exitBlock = fgNewBBinRegion(BBJ_EHCATCHRET, 0, XTnum + 1, step); assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET)); - assert((step == block) || step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + assert((step == block) || !step->HasJump()); if (step == block) { fgRemoveRefPred(step->GetJumpDest(), step); @@ -4716,7 +4715,7 @@ void Compiler::impImportLeave(BasicBlock* block) // stack walks.) assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET)); - assert((step == block) || step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + assert((step == block) || !step->HasJump()); #if FEATURE_EH_CALLFINALLY_THUNKS if (step->KindIs(BBJ_EHCATCHRET)) @@ -4724,8 +4723,7 @@ void Compiler::impImportLeave(BasicBlock* block) // Need to create another step block in the 'try' region that will actually branch to the // call-to-finally thunk. // Note: we don't know the jump target yet - BasicBlock* step2 = - fgNewBBinRegion(BBJ_ALWAYS, XTnum + 1, 0, step DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + BasicBlock* step2 = fgNewBBinRegion(BBJ_ALWAYS, XTnum + 1, 0, step); if (step == block) { fgRemoveRefPred(step->GetJumpDest(), step); @@ -4760,7 +4758,7 @@ void Compiler::impImportLeave(BasicBlock* block) #endif // !FEATURE_EH_CALLFINALLY_THUNKS assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET)); - assert((step == block) || step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + assert((step == block) || !step->HasJump()); // callBlock will call the finally handler callBlock = @@ -4797,7 +4795,7 @@ void Compiler::impImportLeave(BasicBlock* block) } // Note: we don't know the jump target yet - step = fgNewBBafter(BBJ_ALWAYS, callBlock, true DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + step = fgNewBBafter(BBJ_ALWAYS, callBlock, true); stepType = ST_FinallyReturn; /* The new block will inherit this block's weight */ @@ -4861,7 +4859,7 @@ void Compiler::impImportLeave(BasicBlock* block) if ((stepType == ST_FinallyReturn) || (stepType == ST_Catch)) { assert(step); - assert((step == block) || step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + assert((step == block) || !step->HasJump()); if (stepType == ST_FinallyReturn) { @@ -4875,8 +4873,7 @@ void Compiler::impImportLeave(BasicBlock* block) /* Create a new exit block in the try region for the existing step block to jump to in this scope */ // Note: we don't know the jump target yet - BasicBlock* catchStep = - fgNewBBinRegion(BBJ_ALWAYS, XTnum + 1, 0, step DEBUG_ARG(&BasicBlock::bbTempJumpDest)); + BasicBlock* catchStep = fgNewBBinRegion(BBJ_ALWAYS, XTnum + 1, 0, step); if (step == block) { @@ -4938,7 +4935,7 @@ void Compiler::impImportLeave(BasicBlock* block) } else { - assert((step == block) || step->HasJumpTo(&BasicBlock::bbTempJumpDest)); + assert((step == block) || !step->HasJump()); if (step == block) { @@ -5013,7 +5010,7 @@ void Compiler::impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr) // will be treated as pair and handled correctly. if (block->KindIs(BBJ_CALLFINALLY)) { - BasicBlock* dupBlock = bbNewBasicBlock(block->GetJumpKind(), block->GetJumpDest()); + BasicBlock* dupBlock = BasicBlock::bbNewBasicBlock(this, block->GetJumpKind(), block->GetJumpDest()); dupBlock->bbFlags = block->bbFlags; fgAddRefPred(dupBlock->GetJumpDest(), dupBlock); dupBlock->copyEHRegion(block); diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index c245cdb6ee026..4e339e36983fb 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -2012,7 +2012,7 @@ bool Compiler::fgNormalizeEHCase1() { // ...then we want to insert an empty, non-removable block outside the try to be the new first block of the // handler. - BasicBlock* newHndStart = bbNewBasicBlock(BBJ_NONE); + BasicBlock* newHndStart = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); fgInsertBBbefore(handlerStart, newHndStart); fgAddRefPred(handlerStart, newHndStart); @@ -2181,7 +2181,7 @@ bool Compiler::fgNormalizeEHCase2() // We've got multiple 'try' blocks starting at the same place! // Add a new first 'try' block for 'ehOuter' that will be outside 'eh'. - BasicBlock* newTryStart = bbNewBasicBlock(BBJ_NONE); + BasicBlock* newTryStart = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); newTryStart->bbRefs = 0; fgInsertBBbefore(insertBeforeBlk, newTryStart); fgAddRefPred(insertBeforeBlk, newTryStart); @@ -2375,7 +2375,7 @@ bool Compiler::fgCreateFiltersForGenericExceptions() // Create a new bb for the fake filter BasicBlock* handlerBb = eh->ebdHndBeg; - BasicBlock* filterBb = bbNewBasicBlock(BBJ_EHFILTERRET, handlerBb); + BasicBlock* filterBb = BasicBlock::bbNewBasicBlock(this, BBJ_EHFILTERRET, handlerBb); // Now we need to spill CATCH_ARG (it should be the first thing evaluated) GenTree* arg = new (this, GT_CATCH_ARG) GenTree(GT_CATCH_ARG, TYP_REF); @@ -2660,7 +2660,7 @@ bool Compiler::fgNormalizeEHCase3() // Add a new last block for 'ehOuter' that will be outside the EH region with which it encloses and // shares a 'last' pointer - BasicBlock* newLast = bbNewBasicBlock(BBJ_NONE); + BasicBlock* newLast = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); newLast->bbRefs = 0; assert(insertAfterBlk != nullptr); fgInsertBBafter(insertAfterBlk, newLast); diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 355e0f198d5a0..c9bbfd3e3e583 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -8173,11 +8173,11 @@ bool Compiler::fgCreateLoopPreHeader(unsigned lnum) if (isTopEntryLoop) { - preHead = bbNewBasicBlock(BBJ_NONE); + preHead = BasicBlock::bbNewBasicBlock(this, BBJ_NONE); } else { - preHead = bbNewBasicBlock(BBJ_ALWAYS, entry); + preHead = BasicBlock::bbNewBasicBlock(this, BBJ_ALWAYS, entry); } preHead->bbFlags |= BBF_INTERNAL | BBF_LOOP_PREHEADER; diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 774e51ee8f987..100c65c0cc57f 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -1575,7 +1575,7 @@ void SsaBuilder::SetupBBRoot() return; } - BasicBlock* bbRoot = m_pCompiler->bbNewBasicBlock(BBJ_NONE); + BasicBlock* bbRoot = BasicBlock::bbNewBasicBlock(m_pCompiler, BBJ_NONE); bbRoot->bbFlags |= BBF_INTERNAL; // May need to fix up preds list, so remember the old first block.