JIT: Add explicit successor for BBJ_COND false branch

src/coreclr/jit/assertionprop.cpp

@@ -5558,7 +5558,7 @@ class AssertionPropFlowCallback
             {
                 // Scenario where next block and conditional block, both point to the same block.
                 // In such case, intersect the assertions present on both the out edges of predBlock.
-                assert(predBlock->NextIs(block));
+                assert(predBlock->HasNormalJumpTo(block));
                 BitVecOps::IntersectionD(apTraits, pAssertionOut, predBlock->bbAssertionOut);
 
                 if (VerboseDataflow())

src/coreclr/jit/block.cpp

@@ -1112,7 +1112,7 @@ bool BasicBlock::bbFallsThrough() const
             return false;
 
         case BBJ_COND:
-            return true;
+            return NextIs(GetNormalJumpDest());
 
         case BBJ_CALLFINALLY:
             return !HasFlag(BBF_RETLESS_CALL);

src/coreclr/jit/block.h

@@ -532,6 +532,9 @@ struct BasicBlock : private LIR::Range
         BBehfDesc*  bbJumpEhf;  // BBJ_EHFINALLYRET descriptor
     };
 
+    // Points to the successor of a BBJ_COND block if bbJumpDest is not taken
+    BasicBlock* bbNormalJumpDest;
+
 public:
     static BasicBlock* New(Compiler* compiler);
     static BasicBlock* New(Compiler* compiler, BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr);
@@ -573,11 +576,18 @@ struct BasicBlock : private LIR::Range
 
     void SetNext(BasicBlock* next)
     {
+        // TODO: remove
         bbNext = next;
         if (next)
         {
             next->bbPrev = this;
         }
+
+        // BBJ_COND convenience: This ensures bbNormalJumpDest is always consistent with bbNext.
+        // For now, if a BBJ_COND's bbJumpDest is not taken, we expect to fall through,
+        // so bbNormalJumpDest must be the next block.
+        // TODO: Remove this once we allow bbNormalJumpDest to diverge from bbNext
+        bbNormalJumpDest = next;
     }
 
     bool IsFirst() const
@@ -639,11 +649,39 @@ struct BasicBlock : private LIR::Range
         assert(!HasJumpDest() || HasInitializedJumpDest());
     }
 
+    BasicBlock* GetNormalJumpDest() const
+    {
+        assert(KindIs(BBJ_COND));
+        assert((bbNormalJumpDest != nullptr) || IsLast());
+        return bbNormalJumpDest;
+    }
+
+    void SetNormalJumpDest(BasicBlock* jumpDest)
+    {
+        assert(KindIs(BBJ_COND));
+        assert(jumpDest != nullptr);
+        bbNormalJumpDest = jumpDest;
+    }
+
+    bool HasNormalJumpTo(BasicBlock* jumpDest) const
+    {
+        assert(KindIs(BBJ_COND));
+        assert(jumpDest != nullptr);
+        assert(bbNormalJumpDest != nullptr);
+        return (bbNormalJumpDest == jumpDest);
+    }
+
     void SetJumpKindAndTarget(BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr)
     {
         bbJumpKind = jumpKind;
         bbJumpDest = jumpDest;
 
+        // BBJ_COND convenience: We might be setting this block's jump kind to BBJ_COND.
+        // In that case, set bbNormalJumpDest to bbNext to ensure they are consistent after setting the jump kind.
+        // TODO: Allow bbNormalJumpDest to diverge from bbNext
+        // assert(!IsLast());
+        // bbNormalJumpDest = bbNext;
+
         // If bbJumpKind indicates this block has a jump, bbJumpDest cannot be null
         assert(!HasJumpDest() || HasInitializedJumpDest());
     }

src/coreclr/jit/compiler.hpp

@@ -628,9 +628,9 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
             return BasicBlockVisit::Continue;
 
         case BBJ_COND:
-            RETURN_ON_ABORT(func(bbNext));
+            RETURN_ON_ABORT(func(bbNormalJumpDest));
 
-            if (bbJumpDest != bbNext)
+            if (bbJumpDest != bbNormalJumpDest)
             {
                 RETURN_ON_ABORT(func(bbJumpDest));
             }
@@ -694,9 +694,9 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func)
             return func(bbJumpDest);
 
         case BBJ_COND:
-            RETURN_ON_ABORT(func(bbNext));
+            RETURN_ON_ABORT(func(bbNormalJumpDest));
 
-            if (bbJumpDest != bbNext)
+            if (bbJumpDest != bbNormalJumpDest)
             {
                 RETURN_ON_ABORT(func(bbJumpDest));
             }

src/coreclr/jit/fgbasic.cpp

@@ -2962,7 +2962,8 @@ void Compiler::fgLinkBasicBlocks()
 
                 // The fall-through block is also reachable
                 assert(curBBdesc->KindIs(BBJ_COND));
