diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index 01e137432ecf42..c62ead94d2acfa 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -301,20 +301,18 @@ bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const } //------------------------------------------------------------------------ -// CanRemoveJumpToTarget: determine if jump to target can be omitted +// CanRemoveJumpToFalseTarget: determine if jump to false target can be omitted // // Arguments: -// target - true/false target of BBJ_COND block // compiler - current compiler instance // // Returns: -// true if block is a BBJ_COND that can fall into target +// true if block is a BBJ_COND that can fall into its false target // -bool BasicBlock::CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) const +bool BasicBlock::CanRemoveJumpToFalseTarget(Compiler* compiler) const { assert(KindIs(BBJ_COND)); - assert(TrueTargetIs(target) || FalseTargetIs(target)); - return NextIs(target) && !compiler->fgInDifferentRegions(this, target); + return NextIs(bbFalseTarget) && !hasAlign() && !compiler->fgInDifferentRegions(this, bbFalseTarget); } //------------------------------------------------------------------------ @@ -1171,7 +1169,7 @@ unsigned BasicBlock::NumSucc() const return 1; case BBJ_COND: - if (bbTrueTarget == bbFalseTarget) + if (bbTarget == bbNext) { return 1; } @@ -1296,7 +1294,7 @@ unsigned BasicBlock::NumSucc(Compiler* comp) return 1; case BBJ_COND: - if (bbTrueTarget == bbFalseTarget) + if (bbTarget == bbNext) { return 1; } diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 0760baaab01659..ab557e71565d36 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -616,7 +616,7 @@ struct BasicBlock : private LIR::Range bool CanRemoveJumpToNext(Compiler* compiler) const; - bool CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) const; + bool CanRemoveJumpToFalseTarget(Compiler* compiler) const; unsigned GetTargetOffs() const { @@ -669,6 +669,7 @@ struct BasicBlock : private LIR::Range { assert(KindIs(BBJ_COND)); assert(bbTrueTarget != nullptr); + assert(target != nullptr); return (bbTrueTarget == target); } @@ -695,16 +696,15 @@ struct BasicBlock : private LIR::Range { assert(KindIs(BBJ_COND)); assert(bbFalseTarget != nullptr); + assert(target != nullptr); return (bbFalseTarget == target); } void SetCond(BasicBlock* trueTarget, BasicBlock* falseTarget) { - // Switch lowering may temporarily set a block to a BBJ_COND - // with a null false target if it is the last block in the list. - // This invalid state is eventually fixed, so allow it in the below assert. - assert((falseTarget != nullptr) || (falseTarget == bbNext)); assert(trueTarget != nullptr); + // TODO-NoFallThrough: Allow falseTarget to diverge from bbNext + assert(falseTarget == bbNext); bbKind = BBJ_COND; bbTrueTarget = trueTarget; bbFalseTarget = falseTarget; diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index 4a8c08a89858e8..28908f4392d798 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -1317,13 +1317,6 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue) regNumber reg = genConsumeReg(op); inst_RV_RV(INS_tst, reg, reg, genActualType(op)); inst_JMP(EJ_ne, compiler->compCurBB->GetTrueTarget()); - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 489aa6a744942d..d9681d12012eba 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -4650,13 +4650,6 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue) GenTree* op = jtrue->gtGetOp1(); regNumber reg = genConsumeReg(op); GetEmitter()->emitIns_J_R(INS_cbnz, emitActualTypeSize(op), compiler->compCurBB->GetTrueTarget(), reg); - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //------------------------------------------------------------------------ @@ -4884,13 +4877,6 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) GetEmitter()->emitIns_J_R(ins, attr, compiler->compCurBB->GetTrueTarget(), reg); } - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //--------------------------------------------------------------------- diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 133403e2f9d2bd..94a44ecc2512db 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -397,7 +397,7 @@ void CodeGen::genMarkLabelsForCodegen() block->GetTrueTarget()->SetFlags(BBF_HAS_LABEL); // If we need a jump to the false target, give it a label - if (!block->CanRemoveJumpToTarget(block->GetFalseTarget(), compiler)) + if (!block->CanRemoveJumpToFalseTarget(compiler)) { JITDUMP(" " FMT_BB " : branch target\n", block->GetFalseTarget()->bbNum); block->GetFalseTarget()->SetFlags(BBF_HAS_LABEL); diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 67f4bb73c0eb18..17d6f04bbdef55 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -2640,10 +2640,9 @@ void CodeGen::genCodeForJcc(GenTreeCC* jcc) inst_JCC(jcc->gtCondition, compiler->compCurBB->GetTrueTarget()); // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) + if (!compiler->compCurBB->CanRemoveJumpToFalseTarget(compiler)) { - inst_JMP(EJ_jmp, falseTarget); + inst_JMP(EJ_jmp, compiler->compCurBB->GetFalseTarget()); } } diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index 03471bc892f51c..a0d325c4f29b6f 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -4330,13 +4330,6 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) assert(regs != 0); emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), regs); // 5-bits; - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //--------------------------------------------------------------------- @@ -4881,31 +4874,16 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) case GT_JCC: { -#if !FEATURE_FIXED_OUT_ARGS BasicBlock* tgtBlock = compiler->compCurBB->KindIs(BBJ_COND) ? compiler->compCurBB->GetTrueTarget() : compiler->compCurBB->GetTarget(); +#if !FEATURE_FIXED_OUT_ARGS assert((tgtBlock->bbTgtStkDepth * sizeof(int) == genStackLevel) || isFramePointerUsed()); #endif // !FEATURE_FIXED_OUT_ARGS GenTreeCC* jcc = treeNode->AsCC(); assert(jcc->gtCondition.Is(GenCondition::EQ, GenCondition::NE)); instruction ins = jcc->gtCondition.Is(GenCondition::EQ) ? INS_bceqz : INS_bcnez; - - if (compiler->compCurBB->KindIs(BBJ_COND)) - { - emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), (int)1 /* cc */); - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } - } - else - { - emit->emitIns_J(ins, compiler->compCurBB->GetTarget(), (int)1 /* cc */); - } + emit->emitIns_J(ins, tgtBlock, (int)1 /* cc */); } break; diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 05f7a2c11f4432..73f82efa46d597 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -4240,13 +4240,6 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) assert(regs != 0); emit->emitIns_J(ins, compiler->compCurBB->GetTrueTarget(), regs); // 5-bits; - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //--------------------------------------------------------------------- diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 43c006291dd3e5..b806ca975c5072 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -1466,13 +1466,6 @@ void CodeGen::genCodeForJTrue(GenTreeOp* jtrue) regNumber reg = genConsumeReg(op); inst_RV_RV(INS_test, reg, reg, genActualType(op)); inst_JMP(EJ_jne, compiler->compCurBB->GetTrueTarget()); - - // If we cannot fall into the false target, emit a jump to it - BasicBlock* falseTarget = compiler->compCurBB->GetFalseTarget(); - if (!compiler->compCurBB->CanRemoveJumpToTarget(falseTarget, compiler)) - { - inst_JMP(EJ_jmp, falseTarget); - } } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d5b465d6dfa821..e823972ba864b7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5919,7 +5919,7 @@ class Compiler void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext); - BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst, bool noFallThroughQuirk = false); + BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst); bool fgRenumberBlocks(); @@ -5966,6 +5966,8 @@ class Compiler bool fgOptimizeSwitchBranches(BasicBlock* block); + bool fgOptimizeBranchToNext(BasicBlock* block, BasicBlock* bNext, BasicBlock* bPrev); + bool fgOptimizeSwitchJumps(); #ifdef DEBUG void fgPrintEdgeWeights(); diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 3b768ab43e7d1b..879a7fa83edb19 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -693,35 +693,15 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, Bas break; case BBJ_COND: - { - FlowEdge* oldEdge = fgGetPredForBlock(oldTarget, block); - assert(oldEdge != nullptr); + // Functionally equivalent to above if (block->TrueTargetIs(oldTarget)) { - if (block->FalseTargetIs(oldTarget)) - { - assert(oldEdge->getDupCount() == 2); - fgRemoveConditionalJump(block); - assert(block->KindIs(BBJ_ALWAYS)); - block->SetTarget(newTarget); - } - else - { - block->SetTrueTarget(newTarget); - } - } - else - { - assert(block->FalseTargetIs(oldTarget)); - block->SetFalseTarget(newTarget); + block->SetTrueTarget(newTarget); + fgRemoveRefPred(oldTarget, block); + fgAddRefPred(newTarget, block); } - - assert(oldEdge->getDupCount() == 1); - fgRemoveRefPred(oldTarget, block); - fgAddRefPred(newTarget, block); break; - } case BBJ_SWITCH: { @@ -5072,17 +5052,13 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ) if (curr->KindIs(BBJ_COND)) { + curr->SetFalseTarget(curr->Next()); + fgReplacePred(succ, curr, newBlock); if (curr->TrueTargetIs(succ)) { + // Now 'curr' jumps to newBlock curr->SetTrueTarget(newBlock); } - else - { - assert(curr->FalseTargetIs(succ)); - curr->SetFalseTarget(newBlock); - } - - fgReplacePred(succ, curr, newBlock); fgAddRefPred(newBlock, curr); } else if (curr->KindIs(BBJ_SWITCH)) @@ -5292,12 +5268,6 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) /* At this point the bbPreds and bbRefs had better be zero */ noway_assert((block->bbRefs == 0) && (block->bbPreds == nullptr)); - - if (bPrev->KindIs(BBJ_COND) && bPrev->FalseTargetIs(block)) - { - assert(bNext != nullptr); - bPrev->SetFalseTarget(bNext); - } } else // block is empty { @@ -5396,15 +5366,24 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) break; case BBJ_COND: - if (predBlock->TrueTargetIs(block)) + /* The links for the direct predecessor case have already been updated above */ + if (!predBlock->TrueTargetIs(block)) { - predBlock->SetTrueTarget(succBlock); + break; } - if (predBlock->FalseTargetIs(block)) + /* Check if both sides of the BBJ_COND now jump to the same block */ + if (predBlock->FalseTargetIs(succBlock)) { - predBlock->SetFalseTarget(succBlock); + // Make sure we are replacing "block" with "succBlock" in predBlock->bbTarget. + noway_assert(predBlock->TrueTargetIs(block)); + predBlock->SetTrueTarget(succBlock); + fgRemoveConditionalJump(predBlock); + break; } + + noway_assert(predBlock->TrueTargetIs(block)); + predBlock->SetTrueTarget(succBlock); break; case BBJ_CALLFINALLY: @@ -5439,9 +5418,7 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) break; case BBJ_COND: - // block should not be a target anymore - assert(!bPrev->TrueTargetIs(block)); - assert(!bPrev->FalseTargetIs(block)); + bPrev->SetFalseTarget(block->Next()); /* Check if both sides of the BBJ_COND now jump to the same block */ if (bPrev->TrueTargetIs(bPrev->GetFalseTarget())) @@ -5509,7 +5486,7 @@ void Compiler::fgPrepareCallFinallyRetForRemoval(BasicBlock* block) // Newly inserted block after bSrc that jumps to bDst, // or nullptr if bSrc already falls through to bDst // -BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst, bool noFallThroughQuirk /* = false */) +BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) { assert(bSrc != nullptr); assert(fgPredsComputed); @@ -5517,18 +5494,8 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst, b /* If bSrc falls through to a block that is not bDst, we will insert a jump to bDst */ - if (bSrc->KindIs(BBJ_COND) && bSrc->FalseTargetIs(bDst) && !bSrc->NextIs(bDst)) + if (bSrc->KindIs(BBJ_COND) && !bSrc->NextIs(bDst)) { - assert(fgGetPredForBlock(bDst, bSrc) != nullptr); - - // Allow the caller to decide whether to use the old logic of maintaining implicit fallthrough behavior, - // or to allow BBJ_COND blocks' false targets to diverge from bbNext. - // TODO-NoFallThrough: Remove this quirk once callers' dependencies on implicit fallthrough are gone. - if (noFallThroughQuirk) - { - return jmpBlk; - } - // Add a new block after bSrc which jumps to 'bDst' jmpBlk = fgNewBBafter(BBJ_ALWAYS, bSrc, true, bDst); bSrc->SetFalseTarget(jmpBlk); @@ -5699,14 +5666,8 @@ bool Compiler::fgRenumberBlocks() */ bool Compiler::fgIsForwardBranch(BasicBlock* bJump, BasicBlock* bDest, BasicBlock* bSrc /* = NULL */) { - if (bJump->KindIs(BBJ_COND)) - { - assert(bJump->TrueTargetIs(bDest) || bJump->FalseTargetIs(bDest)); - } - else - { - assert(bJump->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET) && bJump->TargetIs(bDest)); - } + assert((bJump->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET) && bJump->TargetIs(bDest)) || + (bJump->KindIs(BBJ_COND) && bJump->TrueTargetIs(bDest))); bool result = false; BasicBlock* bTemp = (bSrc == nullptr) ? bJump : bSrc; diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index f1f1afd357bced..92aac7828e1333 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -136,7 +136,7 @@ void Compiler::fgDebugCheckUpdate() // A conditional branch should never jump to the next block as it can be folded into a BBJ_ALWAYS. if (block->KindIs(BBJ_COND) && block->TrueTargetIs(block->GetFalseTarget())) { - noway_assert(!"BBJ_COND true/false targets are the same!"); + noway_assert(!"Unnecessary jump to the next block!"); } // For a BBJ_CALLFINALLY block we make sure that we are followed by a BBJ_CALLFINALLYRET block @@ -2920,6 +2920,13 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef maxBBNum = max(maxBBNum, block->bbNum); + // BBJ_COND's normal (false) jump target is expected to be the next block + // TODO-NoFallThrough: Allow bbFalseTarget to diverge from bbNext + if (block->KindIs(BBJ_COND)) + { + assert(block->NextIs(block->GetFalseTarget())); + } + // Check that all the successors have the current traversal stamp. Use the 'Compiler*' version of the // iterator, but not for BBJ_SWITCH: we don't want to end up calling GetDescriptorForSwitch(), which will // dynamically create the unique switch list. diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 25afba806486a9..39440510a60e7e 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1442,6 +1442,109 @@ void Compiler::fgUnreachableBlock(BasicBlock* block) fgRemoveBlockAsPred(block); } +//------------------------------------------------------------- +// fgRemoveConditionalJump: Remove or morph a jump when we jump to the same +// block when both the condition is true or false. Remove the branch condition, +// but leave any required side effects. +// +// Arguments: +// block - block with conditional branch +// +void Compiler::fgRemoveConditionalJump(BasicBlock* block) +{ + noway_assert(block->KindIs(BBJ_COND) && block->TrueTargetIs(block->GetFalseTarget())); + assert(compRationalIRForm == block->IsLIR()); + + FlowEdge* flow = fgGetPredForBlock(block->GetFalseTarget(), block); + noway_assert(flow->getDupCount() == 2); + + // Change the BBJ_COND to BBJ_ALWAYS, and adjust the refCount and dupCount. + --block->GetFalseTarget()->bbRefs; + block->SetKind(BBJ_ALWAYS); + block->SetFlags(BBF_NONE_QUIRK); + flow->decrementDupCount(); + +#ifdef DEBUG + if (verbose) + { + printf("Block " FMT_BB " becoming a BBJ_ALWAYS to " FMT_BB " (jump target is the same whether the condition" + " is true or false)\n", + block->bbNum, block->GetTarget()->bbNum); + } +#endif + + // Remove the block jump condition + + if (block->IsLIR()) + { + LIR::Range& blockRange = LIR::AsRange(block); + + GenTree* test = blockRange.LastNode(); + assert(test->OperIsConditionalJump()); + + bool isClosed; + unsigned sideEffects; + LIR::ReadOnlyRange testRange = blockRange.GetTreeRange(test, &isClosed, &sideEffects); + + // TODO-LIR: this should really be checking GTF_ALL_EFFECT, but that produces unacceptable + // diffs compared to the existing backend. + if (isClosed && ((sideEffects & GTF_SIDE_EFFECT) == 0)) + { + // If the jump and its operands form a contiguous, side-effect-free range, + // remove them. + blockRange.Delete(this, block, std::move(testRange)); + } + else + { + // Otherwise, just remove the jump node itself. + blockRange.Remove(test, true); + } + } + else + { + Statement* test = block->lastStmt(); + GenTree* tree = test->GetRootNode(); + + noway_assert(tree->gtOper == GT_JTRUE); + + GenTree* sideEffList = nullptr; + + if (tree->gtFlags & GTF_SIDE_EFFECT) + { + gtExtractSideEffList(tree, &sideEffList); + + if (sideEffList) + { + noway_assert(sideEffList->gtFlags & GTF_SIDE_EFFECT); +#ifdef DEBUG + if (verbose) + { + printf("Extracted side effects list from condition...\n"); + gtDispTree(sideEffList); + printf("\n"); + } +#endif + } + } + + // Delete the cond test or replace it with the side effect tree + if (sideEffList == nullptr) + { + fgRemoveStmt(block, test); + } + else + { + test->SetRootNode(sideEffList); + + if (fgNodeThreading != NodeThreading::None) + { + gtSetStmtInfo(test); + fgSetStmtSeq(test); + } + } + } +} + //------------------------------------------------------------- // fgOptimizeBranchToEmptyUnconditional: // Optimize a jump to an empty block which ends in an unconditional branch. @@ -2507,25 +2610,27 @@ bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock* } //------------------------------------------------------------- -// fgRemoveConditionalJump: Remove or morph a jump when we jump to the same -// block when both the condition is true or false. Remove the branch condition, -// but leave any required side effects. +// fgOptimizeBranchToNext: +// Optimize a block which has a branch to the following block // // Arguments: -// block - block with conditional branch +// block - block with a BBJ_COND jump kind +// bNext - target that block jumps to regardless of its condition +// bPrev - block which is prior to the first block // -void Compiler::fgRemoveConditionalJump(BasicBlock* block) +// Returns: true if changes were made +// +bool Compiler::fgOptimizeBranchToNext(BasicBlock* block, BasicBlock* bNext, BasicBlock* bPrev) { assert(block->KindIs(BBJ_COND)); - assert(block->FalseTargetIs(block->GetTrueTarget())); - BasicBlock* target = block->GetTrueTarget(); + assert(block->TrueTargetIs(bNext)); + assert(block->FalseTargetIs(bNext)); + assert(block->PrevIs(bPrev)); #ifdef DEBUG if (verbose) { - printf("Block " FMT_BB " becoming a BBJ_ALWAYS to " FMT_BB " (jump target is the same whether the condition" - " is true or false)\n", - block->bbNum, target->bbNum); + printf("\nRemoving conditional jump to next block (" FMT_BB " -> " FMT_BB ")\n", block->bbNum, bNext->bbNum); } #endif // DEBUG @@ -2624,20 +2729,17 @@ void Compiler::fgRemoveConditionalJump(BasicBlock* block) } } - /* Conditional is gone - always jump to target */ + /* Conditional is gone - always jump to the next block */ block->SetKind(BBJ_ALWAYS); - if (block->JumpsToNext()) - { - block->SetFlags(BBF_NONE_QUIRK); - } - /* Update bbRefs and bbNum - Conditional predecessors to the same * block are counted twice so we have to remove one of them */ - noway_assert(target->countOfInEdges() > 1); - fgRemoveRefPred(target, block); + noway_assert(bNext->countOfInEdges() > 1); + fgRemoveRefPred(bNext, block); + + return true; } //------------------------------------------------------------- @@ -3560,13 +3662,11 @@ bool Compiler::fgReorderBlocks(bool useProfile) continue; } - bool reorderBlock = useProfile; - const bool isRare = block->isRunRarely(); - BasicBlock* bDest = nullptr; - BasicBlock* bFalseDest = nullptr; - bool forwardBranch = false; - bool backwardBranch = false; - bool forwardFalseBranch = false; + bool reorderBlock = useProfile; + const bool isRare = block->isRunRarely(); + BasicBlock* bDest = nullptr; + bool forwardBranch = false; + bool backwardBranch = false; // Setup bDest if (bPrev->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET)) @@ -3577,13 +3677,9 @@ bool Compiler::fgReorderBlocks(bool useProfile) } else if (bPrev->KindIs(BBJ_COND)) { - assert(bPrev->FalseTargetIs(block) || useProfile); - bDest = bPrev->GetTrueTarget(); - bFalseDest = bPrev->GetFalseTarget(); - // TODO: Cheaper to call fgRenumberBlocks first and compare bbNums? - forwardBranch = fgIsForwardBranch(bPrev, bDest); - backwardBranch = !forwardBranch; - forwardFalseBranch = fgIsForwardBranch(bPrev, bFalseDest); + bDest = bPrev->GetTrueTarget(); + forwardBranch = fgIsForwardBranch(bPrev, bDest); + backwardBranch = !forwardBranch; } // We will look for bPrev as a non rarely run block followed by block as a rarely run block @@ -3716,55 +3812,43 @@ bool Compiler::fgReorderBlocks(bool useProfile) // We will check that the min weight of the bPrev to bDest edge // is more than twice the max weight of the bPrev to block edge. // - // bPrev --------> [BB04, weight 31] + // bPrev --> [BB04, weight 31] // | \. - // falseEdge ---------------> O \. + // edgeToBlock -------------> O \. // [min=8,max=10] V \. - // bFalseDest ---> [BB05, weight 10] \. + // block --> [BB05, weight 10] \. // \. - // trueEdge ------------------------------> O + // edgeToDest ----------------------------> O // [min=21,max=23] | // V // bDest ---------------> [BB08, weight 21] // - FlowEdge* trueEdge = fgGetPredForBlock(bDest, bPrev); - FlowEdge* falseEdge = fgGetPredForBlock(bFalseDest, bPrev); - noway_assert(trueEdge != nullptr); - noway_assert(falseEdge != nullptr); + FlowEdge* edgeToDest = fgGetPredForBlock(bDest, bPrev); + FlowEdge* edgeToBlock = fgGetPredForBlock(block, bPrev); + noway_assert(edgeToDest != nullptr); + noway_assert(edgeToBlock != nullptr); // // Calculate the taken ratio - // A takenRatio of 0.10 means taken 10% of the time, not taken 90% of the time - // A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time - // A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time + // A takenRation of 0.10 means taken 10% of the time, not taken 90% of the time + // A takenRation of 0.50 means taken 50% of the time, not taken 50% of the time + // A takenRation of 0.90 means taken 90% of the time, not taken 10% of the time // double takenCount = - ((double)trueEdge->edgeWeightMin() + (double)trueEdge->edgeWeightMax()) / 2.0; + ((double)edgeToDest->edgeWeightMin() + (double)edgeToDest->edgeWeightMax()) / 2.0; double notTakenCount = - ((double)falseEdge->edgeWeightMin() + (double)falseEdge->edgeWeightMax()) / 2.0; + ((double)edgeToBlock->edgeWeightMin() + (double)edgeToBlock->edgeWeightMax()) / 2.0; double totalCount = takenCount + notTakenCount; // If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse // the branch if (takenCount < (0.51 * totalCount)) { - // We take bFalseDest more often. - // If bFalseDest is a backward branch, we should reverse the condition. - // Else if both branches are forward, not much use in reversing the condition. - reorderBlock = !forwardFalseBranch; - } - else if (bFalseDest == block) - { - // We take bDest more often, and we can fall into bFalseDest, - // so it makes sense to reverse the condition. - profHotWeight = (falseEdge->edgeWeightMin() + falseEdge->edgeWeightMax()) / 2 - 1; + reorderBlock = false; } else { - // We take bDest more often than bFalseDest, but bFalseDest isn't the next block, - // so reversing the branch doesn't make sense since bDest is already a forward branch. - assert(takenCount >= (0.51 * totalCount)); - assert(bFalseDest != block); - reorderBlock = false; + // set profHotWeight + profHotWeight = (edgeToBlock->edgeWeightMin() + edgeToBlock->edgeWeightMax()) / 2 - 1; } } else @@ -3798,16 +3882,8 @@ bool Compiler::fgReorderBlocks(bool useProfile) profHotWeight = (weightDest < weightPrev) ? weightDest : weightPrev; // if the weight of block is greater (or equal) to profHotWeight then we don't reverse the cond - if (bDest->bbWeight >= profHotWeight) - { - // bFalseDest has a greater weight, so if it is a backward branch, - // we should reverse the branch. - reorderBlock = !forwardFalseBranch; - } - else if (bFalseDest != block) + if (block->bbWeight >= profHotWeight) { - // bFalseDest is taken less often, but it isn't the next block, - // so it doesn't make sense to reverse the branch. reorderBlock = false; } } @@ -3869,9 +3945,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) } const bool bTmpJumpsToNext = bTmp->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLYRET) && bTmp->JumpsToNext(); - const bool bTmpFallsThrough = - bTmp->bbFallsThrough() && (!bTmp->KindIs(BBJ_COND) || bTmp->NextIs(bTmp->GetFalseTarget())); - if ((!bTmpJumpsToNext && !bTmpFallsThrough) || (bTmp->bbWeight == BB_ZERO_WEIGHT)) + if ((!bTmp->bbFallsThrough() && !bTmpJumpsToNext) || (bTmp->bbWeight == BB_ZERO_WEIGHT)) { lastNonFallThroughBlock = bTmp; } @@ -4063,7 +4137,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) } // Set connected_bDest to true if moving blocks [bStart .. bEnd] - // connects with the jump dest of bPrev (i.e bDest) and + // connects with the jump dest of bPrev (i.e bDest) and // thus allows bPrev fall through instead of jump. if (bNext == bDest) { @@ -4142,12 +4216,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) { // Treat jumps to next block as fall-through } - else if (bEnd2->KindIs(BBJ_COND) && !bEnd2->FalseTargetIs(bNext)) - { - // Block does not fall through into false target - break; - } - else if (!bEnd2->bbFallsThrough()) + else if (bEnd2->bbFallsThrough() == false) { break; } @@ -4235,7 +4304,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) } else { - if (bPrev->KindIs(BBJ_COND) && bPrev->NextIs(bPrev->GetFalseTarget())) + if (bPrev->bbFallsThrough()) { printf("since it falls into a rarely run block\n"); } @@ -4331,8 +4400,6 @@ bool Compiler::fgReorderBlocks(bool useProfile) // Find new location for the unlinked block(s) // Set insertAfterBlk to the block which will precede the insertion point - EHblkDsc* ehDsc; - if (!bStart->hasTryIndex() && isRare) { // We'll just insert the blocks at the end of the method. If the method @@ -4347,7 +4414,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) { BasicBlock* startBlk; BasicBlock* lastBlk; - ehDsc = ehInitTryBlockRange(bStart, &startBlk, &lastBlk); + EHblkDsc* ehDsc = ehInitTryBlockRange(bStart, &startBlk, &lastBlk); BasicBlock* endBlk; @@ -4589,11 +4656,6 @@ bool Compiler::fgReorderBlocks(bool useProfile) noway_assert(condTest->gtOper == GT_JTRUE); condTest->AsOp()->gtOp1 = gtReverseCond(condTest->AsOp()->gtOp1); - BasicBlock* trueTarget = bPrev->GetTrueTarget(); - BasicBlock* falseTarget = bPrev->GetFalseTarget(); - bPrev->SetTrueTarget(falseTarget); - bPrev->SetFalseTarget(trueTarget); - // may need to rethread // if (fgNodeThreading == NodeThreading::AllTrees) @@ -4603,10 +4665,18 @@ bool Compiler::fgReorderBlocks(bool useProfile) fgSetStmtSeq(condTestStmt); } - if (bStart2 != nullptr) + if (bStart2 == nullptr) + { + /* Set the new jump dest for bPrev to the rarely run or uncommon block(s) */ + bPrev->SetTrueTarget(bStart); + } + else { noway_assert(insertAfterBlk == bPrev); noway_assert(insertAfterBlk->NextIs(block)); + + /* Set the new jump dest for bPrev to the rarely run or uncommon block(s) */ + bPrev->SetTrueTarget(block); } } @@ -4645,35 +4715,32 @@ bool Compiler::fgReorderBlocks(bool useProfile) /* We have decided to insert the block(s) after 'insertAfterBlk' */ fgMoveBlocksAfter(bStart, bEnd, insertAfterBlk); - // useProfile should be true only when finalizing the block layout in Compiler::optOptimizeLayout. - // In this final pass, allow BBJ_COND blocks' false targets to diverge from bbNext. - // TODO-NoFallThrough: Always allow the false targets to diverge. if (bDest) { /* We may need to insert an unconditional branch after bPrev to bDest */ - fgConnectFallThrough(bPrev, bDest, /* noFallThroughQuirk */ useProfile); + fgConnectFallThrough(bPrev, bDest); } else { /* If bPrev falls through, we must insert a jump to block */ - fgConnectFallThrough(bPrev, block, /* noFallThroughQuirk */ useProfile); + fgConnectFallThrough(bPrev, block); } BasicBlock* bSkip = bEnd->Next(); /* If bEnd falls through, we must insert a jump to bNext */ - fgConnectFallThrough(bEnd, bNext, /* noFallThroughQuirk */ useProfile); + fgConnectFallThrough(bEnd, bNext); if (bStart2 == nullptr) { /* If insertAfterBlk falls through, we are forced to */ /* add a jump around the block(s) we just inserted */ - fgConnectFallThrough(insertAfterBlk, bSkip, /* noFallThroughQuirk */ useProfile); + fgConnectFallThrough(insertAfterBlk, bSkip); } else { /* We may need to insert an unconditional branch after bPrev2 to bStart */ - fgConnectFallThrough(bPrev2, bStart, /* noFallThroughQuirk */ useProfile); + fgConnectFallThrough(bPrev2, bStart); } #if DEBUG @@ -4856,41 +4923,25 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) } else if (block->KindIs(BBJ_COND)) { - bDest = block->GetTrueTarget(); - BasicBlock* bFalseDest = block->GetFalseTarget(); - assert(bFalseDest != nullptr); - - if (bFalseDest->KindIs(BBJ_ALWAYS) && bFalseDest->TargetIs(bDest) && bFalseDest->isEmpty()) - { - // Optimize bFalseDest -> BBJ_ALWAYS -> bDest - block->SetFalseTarget(bDest); - fgRemoveRefPred(bFalseDest, block); - fgAddRefPred(bDest, block); - } - else if (bDest->KindIs(BBJ_ALWAYS) && bDest->TargetIs(bFalseDest) && bDest->isEmpty()) - { - // Optimize bDest -> BBJ_ALWAYS -> bFalseDest - block->SetTrueTarget(bFalseDest); - fgRemoveRefPred(bDest, block); - fgAddRefPred(bFalseDest, block); - bDest = bFalseDest; - } - - if (block->FalseTargetIs(bDest)) + bDest = block->GetTrueTarget(); + if (bDest == bNext) { - fgRemoveConditionalJump(block); - change = true; - modified = true; - assert(block->KindIs(BBJ_ALWAYS)); - assert(block->TargetIs(bDest)); + // TODO-NoFallThrough: Fix above condition once bbFalseTarget can diverge from bbNext + assert(block->FalseTargetIs(bNext)); + if (fgOptimizeBranchToNext(block, bNext, bPrev)) + { + change = true; + modified = true; + bDest = nullptr; + } } } if (bDest != nullptr) { // Do we have a JUMP to an empty unconditional JUMP block? - if (bDest->KindIs(BBJ_ALWAYS) && !bDest->TargetIs(bDest) && // special case for self jumps - bDest->isEmpty()) + if (bDest->isEmpty() && bDest->KindIs(BBJ_ALWAYS) && + !bDest->TargetIs(bDest)) // special case for self jumps { // TODO: Allow optimizing branches to blocks that jump to the next block const bool optimizeBranch = !bDest->JumpsToNext() || !bDest->HasFlag(BBF_NONE_QUIRK); @@ -4910,18 +4961,18 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) // (b) block jump target is elsewhere but join free, and // bNext's jump target has a join. // - if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block - (bNext != nullptr) && // block isn't the last block - block->FalseTargetIs(bNext) && // block falls into its false target - (bNext->bbRefs == 1) && // No other block jumps to bNext - bNext->KindIs(BBJ_ALWAYS) && // The next block is a BBJ_ALWAYS block - !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) - bNext->isEmpty() && // and it is an empty block - !bNext->TargetIs(bNext) && // special case for self jumps + if (block->KindIs(BBJ_COND) && // block is a BBJ_COND block + (bNext->bbRefs == 1) && // No other block jumps to bNext + bNext->KindIs(BBJ_ALWAYS) && // The next block is a BBJ_ALWAYS block + !bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them) + bNext->isEmpty() && // and it is an empty block + !bNext->TargetIs(bNext) && // special case for self jumps !bDest->IsFirstColdBlock(this) && !fgInDifferentRegions(block, bDest)) // do not cross hot/cold sections { - assert(!block->IsLast()); + // bbFalseTarget cannot be null + assert(block->FalseTargetIs(bNext)); + assert(bNext != nullptr); // case (a) // @@ -4977,12 +5028,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) } } - // TODO-NoFallThrough: Remove this requirement - if (bDest->KindIs(BBJ_COND) && !bDest->NextIs(bDest->GetFalseTarget())) - { - optimizeJump = false; - } - if (optimizeJump && isJumpToJoinFree) { // In the join free case, we also need to move bDest right after bNext @@ -5063,7 +5108,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) } // Optimize the Conditional JUMP to go to the new target - assert(block->FalseTargetIs(bNext)); block->SetTrueTarget(bNext->GetTarget()); block->SetFalseTarget(bNext->Next()); diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index 1bf41de2dab921..923d3276664988 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -335,12 +335,9 @@ void ProfileSynthesis::AssignLikelihoodSwitch(BasicBlock* block) { // Assume each switch case is equally probable // - const unsigned n = block->NumSucc(); assert(n != 0); - - // Duplicate zero check to silence "divide by zero" compiler warning - const weight_t p = (n != 0) ? (1 / (weight_t)n) : 0; + const weight_t p = 1 / (weight_t)n; // Each unique edge gets some multiple of that basic probability // diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 7242380cb605b7..f1bdeeb22265fd 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -2256,6 +2256,13 @@ bool Compiler::fgNormalizeEHCase2() // fgReplaceJumpTarget(predBlock, newTryStart, insertBeforeBlk); + if (predBlock->NextIs(newTryStart) && predBlock->KindIs(BBJ_COND)) + { + predBlock->SetFalseTarget(newTryStart); + fgRemoveRefPred(insertBeforeBlk, predBlock); + fgAddRefPred(newTryStart, predBlock); + } + JITDUMP("Redirect " FMT_BB " target from " FMT_BB " to " FMT_BB ".\n", predBlock->bbNum, insertBeforeBlk->bbNum, newTryStart->bbNum); } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 27079d52e3bfb9..f1142bcbd215b1 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7200,7 +7200,7 @@ PhaseStatus Lowering::DoPhase() // local var liveness can delete code, which may create empty blocks if (comp->opts.OptimizationEnabled()) { - bool modified = comp->fgUpdateFlowGraph(false, false); + bool modified = comp->fgUpdateFlowGraph(); modified |= comp->fgRemoveDeadBlocks(); if (modified) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index eb9941b5f569bf..d124ac28fd865f 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2436,7 +2436,7 @@ PhaseStatus Compiler::optOptimizeLayout() { noway_assert(opts.OptimizationEnabled()); - fgUpdateFlowGraph(); + fgUpdateFlowGraph(/* doTailDuplication */ false); fgReorderBlocks(/* useProfile */ true); fgUpdateFlowGraph(); @@ -3099,29 +3099,47 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) header->bbNum, enterBlock->bbNum, preheader->bbNum); fgReplaceJumpTarget(enterBlock, preheader, header); - } - - // For BBJ_COND blocks preceding header, fgReplaceJumpTarget set their false targets to preheader. - // Direct fallthrough to preheader by inserting a jump after the block. - // TODO-NoFallThrough: Remove this, and just rely on fgReplaceJumpTarget to do the fixup - BasicBlock* fallthroughSource = header->Prev(); - if (fallthroughSource->KindIs(BBJ_COND) && fallthroughSource->FalseTargetIs(preheader)) - { - FlowEdge* edge = fgRemoveRefPred(preheader, fallthroughSource); - BasicBlock* newAlways = fgNewBBafter(BBJ_ALWAYS, fallthroughSource, true, preheader); - fgAddRefPred(preheader, newAlways, edge); - fgAddRefPred(newAlways, fallthroughSource, edge); - JITDUMP("Created new BBJ_ALWAYS " FMT_BB " to redirect " FMT_BB " to preheader\n", newAlways->bbNum, - fallthroughSource->bbNum); - fallthroughSource->SetFalseTarget(newAlways); + // Fix up fall through + if (enterBlock->KindIs(BBJ_COND) && enterBlock->NextIs(header)) + { + FlowEdge* edge = fgRemoveRefPred(header, enterBlock); + BasicBlock* newAlways = fgNewBBafter(BBJ_ALWAYS, enterBlock, true, preheader); + fgAddRefPred(preheader, newAlways, edge); + fgAddRefPred(newAlways, enterBlock, edge); + + JITDUMP("Created new BBJ_ALWAYS " FMT_BB " to redirect " FMT_BB " to preheader\n", newAlways->bbNum, + enterBlock->bbNum); + enterBlock->SetFalseTarget(newAlways); + } } - // Make sure false target is set correctly for fallthrough into preheader. - if (!preheader->IsFirst()) + // Fix up potential fallthrough into the preheader. + BasicBlock* fallthroughSource = preheader->Prev(); + if ((fallthroughSource != nullptr) && fallthroughSource->KindIs(BBJ_COND)) { - fallthroughSource = preheader->Prev(); - assert(!fallthroughSource->KindIs(BBJ_COND) || fallthroughSource->FalseTargetIs(preheader)); + if (!loop->ContainsBlock(fallthroughSource)) + { + // Either unreachable or an enter edge. The new fallthrough into + // the preheader is what we want. We still need to update refs + // which fgReplaceJumpTarget doesn't do for fallthrough. + FlowEdge* old = fgRemoveRefPred(header, fallthroughSource); + fgAddRefPred(preheader, fallthroughSource, old); + } + else + { + // For a backedge we need to make sure we're still going to the head, + // and not falling into the preheader. + FlowEdge* edge = fgRemoveRefPred(header, fallthroughSource); + BasicBlock* newAlways = fgNewBBafter(BBJ_ALWAYS, fallthroughSource, true, header); + fgAddRefPred(header, newAlways, edge); + fgAddRefPred(newAlways, fallthroughSource, edge); + + JITDUMP("Created new BBJ_ALWAYS " FMT_BB " to redirect " FMT_BB " over preheader\n", newAlways->bbNum, + fallthroughSource->bbNum); + } + + fallthroughSource->SetFalseTarget(fallthroughSource->Next()); } optSetPreheaderWeight(loop, preheader); diff --git a/src/coreclr/jit/stacklevelsetter.cpp b/src/coreclr/jit/stacklevelsetter.cpp index d422b6559632cb..db97352d5e6977 100644 --- a/src/coreclr/jit/stacklevelsetter.cpp +++ b/src/coreclr/jit/stacklevelsetter.cpp @@ -89,29 +89,6 @@ PhaseStatus StackLevelSetter::DoPhase() } } - // The above loop might have moved a BBJ_COND's true target to its next block. - // In such cases, reverse the condition so we can remove a branch. - if (madeChanges) - { - for (BasicBlock* const block : comp->Blocks()) - { - if (block->KindIs(BBJ_COND) && block->CanRemoveJumpToTarget(block->GetTrueTarget(), comp)) - { - // Reverse the jump condition - GenTree* test = block->lastNode(); - assert(test->OperIsConditionalJump()); - GenTree* cond = comp->gtReverseCond(test); - assert(cond == test); // Ensure `gtReverseCond` did not create a new node. - - BasicBlock* newFalseTarget = block->GetTrueTarget(); - BasicBlock* newTrueTarget = block->GetFalseTarget(); - block->SetTrueTarget(newTrueTarget); - block->SetFalseTarget(newFalseTarget); - assert(block->CanRemoveJumpToTarget(newFalseTarget, comp)); - } - } - } - return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } diff --git a/src/coreclr/jit/switchrecognition.cpp b/src/coreclr/jit/switchrecognition.cpp index fa6abd0f23e8bb..7ab33d07c3c34b 100644 --- a/src/coreclr/jit/switchrecognition.cpp +++ b/src/coreclr/jit/switchrecognition.cpp @@ -349,31 +349,6 @@ bool Compiler::optSwitchConvert(BasicBlock* firstBlock, int testsCount, ssize_t* assert((jumpCount > 0) && (jumpCount <= SWITCH_MAX_DISTANCE + 1)); const auto jmpTab = new (this, CMK_BasicBlock) BasicBlock*[jumpCount + 1 /*default case*/]; - // Quirk: lastBlock's false target may have diverged from bbNext. If the false target is behind firstBlock, - // we may create a cycle in the BasicBlock list by setting firstBlock->bbNext to it. - // Add a new BBJ_ALWAYS to the false target to avoid this. - // (We only need this if the false target is behind firstBlock, - // but it's cheaper to just check if the false target has diverged) - // TODO-NoFallThrough: Revisit this quirk? - bool skipPredRemoval = false; - if (!lastBlock->FalseTargetIs(lastBlock->Next())) - { - if (isReversed) - { - assert(lastBlock->FalseTargetIs(blockIfTrue)); - fgRemoveRefPred(blockIfTrue, firstBlock); - blockIfTrue = fgNewBBafter(BBJ_ALWAYS, firstBlock, true, blockIfTrue); - fgAddRefPred(blockIfTrue->GetTarget(), blockIfTrue); - skipPredRemoval = true; - } - else - { - assert(lastBlock->FalseTargetIs(blockIfFalse)); - blockIfFalse = fgNewBBafter(BBJ_ALWAYS, firstBlock, true, blockIfFalse); - fgAddRefPred(blockIfFalse->GetTarget(), blockIfFalse); - } - } - fgHasSwitch = true; firstBlock->GetSwitchTargets()->bbsCount = jumpCount + 1; firstBlock->GetSwitchTargets()->bbsHasDefault = true; @@ -393,10 +368,7 @@ bool Compiler::optSwitchConvert(BasicBlock* firstBlock, int testsCount, ssize_t* } // Unlink blockIfTrue from firstBlock, we're going to link it again in the loop below. - if (!skipPredRemoval) - { - fgRemoveRefPred(blockIfTrue, firstBlock); - } + fgRemoveRefPred(blockIfTrue, firstBlock); for (unsigned i = 0; i < jumpCount; i++) {