diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index e71566da1f76a5..01ead6622e6e14 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -837,46 +837,6 @@ void BasicBlock::CloneBlockState(Compiler* compiler, BasicBlock* to, const Basic } } -//------------------------------------------------------------------------ -// CopyTarget: Copy the block kind and targets. The targets in the `from` block remain valid. -// Use `TransferTarget` to copy the pointer to the target descriptor (e.g., for BBJ_SWITCH/BBJ_EHFINALLYRET) -// after which the `from` block target is invalid. -// -// Arguments: -// compiler - Jit compiler instance -// from - Block to copy from -// -void BasicBlock::CopyTarget(Compiler* compiler, const BasicBlock* from) -{ - switch (from->GetKind()) - { - case BBJ_SWITCH: - SetSwitch(new (compiler, CMK_BasicBlock) BBswtDesc(compiler, from->GetSwitchTargets())); - break; - case BBJ_EHFINALLYRET: - SetEhf(new (compiler, CMK_BasicBlock) BBehfDesc(compiler, from->GetEhfTargets())); - break; - case BBJ_COND: - SetCond(from->GetTrueTarget(), from->GetFalseTarget()); - break; - case BBJ_ALWAYS: - SetKindAndTarget(from->GetKind(), from->GetTarget()); - CopyFlags(from, BBF_NONE_QUIRK); - break; - case BBJ_CALLFINALLY: - case BBJ_CALLFINALLYRET: - case BBJ_EHCATCHRET: - case BBJ_EHFILTERRET: - case BBJ_LEAVE: - SetKindAndTarget(from->GetKind(), from->GetTarget()); - break; - default: - SetKindAndTarget(from->GetKind()); // Clear the target - break; - } - assert(KindIs(from->GetKind())); -} - //------------------------------------------------------------------------ // TransferTarget: Like CopyTarget, but copies the target descriptors for block types which have // them (BBJ_SWITCH/BBJ_EHFINALLYRET), that is, take their memory, after which the `from` block @@ -1781,6 +1741,22 @@ bool BasicBlock::hasEHBoundaryOut() const return returnVal; } +//------------------------------------------------------------------------ +// BBswtDesc copy ctor: copy a switch descriptor, but don't set up the jump table +// +// Arguments: +// other - existing switch descriptor to copy (except for its jump table) +// +BBswtDesc::BBswtDesc(const BBswtDesc* other) + : bbsDstTab(nullptr) + , bbsCount(other->bbsCount) + , bbsDominantCase(other->bbsDominantCase) + , bbsDominantFraction(other->bbsDominantFraction) + , bbsHasDefault(other->bbsHasDefault) + , bbsHasDominantCase(other->bbsHasDominantCase) +{ +} + //------------------------------------------------------------------------ // BBswtDesc copy ctor: copy a switch descriptor // diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index d434ae8a92b01f..041af02b1c15ad 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -1660,9 +1660,6 @@ struct BasicBlock : private LIR::Range // Clone block state and statements from `from` block to `to` block (which must be new/empty) static void CloneBlockState(Compiler* compiler, BasicBlock* to, const BasicBlock* from); - // Copy the block kind and targets. The `from` block is untouched. - void CopyTarget(Compiler* compiler, const BasicBlock* from); - // Copy the block kind and take memory ownership of the targets. void TransferTarget(BasicBlock* from); @@ -1839,6 +1836,8 @@ struct BBswtDesc { } + BBswtDesc(const BBswtDesc* other); + BBswtDesc(Compiler* comp, const BBswtDesc* other); void removeDefault() diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 07d1ff60aee960..36416edae382d2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6839,16 +6839,9 @@ class Compiler bool optExtractInitTestIncr( BasicBlock** pInitBlock, BasicBlock* bottom, BasicBlock* top, GenTree** ppInit, GenTree** ppTest, GenTree** ppIncr); - enum class RedirectBlockOption - { - DoNotChangePredLists, // do not modify pred lists - UpdatePredLists, // add/remove to pred lists - AddToPredLists, // only add to pred lists - }; - void optRedirectBlock(BasicBlock* blk, - BlockToBlockMap* redirectMap, - const RedirectBlockOption = RedirectBlockOption::DoNotChangePredLists); + BasicBlock* newBlk, + BlockToBlockMap* redirectMap); // Marks the containsCall information to "loop" and any parent loops. void AddContainsCallAllContainingLoops(FlowGraphNaturalLoop* loop); diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 6f9eba855f8399..9ba1d20cd4bd6b 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -620,14 +620,13 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas case BBJ_EHCATCHRET: case BBJ_EHFILTERRET: case BBJ_LEAVE: // This function can be called before import, so we still have BBJ_LEAVE - - if (block->TargetIs(oldTarget)) - { - block->SetTarget(newTarget); - FlowEdge* const oldEdge = fgRemoveRefPred(oldTarget, block); - fgAddRefPred(newTarget, block, oldEdge); - } + { + assert(block->TargetIs(oldTarget)); + block->SetTarget(newTarget); + FlowEdge* const oldEdge = fgRemoveRefPred(oldTarget, block); + fgAddRefPred(newTarget, block, oldEdge); break; + } case BBJ_COND: @@ -709,6 +708,10 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas break; } + case BBJ_EHFINALLYRET: + fgReplaceEhfSuccessor(block, oldTarget, newTarget); + break; + default: assert(!"Block doesn't have a jump target!"); unreached(); @@ -5297,11 +5300,8 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) case BBJ_ALWAYS: case BBJ_EHCATCHRET: case BBJ_SWITCH: - fgReplaceJumpTarget(predBlock, block, succBlock); - break; - case BBJ_EHFINALLYRET: - fgReplaceEhfSuccessor(predBlock, block, succBlock); + fgReplaceJumpTarget(predBlock, block, succBlock); break; } } diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index c72927f589fb4a..4c9a6e3b1dab10 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -1099,8 +1099,7 @@ PhaseStatus Compiler::fgCloneFinally() } else { - newBlock->CopyTarget(this, block); - optRedirectBlock(newBlock, &blockMap, RedirectBlockOption::AddToPredLists); + optRedirectBlock(block, newBlock, &blockMap); } } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 2291139538e128..a03b72a4f42b58 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5608,36 +5608,9 @@ void FlowGraphNaturalLoop::Duplicate(BasicBlock** insertAfter, BlockToBlockMap* // Jump target should not be set yet assert(!newBlk->HasInitializedTarget()); - // First copy the jump destination(s) from "blk". - newBlk->CopyTarget(comp, blk); - - // Now redirect the new block according to "blockMap". - comp->optRedirectBlock(newBlk, map); - - // Add predecessor edges for the new successors, as well as the fall-through paths. - switch (newBlk->GetKind()) - { - case BBJ_ALWAYS: - case BBJ_CALLFINALLY: - case BBJ_CALLFINALLYRET: - comp->fgAddRefPred(newBlk->GetTarget(), newBlk); - break; - - case BBJ_COND: - comp->fgAddRefPred(newBlk->GetFalseTarget(), newBlk); - comp->fgAddRefPred(newBlk->GetTrueTarget(), newBlk); - break; - - case BBJ_SWITCH: - for (BasicBlock* const switchDest : newBlk->SwitchTargets()) - { - comp->fgAddRefPred(switchDest, newBlk); - } - break; - - default: - break; - } + // Redirect the new block according to "blockMap". + // opRedirectBlock will set newBlk's successors, and add pred edges for the successors. + comp->optRedirectBlock(blk, newBlk, map); return BasicBlockVisit::Continue; }); diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index bcb0bc90d071cf..436ce347e0ed45 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -553,170 +553,162 @@ void Compiler::optCheckPreds() #endif // DEBUG //------------------------------------------------------------------------ -// optRedirectBlock: Replace the branch successors of a block based on a block map. +// optRedirectBlock: Initialize the branch successors of a block based on a block map. // -// Updates the successors of `blk`: if `blk2` is a branch successor of `blk`, and there is a mapping -// for `blk2->blk3` in `redirectMap`, change `blk` so that `blk3` is this branch successor. +// Updates the successors of `newBlk`, a copy of `blk`: +// If `blk2` is a branch successor of `blk`, and there is a mapping +// for `blk2->blk3` in `redirectMap`, make `blk3` a successor of `newBlk`. +// Else, make `blk2` a successor of `newBlk`. // // Arguments: -// blk - block to redirect -// redirectMap - block->block map specifying how the `blk` target will be redirected. -// predOption - specifies how to update the pred lists +// blk - the original block, which doesn't need redirecting +// newBlk - copy of blk, with uninitialized successors +// redirectMap - block->block map specifying how to redirect the target of `blk`. // // Notes: -// Pred lists for successors of `blk` may be changed, depending on `predOption`. +// Initially, `newBlk` should not have any successors set. +// Upon returning, `newBlk` should have all of its successors initialized. +// `blk` must have its successors set upon entry; these won't be changed. // -void Compiler::optRedirectBlock(BasicBlock* blk, BlockToBlockMap* redirectMap, RedirectBlockOption predOption) +void Compiler::optRedirectBlock(BasicBlock* blk, BasicBlock* newBlk, BlockToBlockMap* redirectMap) { - const bool updatePreds = (predOption == RedirectBlockOption::UpdatePredLists); - const bool addPreds = (predOption == RedirectBlockOption::AddToPredLists); + // Caller should not have initialized newBlk's target yet + assert(newBlk->KindIs(BBJ_ALWAYS)); + assert(!newBlk->HasInitializedTarget()); - BasicBlock* newJumpDest = nullptr; + BasicBlock* newTarget; + // Initialize the successors of "newBlk". + // For each successor, use "blockMap" to determine if the successor needs to be redirected. switch (blk->GetKind()) { - case BBJ_THROW: - case BBJ_RETURN: - case BBJ_EHFILTERRET: - case BBJ_EHFAULTRET: - case BBJ_EHCATCHRET: - // These have no jump destination to update. - break; - - case BBJ_CALLFINALLY: - if (addPreds && blk->bbFallsThrough()) - { - fgAddRefPred(blk->Next(), blk); - } + case BBJ_ALWAYS: + // Copy BBF_NONE_QUIRK flag for BBJ_ALWAYS blocks only + newBlk->CopyFlags(blk, BBF_NONE_QUIRK); FALLTHROUGH; - case BBJ_ALWAYS: - case BBJ_LEAVE: + case BBJ_CALLFINALLY: case BBJ_CALLFINALLYRET: - // All of these have a single jump destination to update. - if (redirectMap->Lookup(blk->GetTarget(), &newJumpDest)) + case BBJ_LEAVE: + // Determine if newBlk should be redirected to a different target from blk's target + if (redirectMap->Lookup(blk->GetTarget(), &newTarget)) { - if (updatePreds) - { - fgRemoveRefPred(blk->GetTarget(), blk); - } - blk->SetTarget(newJumpDest); - if (updatePreds || addPreds) - { - fgAddRefPred(newJumpDest, blk); - } + // newBlk needs to be redirected to a new target + newBlk->SetKindAndTarget(blk->GetKind(), newTarget); } - else if (addPreds) + else { - fgAddRefPred(blk->GetTarget(), blk); + // newBlk uses the same target as blk + newBlk->SetKindAndTarget(blk->GetKind(), blk->GetTarget()); } + + fgAddRefPred(newBlk->GetTarget(), newBlk); break; case BBJ_COND: - // Update jump taken when condition is true - if (redirectMap->Lookup(blk->GetTrueTarget(), &newJumpDest)) + { + BasicBlock* trueTarget; + BasicBlock* falseTarget; + + // Determine if newBLk should be redirected to a different true target from blk's true target + if (redirectMap->Lookup(blk->GetTrueTarget(), &newTarget)) { - if (updatePreds) - { - fgRemoveRefPred(blk->GetTrueTarget(), blk); - } - blk->SetTrueTarget(newJumpDest); - if (updatePreds || addPreds) - { - fgAddRefPred(newJumpDest, blk); - } + // newBlk needs to be redirected to a new true target + trueTarget = newTarget; } - else if (addPreds) + else { - fgAddRefPred(blk->GetTrueTarget(), blk); + // newBlk uses the same true target as blk + trueTarget = blk->GetTrueTarget(); } - // Update jump taken when condition is false - if (redirectMap->Lookup(blk->GetFalseTarget(), &newJumpDest)) + // Do the same lookup for the false target + if (redirectMap->Lookup(blk->GetFalseTarget(), &newTarget)) { - if (updatePreds) - { - fgRemoveRefPred(blk->GetFalseTarget(), blk); - } - blk->SetFalseTarget(newJumpDest); - if (updatePreds || addPreds) - { - fgAddRefPred(newJumpDest, blk); - } + falseTarget = newTarget; } - else if (addPreds) + else { - fgAddRefPred(blk->GetFalseTarget(), blk); + falseTarget = blk->GetFalseTarget(); } + + fgAddRefPred(trueTarget, newBlk); + fgAddRefPred(falseTarget, newBlk); + newBlk->SetCond(trueTarget, falseTarget); break; + } case BBJ_EHFINALLYRET: { - BBehfDesc* ehfDesc = blk->GetEhfTargets(); - BasicBlock* newSucc = nullptr; - for (unsigned i = 0; i < ehfDesc->bbeCount; i++) + BBehfDesc* currEhfDesc = blk->GetEhfTargets(); + BBehfDesc* newEhfDesc = new (this, CMK_BasicBlock) BBehfDesc; + newEhfDesc->bbeCount = currEhfDesc->bbeCount; + newEhfDesc->bbeSuccs = new (this, CMK_BasicBlock) BasicBlock*[newEhfDesc->bbeCount]; + BasicBlock** jumpPtr = newEhfDesc->bbeSuccs; + + for (BasicBlock* const ehfTarget : blk->EHFinallyRetSuccs()) { - BasicBlock* const succ = ehfDesc->bbeSuccs[i]; - if (redirectMap->Lookup(succ, &newSucc)) + // Determine if newBlk should target ehfTarget, or be redirected + if (redirectMap->Lookup(ehfTarget, &newTarget)) { - if (updatePreds) - { - fgRemoveRefPred(succ, blk); - } - if (updatePreds || addPreds) - { - fgAddRefPred(newSucc, blk); - } - ehfDesc->bbeSuccs[i] = newSucc; + *jumpPtr = newTarget; } - else if (addPreds) + else { - fgAddRefPred(succ, blk); + *jumpPtr = ehfTarget; } + + fgAddRefPred(*jumpPtr, newBlk); + jumpPtr++; } + + newBlk->SetEhf(newEhfDesc); + break; } - break; case BBJ_SWITCH: { - bool redirected = false; - for (unsigned i = 0; i < blk->GetSwitchTargets()->bbsCount; i++) + BBswtDesc* currSwtDesc = blk->GetSwitchTargets(); + BBswtDesc* newSwtDesc = new (this, CMK_BasicBlock) BBswtDesc(currSwtDesc); + newSwtDesc->bbsDstTab = new (this, CMK_BasicBlock) BasicBlock*[newSwtDesc->bbsCount]; + BasicBlock** jumpPtr = newSwtDesc->bbsDstTab; + + for (BasicBlock* const switchTarget : blk->SwitchTargets()) { - BasicBlock* const switchDest = blk->GetSwitchTargets()->bbsDstTab[i]; - if (redirectMap->Lookup(switchDest, &newJumpDest)) - { - if (updatePreds) - { - fgRemoveRefPred(switchDest, blk); - } - if (updatePreds || addPreds) - { - fgAddRefPred(newJumpDest, blk); - } - blk->GetSwitchTargets()->bbsDstTab[i] = newJumpDest; - redirected = true; - } - else if (addPreds) + // Determine if newBlk should target switchTarget, or be redirected + if (redirectMap->Lookup(switchTarget, &newTarget)) { - fgAddRefPred(switchDest, blk); + *jumpPtr = newTarget; } - } - // If any redirections happened, invalidate the switch table map for the switch. - if (redirected) - { - // Don't create a new map just to try to remove an entry. - BlockToSwitchDescMap* switchMap = GetSwitchDescMap(/* createIfNull */ false); - if (switchMap != nullptr) + else { - switchMap->Remove(blk); + *jumpPtr = switchTarget; } + + fgAddRefPred(*jumpPtr, newBlk); + jumpPtr++; } + + newBlk->SetSwitch(newSwtDesc); + break; } - break; + + case BBJ_EHCATCHRET: + case BBJ_EHFILTERRET: + // newBlk's jump target should not need to be redirected + assert(!redirectMap->Lookup(blk->GetTarget(), &newTarget)); + newBlk->SetKindAndTarget(blk->GetKind(), blk->GetTarget()); + fgAddRefPred(newBlk->GetTarget(), newBlk); + break; default: - unreached(); + // blk doesn't have a jump destination + assert(blk->NumSucc() == 0); + newBlk->SetKindAndTarget(blk->GetKind()); + break; } + + assert(newBlk->KindIs(blk->GetKind())); } //----------------------------------------------------------------------------- @@ -2212,8 +2204,6 @@ bool Compiler::optInvertWhileLoop(BasicBlock* block) // is maintained no matter which condition block we point to, but we'll lose optimization potential // (and create spaghetti code) if we get it wrong. // - BlockToBlockMap blockMap(getAllocator(CMK_LoopOpt)); - bool blockMapInitialized = false; unsigned const loopFirstNum = bTop->bbNum; unsigned const loopBottomNum = bTest->bbNum; @@ -2226,16 +2216,30 @@ bool Compiler::optInvertWhileLoop(BasicBlock* block) continue; } - if (!blockMapInitialized) - { - blockMapInitialized = true; - blockMap.Set(bTest, bNewCond); - } - // Redirect the predecessor to the new block. JITDUMP("Redirecting non-loop " FMT_BB " -> " FMT_BB " to " FMT_BB " -> " FMT_BB "\n", predBlock->bbNum, bTest->bbNum, predBlock->bbNum, bNewCond->bbNum); - optRedirectBlock(predBlock, &blockMap, RedirectBlockOption::UpdatePredLists); + + switch (predBlock->GetKind()) + { + case BBJ_ALWAYS: + case BBJ_CALLFINALLY: + case BBJ_CALLFINALLYRET: + case BBJ_COND: + case BBJ_SWITCH: + case BBJ_EHFINALLYRET: + fgReplaceJumpTarget(predBlock, bTest, bNewCond); + break; + + case BBJ_EHCATCHRET: + case BBJ_EHFILTERRET: + // These block types should not need redirecting + break; + + default: + assert(!"Unexpected bbKind for predecessor block"); + break; + } } // If we have profile data for all blocks and we know that we are cloning the