-                fgAddRefPred<initializingPreds>(curBBdesc->Next(), curBBdesc, oldEdge);
+                curBBdesc->SetNormalJumpDest(curBBdesc->Next());
+                fgAddRefPred<initializingPreds>(curBBdesc->GetNormalJumpDest(), curBBdesc, oldEdge);
                 break;
             }
 
@@ -4242,7 +4243,7 @@ void Compiler::fgCheckBasicBlockControlFlow()
 
             case BBJ_COND: // block conditionally jumps to the target
 
-                fgControlFlowPermitted(blk, blk->Next());
+                fgControlFlowPermitted(blk, blk->GetNormalJumpDest());
 
                 fgControlFlowPermitted(blk, blk->GetJumpDest());
 
@@ -5030,6 +5031,7 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
         // an immediately following block of a BBJ_SWITCH (which has
         // no fall-through path). For this case, simply insert a new
         // fall-through block after 'curr'.
+        // TODO: Once bbNormalJumpDest can diverge from bbNext, this will be unnecessary for BBJ_COND
         newBlock = fgNewBBafter(BBJ_ALWAYS, curr, true /* extendRegion */, /* jumpDest */ succ);
         newBlock->SetFlags(BBF_NONE_QUIRK);
         assert(newBlock->JumpsToNext());
@@ -5386,8 +5388,8 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
                         break;
                     }
 
-                    /* Check if both side of the BBJ_COND now jump to the same block */
-                    if (predBlock->NextIs(succBlock))
+                    /* Check if both sides of the BBJ_COND now jump to the same block */
+                    if (predBlock->HasNormalJumpTo(succBlock))
                     {
                         // Make sure we are replacing "block" with "succBlock" in predBlock->bbJumpDest.
                         noway_assert(predBlock->HasJumpTo(block));
@@ -5433,8 +5435,8 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
                 break;
 
             case BBJ_COND:
-                /* Check for branch to next block */
-                if (bPrev->JumpsToNext())
+                /* Check if both sides of the BBJ_COND now jump to the same block */
+                if (bPrev->HasJumpTo(bPrev->GetNormalJumpDest()))
                 {
                     fgRemoveConditionalJump(bPrev);
                 }
@@ -6336,6 +6338,8 @@ bool Compiler::fgIsBetterFallThrough(BasicBlock* bCur, BasicBlock* bAlt)
     }
 
     // Currently bNext is the fall through for bCur
+    // TODO: Allow bbNormalJumpDest to diverge from bbNext for BBJ_COND
+    assert(!bCur->KindIs(BBJ_COND) || bCur->NextIs(bCur->GetNormalJumpDest()));
     BasicBlock* bNext = bCur->Next();
     noway_assert(bNext != nullptr);
 

src/coreclr/jit/fgdiagnostic.cpp

@@ -2751,7 +2751,7 @@ bool BBPredsChecker::CheckJump(BasicBlock* blockPred, BasicBlock* block)
     switch (blockPred->GetJumpKind())
     {
         case BBJ_COND:
-            assert(blockPred->NextIs(block) || blockPred->HasJumpTo(block));
+            assert(blockPred->HasNormalJumpTo(block) || blockPred->HasJumpTo(block));
             return true;
 
         case BBJ_ALWAYS:
@@ -2957,6 +2957,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: Allow bbNormalJumpDest to diverge from bbNext
+        if (block->KindIs(BBJ_COND))
+        {
+            assert(block->NextIs(block->GetNormalJumpDest()));
+        }
+
         // 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.

src/coreclr/jit/fgehopt.cpp

@@ -2058,7 +2058,7 @@ PhaseStatus Compiler::fgTailMergeThrows()
                 case BBJ_COND:
                 {
                     // Flow to non canonical block could be via fall through or jump or both.
-                    if (predBlock->NextIs(nonCanonicalBlock))
+                    if (predBlock->HasNormalJumpTo(nonCanonicalBlock))
                     {
                         fgTailMergeThrowsFallThroughHelper(predBlock, nonCanonicalBlock, canonicalBlock, predEdge);
                     }
@@ -2134,7 +2134,8 @@ void Compiler::fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock,
                                                   BasicBlock* canonicalBlock,
                                                   FlowEdge*   predEdge)
 {
-    assert(predBlock->NextIs(nonCanonicalBlock));
+    assert(predBlock->KindIs(BBJ_COND));
+    assert(predBlock->HasNormalJumpTo(nonCanonicalBlock));
 
     BasicBlock* const newBlock = fgNewBBafter(BBJ_ALWAYS, predBlock, true, canonicalBlock);
 

src/coreclr/jit/fgflow.cpp

@@ -357,7 +357,7 @@ void Compiler::fgRemoveBlockAsPred(BasicBlock* block)
 
         case BBJ_COND:
             fgRemoveRefPred(block->GetJumpDest(), block);
-            fgRemoveRefPred(block->Next(), block);
+            fgRemoveRefPred(block->GetNormalJumpDest(), block);
             break;
 
         case BBJ_EHFINALLYRET: