JIT: Try to maintain fallthrough in Compiler::fgSplitEdge

src/coreclr/jit/fgbasic.cpp

@@ -4860,44 +4860,41 @@ BasicBlock* Compiler::fgSplitBlockAtBeginning(BasicBlock* curr)
 
 //------------------------------------------------------------------------
 // fgSplitEdge: Splits the edge between a block 'curr' and its successor 'succ' by creating a new block
-//              that replaces 'succ' as a successor of 'curr', and which branches unconditionally
-//              to (or falls through to) 'succ'. Note that for a BBJ_COND block 'curr',
-//              'succ' might be the fall-through path or the branch path from 'curr'.
+//              that replaces 'succ' as a successor of 'curr', and which branches unconditionally to 'succ'.
 //
 // Arguments:
 //    curr - A block which branches to 'succ'
 //    succ - The target block
 //
 // Return Value:
-//    Returns a new block, that is a successor of 'curr' and which branches unconditionally to 'succ'
+//    Returns a new block, that is a successor of 'curr' and branches unconditionally to 'succ'
 //
 // Assumptions:
 //    'curr' must have a bbKind of BBJ_COND, BBJ_ALWAYS, or BBJ_SWITCH
 //
 // Notes:
 //    The returned block is empty.
-//    Can be invoked before pred lists are built.
-
+//
 BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
 {
     assert(curr->KindIs(BBJ_COND, BBJ_SWITCH, BBJ_ALWAYS));
     assert(fgPredsComputed);
     assert(fgGetPredForBlock(succ, curr) != nullptr);
 
     BasicBlock* newBlock;
-    if (curr->NextIs(succ))
+
+    // Place 'newBlock' before 'succ' if we can do so without breaking EH invariants.
+    // This avoids disrupting existing fallthrough out of 'curr' if it has multiple successors.
+    // Else, place 'newBlock' after 'curr'; at the very least, 'curr' will fall through into 'newBlock'.
+    if (BasicBlock::sameEHRegion(curr, succ) && !bbIsTryBeg(succ))
     {
-        // The successor is the fall-through path of a BBJ_COND, or
-        // 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'.
-        newBlock = fgNewBBafter(BBJ_ALWAYS, curr, true /* extendRegion */);
+        newBlock = fgNewBBbefore(BBJ_ALWAYS, succ, /* extendRegion */ true);
     }
     else
     {
-        // The new block always jumps to 'succ'
-        newBlock = fgNewBBinRegion(BBJ_ALWAYS, curr, /* isRunRarely */ curr->isRunRarely());
+        newBlock = fgNewBBafter(BBJ_ALWAYS, curr, /* extendRegion */ true);
     }
+
     newBlock->CopyFlags(curr, succ->GetFlagsRaw() & BBF_BACKWARD_JUMP);
 
     // Async resumption stubs are permitted to branch into EH regions, so if we
@@ -4915,16 +4912,10 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
     newBlock->SetTargetEdge(newSuccEdge);
 
     // Set weight for newBlock
-    //
-    FlowEdge* const currNewEdge = fgGetPredForBlock(newBlock, curr);
-    newBlock->bbWeight          = currNewEdge->getLikelyWeight();
+    assert(newBlock->bbPreds != nullptr);
+    newBlock->bbWeight = newBlock->bbPreds->getLikelyWeight();
     newBlock->CopyFlags(curr, BBF_PROF_WEIGHT);
 
-    if (newBlock->bbWeight == BB_ZERO_WEIGHT)
-    {
-        newBlock->bbSetRunRarely();
-    }
-
     // The bbLiveIn and bbLiveOut are both equal to the bbLiveIn of 'succ'
     if (fgLocalVarLivenessDone)
     {

src/coreclr/jit/lsra.cpp

@@ -9561,10 +9561,8 @@ void LinearScan::resolveEdge(BasicBlock*      fromBlock,
             block = toBlock;
             break;
         case ResolveCritical:
-            // fgSplitEdge may add one or two BasicBlocks.  It returns the block that splits
-            // the edge from 'fromBlock' and 'toBlock', but if it inserts that block right after
-            // a block with a fall-through it will have to create another block to handle that edge.
-            // These new blocks can be mapped to existing blocks in order to correctly handle
+            // fgSplitEdge creates an intermediary block between 'fromBlock' and 'toBlock'.
+            // The new block can be mapped to existing blocks in order to correctly handle
             // the calls to recordVarLocationsAtStartOfBB() from codegen.  That mapping is handled
             // in resolveEdges(), after all the edge resolution has been done (by calling this
             // method for each edge).