JIT: build pred lists before the eh opt phases (#80856)

Fix the EH opts to properly maintain pred lists, and move building of the
pred lists to just before the EH opts.

Contributes to #80193.

src/coreclr/jit/compiler.cpp

@@ -4482,6 +4482,24 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     //
     DoPhase(this, PHASE_MORPH_ADD_INTERNAL, &Compiler::fgAddInternal);
 
+    // Compute bbNum, bbRefs and bbPreds
+    //
+    // This is the first time full (not cheap) preds will be computed.
+    // And, if we have profile data, we can now check integrity.
+    //
+    // From this point on the flowgraph information such as bbNum,
+    // bbRefs or bbPreds has to be kept updated.
+    //
+    auto computePredsPhase = [this]() {
+        JITDUMP("\nRenumbering the basic blocks for fgComputePred\n");
+        fgRenumberBlocks();
+        noway_assert(!fgComputePredsDone);
+        fgComputePreds();
+        // Enable flow graph checks
+        activePhaseChecks |= PhaseChecks::CHECK_FG;
+    };
+    DoPhase(this, PHASE_COMPUTE_PREDS, computePredsPhase);
+
     // Remove empty try regions
     //
     DoPhase(this, PHASE_EMPTY_TRY, &Compiler::fgRemoveEmptyTry);
@@ -4566,25 +4584,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
     }
 #endif
 
-    // Compute bbNum, bbRefs and bbPreds
-    //
-    // This is the first time full (not cheap) preds will be computed.
-    // And, if we have profile data, we can now check integrity.
-    //
-    // From this point on the flowgraph information such as bbNum,
-    // bbRefs or bbPreds has to be kept updated.
-    //
-    auto computePredsPhase = [this]() {
-        JITDUMP("\nRenumbering the basic blocks for fgComputePred\n");
-        fgRenumberBlocks();
-        noway_assert(!fgComputePredsDone);
-        fgComputePreds();
-        // Enable flow graph checks
-        activePhaseChecks |= PhaseChecks::CHECK_FG;
-    };
-    DoPhase(this, PHASE_COMPUTE_PREDS, computePredsPhase);
-
-    // Now that we have pred lists, do some flow-related optimizations
+    // Do some flow-related optimizations
     //
     if (opts.OptimizationEnabled())
     {

src/coreclr/jit/fgehopt.cpp

@@ -37,8 +37,8 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
     assert(!fgFuncletsCreated);
 #endif // FEATURE_EH_FUNCLETS
 
-    // Assume we don't need to update the bbPreds lists.
-    assert(!fgComputePredsDone);
+    // We need to update the bbPreds lists.
+    assert(fgComputePredsDone);
 
     if (compHndBBtabCount == 0)
     {
@@ -167,13 +167,13 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
 
                 // Ref count updates.
                 fgAddRefPred(postTryFinallyBlock, currentBlock);
-                // fgRemoveRefPred(firstBlock, currentBlock);
+                fgRemoveRefPred(firstBlock, currentBlock);
 
                 // Delete the leave block, which should be marked as
-                // keep always.
+                // keep always and have the sole finally block as a pred.
                 assert((leaveBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0);
                 nextBlock = leaveBlock->bbNext;
-
+                fgRemoveRefPred(leaveBlock, firstBlock);
                 leaveBlock->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS;
                 fgRemoveBlock(leaveBlock, /* unreachable */ true);
 
@@ -292,8 +292,8 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
     assert(!fgFuncletsCreated);
 #endif // FEATURE_EH_FUNCLETS
 
-    // Assume we don't need to update the bbPreds lists.
-    assert(!fgComputePredsDone);
+    // We need to update the bbPreds lists.
+    assert(fgComputePredsDone);
 
     bool enableRemoveEmptyTry = true;
 
@@ -545,6 +545,7 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
                     block->bbJumpKind = BBJ_ALWAYS;
                     block->bbJumpDest = continuation;
                     fgAddRefPred(continuation, block);
+                    fgRemoveRefPred(leave, block);
                 }
             }
 
@@ -570,6 +571,13 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
         // the EH region indices of any nested EH in the (former) handler.
         fgRemoveEHTableEntry(XTnum);
 
+        // (7) The handler entry has an artificial extra ref count. Remove it.
+        // There also should be one normal ref, from the try, and the handler
+        // may contain internal branches back to its start. So the ref count
+        // should currently be at least 2.
+        assert(firstHandlerBlock->bbRefs >= 2);
+        firstHandlerBlock->bbRefs -= 1;
+
         // Another one bites the dust...
         emptyCount++;
     }
@@ -621,8 +629,8 @@ PhaseStatus Compiler::fgCloneFinally()
     assert(!fgFuncletsCreated);
 #endif // FEATURE_EH_FUNCLETS
 
-    // Assume we don't need to update the bbPreds lists.
-    assert(!fgComputePredsDone);
+    // We need to update the bbPreds lists.
+    assert(fgComputePredsDone);
 
     bool enableCloning = true;
 
@@ -1134,7 +1142,7 @@ PhaseStatus Compiler::fgCloneFinally()
             else
             {
                 optCopyBlkDest(block, newBlock);
-                optRedirectBlock(newBlock, &blockMap);
+                optRedirectBlock(newBlock, &blockMap, RedirectBlockOption::AddToPredLists);
             }
         }
 
@@ -1173,13 +1181,22 @@ PhaseStatus Compiler::fgCloneFinally()
 
                         // Ref count updates.
                         fgAddRefPred(firstCloneBlock, currentBlock);
-                        // fgRemoveRefPred(firstBlock, currentBlock);
+                        fgRemoveRefPred(firstBlock, currentBlock);
 
                         // Delete the leave block, which should be marked as
                         // keep always.
                         assert((leaveBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0);
                         nextBlock = leaveBlock->bbNext;
 
+                        // All preds should be BBJ_EHFINALLYRETs from the finally.
+                        for (BasicBlock* const leavePred : leaveBlock->PredBlocks())
+                        {
+                            assert(leavePred->bbJumpKind == BBJ_EHFINALLYRET);
+                            assert(leavePred->getHndIndex() == XTnum);
+                        }
+
+                        leaveBlock->bbRefs  = 0;
+                        leaveBlock->bbPreds = nullptr;
                         leaveBlock->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS;
                         fgRemoveBlock(leaveBlock, /* unreachable */ true);
 
@@ -1625,8 +1642,8 @@ PhaseStatus Compiler::fgMergeFinallyChains()
     assert(!fgFuncletsCreated);
 #endif // FEATURE_EH_FUNCLETS
 
-    // Assume we don't need to update the bbPreds lists.
-    assert(!fgComputePredsDone);
+    // We need to update the bbPreds lists.
+    assert(fgComputePredsDone);
 
     if (compHndBBtabCount == 0)
     {