From 25efee0de1c72e40222bbfc7e864fc3f9aa3aa8b Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Sun, 31 Dec 2023 19:46:58 -0500 Subject: [PATCH] JIT: Set `bbFalseTarget` upon `BBJ_COND` initialization/modification (#96265) Part of #93020. Previously, bbFalseTarget was hard-coded to match bbNext in BasicBlock::SetNext. We still require bbFalseTarget to point to the next block for BBJ_COND blocks, but I've removed the logic for updating bbFalseTarget from SetNext, and placed calls to SetFalseTarget wherever bbFalseTarget needs to be updated because the BBJ_COND block has been created or moved relative to its false successor. This helps set us up to start removing logic that enforces the block's false successor is the next block. --- src/coreclr/jit/block.cpp | 2 +- src/coreclr/jit/block.h | 12 +-- src/coreclr/jit/fgbasic.cpp | 142 +++++++++++++--------------- src/coreclr/jit/fgehopt.cpp | 1 + src/coreclr/jit/fgopt.cpp | 13 +-- src/coreclr/jit/flowgraph.cpp | 8 +- src/coreclr/jit/helperexpansion.cpp | 19 ++-- src/coreclr/jit/jiteh.cpp | 3 +- src/coreclr/jit/loopcloning.cpp | 12 ++- src/coreclr/jit/lower.cpp | 10 +- src/coreclr/jit/morph.cpp | 7 +- src/coreclr/jit/optimizebools.cpp | 6 +- src/coreclr/jit/optimizer.cpp | 15 ++- 13 files changed, 130 insertions(+), 120 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index e5d22453a6fb0..7d0c731819ea9 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -1123,7 +1123,7 @@ bool BasicBlock::bbFallsThrough() const return false; case BBJ_COND: - return NextIs(GetFalseTarget()); + return true; case BBJ_CALLFINALLY: return !HasFlag(BBF_RETLESS_CALL); diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 252ba578425bd..68f72f30ecb74 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -584,12 +584,6 @@ struct BasicBlock : private LIR::Range { next->bbPrev = this; } - - // BBJ_COND convenience: This ensures bbFalseTarget is always consistent with bbNext. - // For now, if a BBJ_COND's bbTrueTarget is not taken, we expect to fall through, - // so bbFalseTarget must be the next block. - // TODO-NoFallThrough: Remove this once we allow bbFalseTarget to diverge from bbNext - bbFalseTarget = next; } bool IsFirst() const @@ -703,9 +697,9 @@ struct BasicBlock : private LIR::Range void SetCond(BasicBlock* trueTarget) { assert(trueTarget != nullptr); - bbKind = BBJ_COND; - bbTrueTarget = trueTarget; - // TODO-NoFallThrough: also set bbFalseTarget + bbKind = BBJ_COND; + bbTrueTarget = trueTarget; + bbFalseTarget = bbNext; } // Set both the block kind and target. This can clear `bbTarget` when setting diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 5d9777db63e2d..e5b815f186478 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -4761,9 +4761,7 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr) { // We'd like to use fgNewBBafter(), but we need to update the preds list before linking in the new block. // (We need the successors of 'curr' to be correct when we do this.) - BasicBlock* newBlock; - - newBlock = BasicBlock::New(this); + BasicBlock* newBlock = BasicBlock::New(this); // Start the new block with no refs. When we set the preds below, this will get updated correctly. newBlock->bbRefs = 0; @@ -4789,10 +4787,6 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr) } } - // Transfer the kind and target. Do this after the code above, to avoid null-ing out the old targets used by the - // above code. - newBlock->TransferTarget(curr); - newBlock->inheritWeight(curr); // Set the new block's flags. Note that the new block isn't BBF_INTERNAL unless the old block is. @@ -4821,6 +4815,11 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr) // Remove flags from the old block that are no longer possible. curr->RemoveFlags(BBF_HAS_JMP | BBF_RETLESS_CALL); + // Transfer the kind and target. Do this after the code above, to avoid null-ing out the old targets used by the + // above code (and so newBlock->bbNext is valid, so SetCond() can initialize bbFalseTarget if newBlock is a + // BBJ_COND). + newBlock->TransferTarget(curr); + // Default to fallthrough, and add the arc for that. curr->SetKindAndTarget(BBJ_ALWAYS, newBlock); curr->SetFlags(BBF_NONE_QUIRK); @@ -5080,6 +5079,7 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ) if (curr->KindIs(BBJ_COND)) { + curr->SetFalseTarget(curr->Next()); fgReplacePred(succ, curr, newBlock); if (curr->TrueTargetIs(succ)) { @@ -5455,6 +5455,8 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) break; case BBJ_COND: + bPrev->SetFalseTarget(block->Next()); + /* Check if both sides of the BBJ_COND now jump to the same block */ if (bPrev->TrueTargetIs(bPrev->GetFalseTarget())) { @@ -5514,7 +5516,7 @@ void Compiler::fgPrepareCallFinallyRetForRemoval(BasicBlock* block) // fgConnectFallThrough: fix flow from a block that previously had a fall through // // Arguments: -// bSrc - source of fall through (may be null?) +// bSrc - source of fall through // bDst - target of fall through // // Returns: @@ -5523,87 +5525,77 @@ void Compiler::fgPrepareCallFinallyRetForRemoval(BasicBlock* block) // BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) { + assert(bSrc != nullptr); assert(fgPredsComputed); BasicBlock* jmpBlk = nullptr; - /* If bSrc is non-NULL */ + /* If bSrc falls through to a block that is not bDst, we will insert a jump to bDst */ - if (bSrc != nullptr) + if (bSrc->KindIs(BBJ_COND) && !bSrc->NextIs(bDst)) { - /* If bSrc falls through to a block that is not bDst, we will insert a jump to bDst */ - - if (bSrc->bbFallsThrough() && !bSrc->NextIs(bDst)) - { - switch (bSrc->GetKind()) - { - case BBJ_CALLFINALLY: - case BBJ_COND: + // Add a new block after bSrc which jumps to 'bDst' + jmpBlk = fgNewBBafter(BBJ_ALWAYS, bSrc, true, bDst); + bSrc->SetFalseTarget(jmpBlk); + fgAddRefPred(jmpBlk, bSrc, fgGetPredForBlock(bDst, bSrc)); - // Add a new block after bSrc which jumps to 'bDst' - jmpBlk = fgNewBBafter(BBJ_ALWAYS, bSrc, true, bDst); - fgAddRefPred(jmpBlk, bSrc, fgGetPredForBlock(bDst, bSrc)); + // Record the loop number in the new block + jmpBlk->bbNatLoopNum = bSrc->bbNatLoopNum; - // Record the loop number in the new block - jmpBlk->bbNatLoopNum = bSrc->bbNatLoopNum; - - // When adding a new jmpBlk we will set the bbWeight and bbFlags - // - if (fgHaveValidEdgeWeights && fgHaveProfileWeights()) - { - FlowEdge* const newEdge = fgGetPredForBlock(jmpBlk, bSrc); - - jmpBlk->bbWeight = (newEdge->edgeWeightMin() + newEdge->edgeWeightMax()) / 2; - if (bSrc->bbWeight == BB_ZERO_WEIGHT) - { - jmpBlk->bbWeight = BB_ZERO_WEIGHT; - } - - if (jmpBlk->bbWeight == BB_ZERO_WEIGHT) - { - jmpBlk->SetFlags(BBF_RUN_RARELY); - } - - weight_t weightDiff = (newEdge->edgeWeightMax() - newEdge->edgeWeightMin()); - weight_t slop = BasicBlock::GetSlopFraction(bSrc, bDst); - // - // If the [min/max] values for our edge weight is within the slop factor - // then we will set the BBF_PROF_WEIGHT flag for the block - // - if (weightDiff <= slop) - { - jmpBlk->SetFlags(BBF_PROF_WEIGHT); - } - } - else - { - // We set the bbWeight to the smaller of bSrc->bbWeight or bDst->bbWeight - if (bSrc->bbWeight < bDst->bbWeight) - { - jmpBlk->bbWeight = bSrc->bbWeight; - jmpBlk->CopyFlags(bSrc, BBF_RUN_RARELY); - } - else - { - jmpBlk->bbWeight = bDst->bbWeight; - jmpBlk->CopyFlags(bDst, BBF_RUN_RARELY); - } - } + // When adding a new jmpBlk we will set the bbWeight and bbFlags + // + if (fgHaveValidEdgeWeights && fgHaveProfileWeights()) + { + FlowEdge* const newEdge = fgGetPredForBlock(jmpBlk, bSrc); - fgReplacePred(bDst, bSrc, jmpBlk); + jmpBlk->bbWeight = (newEdge->edgeWeightMin() + newEdge->edgeWeightMax()) / 2; + if (bSrc->bbWeight == BB_ZERO_WEIGHT) + { + jmpBlk->bbWeight = BB_ZERO_WEIGHT; + } - JITDUMP("Added an unconditional jump to " FMT_BB " after block " FMT_BB "\n", - jmpBlk->GetTarget()->bbNum, bSrc->bbNum); - break; + if (jmpBlk->bbWeight == BB_ZERO_WEIGHT) + { + jmpBlk->SetFlags(BBF_RUN_RARELY); + } - default: - noway_assert(!"Unexpected bbKind"); - break; + weight_t weightDiff = (newEdge->edgeWeightMax() - newEdge->edgeWeightMin()); + weight_t slop = BasicBlock::GetSlopFraction(bSrc, bDst); + // + // If the [min/max] values for our edge weight is within the slop factor + // then we will set the BBF_PROF_WEIGHT flag for the block + // + if (weightDiff <= slop) + { + jmpBlk->SetFlags(BBF_PROF_WEIGHT); } } - else if (bSrc->KindIs(BBJ_ALWAYS) && bSrc->HasInitializedTarget() && bSrc->JumpsToNext()) + else { - bSrc->SetFlags(BBF_NONE_QUIRK); + // We set the bbWeight to the smaller of bSrc->bbWeight or bDst->bbWeight + if (bSrc->bbWeight < bDst->bbWeight) + { + jmpBlk->bbWeight = bSrc->bbWeight; + jmpBlk->CopyFlags(bSrc, BBF_RUN_RARELY); + } + else + { + jmpBlk->bbWeight = bDst->bbWeight; + jmpBlk->CopyFlags(bDst, BBF_RUN_RARELY); + } } + + fgReplacePred(bDst, bSrc, jmpBlk); + + JITDUMP("Added an unconditional jump to " FMT_BB " after block " FMT_BB "\n", jmpBlk->GetTarget()->bbNum, + bSrc->bbNum); + } + else if (bSrc->KindIs(BBJ_ALWAYS) && bSrc->HasInitializedTarget() && bSrc->JumpsToNext()) + { + bSrc->SetFlags(BBF_NONE_QUIRK); + } + else if (bSrc->KindIs(BBJ_COND) && bSrc->NextIs(bDst)) + { + bSrc->SetFalseTarget(bDst); } return jmpBlk; diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index c6b6fa5688441..a4c38e122e2b7 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -2118,6 +2118,7 @@ void Compiler::fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock, assert(predBlock->FalseTargetIs(nonCanonicalBlock)); BasicBlock* const newBlock = fgNewBBafter(BBJ_ALWAYS, predBlock, true, canonicalBlock); + predBlock->SetFalseTarget(newBlock); JITDUMP("*** " FMT_BB " now falling through to empty " FMT_BB " and then to " FMT_BB "\n", predBlock->bbNum, newBlock->bbNum, canonicalBlock->bbNum); diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 958d4c2587b91..5a5fe92155c4e 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -3703,17 +3703,17 @@ bool Compiler::fgOptimizeUncondBranchToSimpleCond(BasicBlock* block, BasicBlock* fgInsertStmtAtEnd(block, cloneStmt); } + // add an unconditional block after this block to jump to the target block's fallthrough block + // + assert(!target->IsLast()); + BasicBlock* next = fgNewBBafter(BBJ_ALWAYS, block, true, target->GetFalseTarget()); + // Fix up block's flow // block->SetCond(target->GetTrueTarget()); fgAddRefPred(block->GetTrueTarget(), block); fgRemoveRefPred(target, block); - // add an unconditional block after this block to jump to the target block's fallthrough block - // - assert(!target->IsLast()); - BasicBlock* next = fgNewBBafter(BBJ_ALWAYS, block, true, target->GetFalseTarget()); - // The new block 'next' will inherit its weight from 'block' // next->inheritWeight(block); @@ -4118,7 +4118,6 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump) bJump->CopyFlags(bDest, BBF_COPY_PROPAGATE); bJump->SetCond(bDestNormalTarget); - bJump->SetFalseTarget(bJump->Next()); /* Update bbRefs and bbPreds */ @@ -6199,6 +6198,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) if (bDest->KindIs(BBJ_COND)) { BasicBlock* const bFixup = fgNewBBafter(BBJ_ALWAYS, bDest, true, bDestNext); + bDest->SetFalseTarget(bFixup); bFixup->inheritWeight(bDestNext); fgRemoveRefPred(bDestNext, bDest); @@ -6234,6 +6234,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) // Optimize the Conditional JUMP to go to the new target block->SetTrueTarget(bNext->GetTarget()); + block->SetFalseTarget(bNext->Next()); fgAddRefPred(bNext->GetTarget(), block, fgRemoveRefPred(bNext->GetTarget(), bNext)); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 233d719c7308f..14035b02c9e94 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -270,7 +270,10 @@ BasicBlock* Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block) BasicBlock* top = block; unsigned char lpIndexFallThrough = BasicBlock::NOT_IN_LOOP; - if (top->KindIs(BBJ_COND)) + BBKinds oldJumpKind = top->GetKind(); + unsigned char lpIndex = top->bbNatLoopNum; + + if (oldJumpKind == BBJ_COND) { lpIndexFallThrough = top->GetFalseTarget()->bbNatLoopNum; } @@ -283,9 +286,6 @@ BasicBlock* Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block) bottom->TransferTarget(top); - BBKinds oldJumpKind = top->GetKind(); - unsigned char lpIndex = top->bbNatLoopNum; - // Update block flags const BasicBlockFlags originalFlags = top->GetFlagsRaw() | BBF_GC_SAFE_POINT; diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index 7b4c4fe4610d9..bcc6982d8e1b8 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -286,7 +286,7 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm // Fallback basic block GenTree* fallbackValueDef = gtNewStoreLclVarNode(rtLookupLcl->GetLclNum(), call); BasicBlock* fallbackBb = - fgNewBBFromTreeAfter(BBJ_ALWAYS, nullcheckBb, fallbackValueDef, debugInfo, nullcheckBb->GetFalseTarget(), true); + fgNewBBFromTreeAfter(BBJ_ALWAYS, nullcheckBb, fallbackValueDef, debugInfo, nullcheckBb->Next(), true); assert(fallbackBb->JumpsToNext()); fallbackBb->SetFlags(BBF_NONE_QUIRK); @@ -298,6 +298,9 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm GenTree* fastpathValueDef = gtNewStoreLclVarNode(rtLookupLcl->GetLclNum(), fastPathValueClone); BasicBlock* fastPathBb = fgNewBBFromTreeAfter(BBJ_ALWAYS, nullcheckBb, fastpathValueDef, debugInfo, block); + // Set nullcheckBb's false jump target + nullcheckBb->SetFalseTarget(fastPathBb); + BasicBlock* sizeCheckBb = nullptr; if (needsSizeCheck) { @@ -339,6 +342,7 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm GenTree* jtrue = gtNewOperNode(GT_JTRUE, TYP_VOID, sizeCheck); // sizeCheckBb fails - jump to fallbackBb sizeCheckBb = fgNewBBFromTreeAfter(BBJ_COND, prevBb, jtrue, debugInfo, fallbackBb); + sizeCheckBb->SetFalseTarget(nullcheckBb); } // @@ -780,11 +784,13 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement* gtNewStoreLclVarNode(threadStaticBlockLclNum, gtCloneExpr(threadStaticBlockBaseLclValueUse)); BasicBlock* fastPathBb = fgNewBBFromTreeAfter(BBJ_ALWAYS, fallbackBb, fastPathValueDef, debugInfo, block, true); - // Set maxThreadStaticBlocksCondBB's true jump target + // Set maxThreadStaticBlocksCondBB's jump targets maxThreadStaticBlocksCondBB->SetTrueTarget(fallbackBb); + maxThreadStaticBlocksCondBB->SetFalseTarget(threadStaticBlockNullCondBB); - // Set threadStaticBlockNullCondBB's true jump target + // Set threadStaticBlockNullCondBB's jump targets threadStaticBlockNullCondBB->SetTrueTarget(fastPathBb); + threadStaticBlockNullCondBB->SetFalseTarget(fallbackBb); // // Update preds in all new blocks @@ -1095,8 +1101,7 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G // Fallback basic block // TODO-CQ: for JIT we can replace the original call with CORINFO_HELP_INITCLASS // that only accepts a single argument - BasicBlock* helperCallBb = - fgNewBBFromTreeAfter(BBJ_ALWAYS, isInitedBb, call, debugInfo, isInitedBb->GetFalseTarget(), true); + BasicBlock* helperCallBb = fgNewBBFromTreeAfter(BBJ_ALWAYS, isInitedBb, call, debugInfo, isInitedBb->Next(), true); assert(helperCallBb->JumpsToNext()); helperCallBb->SetFlags(BBF_NONE_QUIRK); @@ -1172,6 +1177,7 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G fgAddRefPred(isInitedBb, prevBb); // Both fastPathBb and helperCallBb have a single common pred - isInitedBb + isInitedBb->SetFalseTarget(helperCallBb); fgAddRefPred(helperCallBb, isInitedBb); // @@ -1451,7 +1457,7 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock, // In theory, we could just emit the const U8 data to the data section and use GT_BLK here // but that would be a bit less efficient since we would have to load the data from memory. // - BasicBlock* fastpathBb = fgNewBBafter(BBJ_ALWAYS, lengthCheckBb, true, lengthCheckBb->GetFalseTarget()); + BasicBlock* fastpathBb = fgNewBBafter(BBJ_ALWAYS, lengthCheckBb, true, lengthCheckBb->Next()); assert(fastpathBb->JumpsToNext()); fastpathBb->SetFlags(BBF_INTERNAL | BBF_NONE_QUIRK); @@ -1514,6 +1520,7 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock, assert(prevBb->JumpsToNext()); fgAddRefPred(lengthCheckBb, prevBb); // lengthCheckBb has two successors: block and fastpathBb + lengthCheckBb->SetFalseTarget(fastpathBb); fgAddRefPred(fastpathBb, lengthCheckBb); fgAddRefPred(block, lengthCheckBb); // fastpathBb flows into block diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 846bf7032e202..f1bdeeb22265f 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -2256,8 +2256,9 @@ bool Compiler::fgNormalizeEHCase2() // fgReplaceJumpTarget(predBlock, newTryStart, insertBeforeBlk); - if (predBlock->NextIs(newTryStart) && predBlock->bbFallsThrough()) + if (predBlock->NextIs(newTryStart) && predBlock->KindIs(BBJ_COND)) { + predBlock->SetFalseTarget(newTryStart); fgRemoveRefPred(insertBeforeBlk, predBlock); fgAddRefPred(newTryStart, predBlock); } diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 7b85126a20422..5e6c1b89ace0e 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -867,9 +867,10 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler* JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", newBlk->bbNum, newBlk->GetTrueTarget()->bbNum); comp->fgAddRefPred(newBlk->GetTrueTarget(), newBlk); - if (insertAfter->bbFallsThrough()) + if (insertAfter->KindIs(BBJ_COND)) { JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", insertAfter->bbNum, newBlk->bbNum); + insertAfter->SetFalseTarget(newBlk); comp->fgAddRefPred(newBlk, insertAfter); } @@ -2081,12 +2082,11 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex // bottomNext BasicBlock* bottom = loop->GetLexicallyBottomMostBlock(); BasicBlock* newPred = bottom; - if (bottom->bbFallsThrough()) + if (bottom->KindIs(BBJ_COND)) { - // Once bbFalseTarget can diverge from bbNext, BBJ_COND blocks will "fall through" - // only if bbFalseTarget is still the next block - assert(!bottom->KindIs(BBJ_COND) || bottom->NextIs(bottom->GetFalseTarget())); + // TODO-NoFallThrough: Shouldn't need new BBJ_ALWAYS block once bbFalseTarget can diverge from bbNext BasicBlock* bottomNext = bottom->Next(); + assert(bottom->FalseTargetIs(bottomNext)); JITDUMP("Create branch around cloned loop\n"); BasicBlock* bottomRedirBlk = fgNewBBafter(BBJ_ALWAYS, bottom, /*extendRegion*/ true, bottomNext); JITDUMP("Adding " FMT_BB " after " FMT_BB "\n", bottomRedirBlk->bbNum, bottom->bbNum); @@ -2095,6 +2095,7 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex // This is in the scope of a surrounding loop, if one exists -- the parent of the loop we're cloning. bottomRedirBlk->bbNatLoopNum = ambientLoop; + bottom->SetFalseTarget(bottomRedirBlk); fgAddRefPred(bottomRedirBlk, bottom); JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", bottom->bbNum, bottomRedirBlk->bbNum); fgReplacePred(bottomNext, bottom, bottomRedirBlk); @@ -2306,6 +2307,7 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex // And make sure we insert a pred link for the final fallthrough into the fast preheader. assert(condLast->NextIs(fastPreheader)); + condLast->SetFalseTarget(fastPreheader); fgAddRefPred(fastPreheader, condLast); // Don't unroll loops that we've cloned -- the unroller expects any loop it should unroll to diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index adee5fd745625..26addf635a8ec 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -1074,6 +1074,7 @@ GenTree* Lowering::LowerSwitch(GenTree* node) { BasicBlock* newBlock = comp->fgNewBBafter(BBJ_ALWAYS, currentBlock, true, currentBlock->Next()); newBlock->SetFlags(BBF_NONE_QUIRK); + currentBlock->SetFalseTarget(newBlock); comp->fgAddRefPred(newBlock, currentBlock); // The fall-through predecessor. currentBlock = newBlock; currentBBRange = &LIR::AsRange(currentBlock); @@ -1313,9 +1314,6 @@ bool Lowering::TryLowerSwitchToBitTest( // GenCondition::C generates JC so we jump to bbCase1 when the bit is set bbSwitchCondition = GenCondition::C; bbSwitch->SetCond(bbCase1); - - comp->fgAddRefPred(bbCase0, bbSwitch); - comp->fgAddRefPred(bbCase1, bbSwitch); } else { @@ -1324,11 +1322,11 @@ bool Lowering::TryLowerSwitchToBitTest( // GenCondition::NC generates JNC so we jump to bbCase0 when the bit is not set bbSwitchCondition = GenCondition::NC; bbSwitch->SetCond(bbCase0); - - comp->fgAddRefPred(bbCase0, bbSwitch); - comp->fgAddRefPred(bbCase1, bbSwitch); } + comp->fgAddRefPred(bbCase0, bbSwitch); + comp->fgAddRefPred(bbCase1, bbSwitch); + var_types bitTableType = (bitCount <= (genTypeSize(TYP_INT) * 8)) ? TYP_INT : TYP_LONG; GenTree* bitTableIcon = comp->gtNewIconNode(bitTable, bitTableType); diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 75f885ac8d990..2d323638ab028 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14677,9 +14677,12 @@ bool Compiler::fgExpandQmarkForCastInstOf(BasicBlock* block, Statement* stmt) block->SetTarget(asgBlock); fgAddRefPred(asgBlock, block); fgAddRefPred(cond1Block, asgBlock); + fgAddRefPred(remainderBlock, helperBlock); + + cond1Block->SetFalseTarget(cond2Block); + cond2Block->SetFalseTarget(helperBlock); fgAddRefPred(cond2Block, cond1Block); fgAddRefPred(helperBlock, cond2Block); - fgAddRefPred(remainderBlock, helperBlock); fgAddRefPred(remainderBlock, cond1Block); fgAddRefPred(remainderBlock, cond2Block); @@ -14898,10 +14901,10 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt) // bbj_cond(true) // gtReverseCond(condExpr); - condBlock->SetCond(elseBlock); thenBlock = fgNewBBafter(BBJ_ALWAYS, condBlock, true, remainderBlock); thenBlock->SetFlags(propagateFlagsToAll); + condBlock->SetCond(elseBlock); if (!block->HasFlag(BBF_INTERNAL)) { thenBlock->RemoveFlags(BBF_INTERNAL); diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index e5d366548e53a..b96c4ae024e40 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1298,6 +1298,7 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() if (optReturnBlock) { + assert(m_b1->KindIs(BBJ_COND)); assert(m_b2->KindIs(BBJ_RETURN)); assert(m_b1->FalseTargetIs(m_b2)); assert(m_b3 != nullptr); @@ -1316,10 +1317,11 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() { // Update bbRefs and bbPreds // - // Replace pred 'm_b2' for 'm_b2->bbNext' with 'm_b1' - // Remove pred 'm_b2' for 'm_b2->bbTarget' + // Replace pred 'm_b2' for 'm_b2->bbFalseTarget' with 'm_b1' + // Remove pred 'm_b2' for 'm_b2->bbTrueTarget' m_comp->fgReplacePred(m_b2->GetFalseTarget(), m_b2, m_b1); m_comp->fgRemoveRefPred(m_b2->GetTrueTarget(), m_b2); + m_b1->SetFalseTarget(m_b2->GetFalseTarget()); } // Get rid of the second block diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e2b65109ffed7..460ba61f3c027 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2270,6 +2270,7 @@ class LoopSearch // Redirect the Conditional JUMP to go to `oldNext` block->SetTrueTarget(oldNext); + block->SetFalseTarget(newNext); } else { @@ -2981,6 +2982,7 @@ bool Compiler::optCanonicalizeLoop(unsigned char loopInd) fgSetEHRegionForNewLoopHead(newH, t); + h->SetFalseTarget(newH); fgRemoveRefPred(t, h); fgAddRefPred(t, newH); fgAddRefPred(newH, h); @@ -3162,6 +3164,7 @@ bool Compiler::optCanonicalizeLoopCore(unsigned char loopInd, LoopCanonicalizati { BasicBlock* const hj = h->GetTrueTarget(); assert((hj->bbNum < t->bbNum) || (hj->bbNum > b->bbNum)); + h->SetFalseTarget(newT); } else { @@ -4373,15 +4376,19 @@ PhaseStatus Compiler::optUnrollLoops() // BasicBlock* const clonedTop = blockMap[loop.lpTop]; BasicBlock* clonedTopPrev = clonedTop->Prev(); - assert(clonedTopPrev->KindIs(BBJ_ALWAYS, BBJ_COND)); if (clonedTopPrev->KindIs(BBJ_ALWAYS)) { assert(!clonedTopPrev->HasInitializedTarget()); clonedTopPrev->SetTarget(clonedTop); } + else + { + assert(clonedTopPrev->KindIs(BBJ_COND)); + clonedTopPrev->SetFalseTarget(clonedTop); + } - fgAddRefPred(clonedTop, clonedTop->Prev()); + fgAddRefPred(clonedTop, clonedTopPrev); /* update the new value for the unrolled iterator */ @@ -5023,6 +5030,7 @@ bool Compiler::optInvertWhileLoop(BasicBlock* block) // Update pred info // + bNewCond->SetFalseTarget(bTop); fgAddRefPred(bJoin, bNewCond); fgAddRefPred(bTop, bNewCond); @@ -8227,7 +8235,8 @@ bool Compiler::fgCreateLoopPreHeader(unsigned lnum) } else { - noway_assert((entry == top) && (predBlock == head) && predBlock->FalseTargetIs(preHead)); + noway_assert((entry == top) && (predBlock == head) && predBlock->NextIs(preHead)); + predBlock->SetFalseTarget(preHead); } fgRemoveRefPred(entry, predBlock); fgAddRefPred(preHead, predBlock);