JIT: move funclet separation later (#108456)

Defer funclet separation until after we've run the optimization phases.
This is a prerequisite for introducing a late EH opts phase that will
remove now-empty finallies or faults.

Also update `fgNewBBAtTryRegionEnd` so that the new block's enclosing region
is the try, not some handler region nested within the try (we see this
case now during loop canonicalization, because we haven't yet moved the
handlers).

src/coreclr/jit/compiler.cpp

@@ -4867,13 +4867,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     //
     DoPhase(this, PHASE_COMPUTE_BLOCK_WEIGHTS, &Compiler::fgComputeBlockWeights);
 
-    if (UsesFunclets())
-    {
-        // Create funclets from the EH handlers.
-        //
-        DoPhase(this, PHASE_CREATE_FUNCLETS, &Compiler::fgCreateFunclets);
-    }
-
     if (opts.OptimizationEnabled())
     {
         // Invert loops
@@ -5141,6 +5134,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     DoPhase(this, PHASE_STRESS_SPLIT_TREE, &Compiler::StressSplitTree);
 #endif
 
+    if (UsesFunclets())
+    {
+        // Create funclets from the EH handlers.
+        //
+        DoPhase(this, PHASE_CREATE_FUNCLETS, &Compiler::fgCreateFunclets);
+    }
+
     // Expand casts
     DoPhase(this, PHASE_EXPAND_CASTS, &Compiler::fgLateCastExpansion);
 

src/coreclr/jit/fgbasic.cpp

@@ -5370,8 +5370,6 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
 //   renumber the blocks, none of them actually change number, but we shrink the
 //   maximum assigned block number. This affects the block set epoch).
 //
-//   As a consequence of renumbering, block pred lists may need to be reordered.
-//
 bool Compiler::fgRenumberBlocks()
 {
     assert(fgPredsComputed);
@@ -6725,18 +6723,14 @@ BasicBlock* Compiler::fgNewBBinRegionWorker(BBKinds     jumpKind,
 // Returns:
 //    The new block
 //
-// Notes:
-//    newBlock will be in the try region specified by tryIndex, which may not necessarily
-//    be the same as oldTryLast->getTryIndex() if the latter is a child region.
-//    However, newBlock and oldTryLast will be in the same handler region.
-//
 BasicBlock* Compiler::fgNewBBatTryRegionEnd(BBKinds jumpKind, unsigned tryIndex)
 {
     EHblkDsc*         HBtab      = ehGetDsc(tryIndex);
+    BasicBlock* const oldTryBeg  = HBtab->ebdTryBeg;
     BasicBlock* const oldTryLast = HBtab->ebdTryLast;
     BasicBlock* const newBlock   = fgNewBBafter(jumpKind, oldTryLast, /* extendRegion */ false);
     newBlock->setTryIndex(tryIndex);
-    newBlock->copyHndIndex(oldTryLast);
+    newBlock->copyHndIndex(oldTryBeg);
 
     // Update this try region's (and all parent try regions') last block pointer
     //
@@ -6746,22 +6740,8 @@ BasicBlock* Compiler::fgNewBBatTryRegionEnd(BBKinds jumpKind, unsigned tryIndex)
         fgSetTryEnd(HBtab, newBlock);
     }
 
-    // If we inserted newBlock at the end of a handler region, repeat the above pass for handler regions
-    //
-    if (newBlock->hasHndIndex())
-    {
-        const unsigned hndIndex = newBlock->getHndIndex();
-        HBtab                   = ehGetDsc(hndIndex);
-        for (unsigned XTnum = hndIndex; (XTnum < compHndBBtabCount) && (HBtab->ebdHndLast == oldTryLast);
-             XTnum++, HBtab++)
-        {
-            assert((XTnum == hndIndex) || (XTnum == ehGetEnclosingHndIndex(XTnum - 1)));
-            fgSetHndEnd(HBtab, newBlock);
-        }
-    }
-
     assert(newBlock->getTryIndex() == tryIndex);
-    assert(BasicBlock::sameHndRegion(newBlock, oldTryLast));
+    assert(BasicBlock::sameHndRegion(newBlock, oldTryBeg));
     return newBlock;
 }
 

src/coreclr/jit/fgopt.cpp

@@ -3286,9 +3286,8 @@ bool Compiler::fgExpandRarelyRunBlocks()
 #endif
 
 //-----------------------------------------------------------------------------
-// fgReorderBlocks: reorder blocks to favor frequent fall through paths,
-//   move rare blocks to the end of the method/eh region, and move
-//   funclets to the ends of methods.
+// fgReorderBlocks: reorder blocks to favor frequent fall through paths
+//   and move rare blocks to the end of the method/eh region.
 //
 // Arguments:
 //   useProfile - if true, use profile data (if available) to more aggressively
@@ -3306,8 +3305,6 @@ bool Compiler::fgReorderBlocks(bool useProfile)
 {
     noway_assert(opts.compDbgCode == false);
 
-    assert(UsesFunclets() == fgFuncletsCreated);
-
     // We can't relocate anything if we only have one block
     if (fgFirstBB->IsLast())
     {