JIT: revise checking for computed preds (#81582)
authorAndy Ayers <andya@microsoft.com>
Fri, 3 Feb 2023 16:47:55 +0000 (08:47 -0800)
committerGitHub <noreply@github.com>
Fri, 3 Feb 2023 16:47:55 +0000 (08:47 -0800)
Since pred lists now exist pervasively, change most methods that had
conditional pred list updates to assert preds exist and always update.

To make sure I got all uses, I renamed `fgComputePredsDone` to
`fgPredsComputed`.

Remove the ability to drop EH, as it doesn't update pred lists properly
and so has been broken for quite a while now.

Remove some of the logic for fixing up finally targets, since we now
always have pred lists around and so don't need the blanket invaliation.

Closes #80193.

15 files changed:
src/coreclr/jit/block.h
src/coreclr/jit/compiler.cpp
src/coreclr/jit/compiler.h
src/coreclr/jit/compiler.hpp
src/coreclr/jit/fgbasic.cpp
src/coreclr/jit/fgdiagnostic.cpp
src/coreclr/jit/fgehopt.cpp
src/coreclr/jit/fgflow.cpp
src/coreclr/jit/fgopt.cpp
src/coreclr/jit/fgprofile.cpp
src/coreclr/jit/flowgraph.cpp
src/coreclr/jit/indirectcalltransformer.cpp
src/coreclr/jit/jiteh.cpp
src/coreclr/jit/morph.cpp
src/coreclr/jit/ssabuilder.cpp

index 1bc175b..1370033 100644 (file)
@@ -1052,7 +1052,7 @@ struct BasicBlock : private LIR::Range
     };
 
     // Basic block predecessor lists. Predecessor lists are created by fgLinkBasicBlocks(), stored
-    // in 'bbPreds', and then maintained throughout compilation. 'fgComputePredsDone' will be 'true' after the
+    // in 'bbPreds', and then maintained throughout compilation. 'fgPredsComputed' will be 'true' after the
     // predecessor lists are created.
     //
     flowList* bbPreds; // ptr to list of predecessors
index b60013a..20a6d97 100644 (file)
@@ -4654,15 +4654,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         lvaRefCountState       = RCS_INVALID;
         fgLocalVarLivenessDone = false;
 
-#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-        if (fgNeedToAddFinallyTargetBits)
-        {
-            // We previously wiped out the BBF_FINALLY_TARGET bits due to some morphing; add them back.
-            fgAddFinallyTargetFlags();
-            fgNeedToAddFinallyTargetBits = false;
-        }
-#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-
         // Decide the kind of code we want to generate
         fgSetOptions();
 
index 9c42682..80b9729 100644 (file)
@@ -2361,10 +2361,6 @@ public:
 
 #endif // FEATURE_EH_FUNCLETS
 
-#if !FEATURE_EH
-    void fgRemoveEH();
-#endif // !FEATURE_EH
-
     void fgSortEHTable();
 
     // Causes the EH table to obey some well-formedness conditions, by inserting
@@ -4512,7 +4508,7 @@ public:
 #endif // FEATURE_JIT_METHOD_PERF
 
     bool fgModified;             // True if the flow graph has been modified recently
-    bool fgComputePredsDone;     // Have we computed the bbPreds list
+    bool fgPredsComputed;        // Have we computed the bbPreds list
     bool fgDomsComputed;         // Have we computed the dominator sets?
     bool fgReturnBlocksComputed; // Have we computed the return blocks list?
     bool fgOptimizedFinally;     // Did we optimize any try-finallys?
@@ -4634,7 +4630,6 @@ public:
     void fgAddFinallyTargetFlags();
 
 #endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-
     PhaseStatus fgTailMergeThrows();
     void fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock,
                                             BasicBlock* nonCanonicalBlock,
@@ -4645,12 +4640,6 @@ public:
                                        BasicBlock* canonicalBlock,
                                        flowList*   predEdge);
 
-#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-    // Sometimes we need to defer updating the BBF_FINALLY_TARGET bit. fgNeedToAddFinallyTargetBits signals
-    // when this is necessary.
-    bool fgNeedToAddFinallyTargetBits;
-#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-
     bool fgRetargetBranchesToCanonicalCallFinally(BasicBlock*      block,
                                                   BasicBlock*      handler,
                                                   BlockToBlockMap& continuationMap);
index 41c1ae5..94dcf70 100644 (file)
@@ -2739,6 +2739,7 @@ inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
 inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
 {
     JITDUMP("Converting " FMT_BB " to BBJ_THROW\n", block->bbNum);
+    assert(fgPredsComputed);
 
     // Ordering of the following operations matters.
     // First, note if we are looking at the first block of a call always pair.
@@ -2768,20 +2769,7 @@ inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
         leaveBlk->bbPreds = nullptr;
 
 #if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
-        // This function (fgConvertBBToThrowBB) can be called before the predecessor lists are created (e.g., in
-        // fgMorph). The fgClearFinallyTargetBit() function to update the BBF_FINALLY_TARGET bit depends on these
-        // predecessor lists. If there are no predecessor lists, we immediately clear all BBF_FINALLY_TARGET bits
-        // (to allow subsequent dead code elimination to delete such blocks without asserts), and set a flag to
-        // recompute them later, before they are required.
-        if (fgComputePredsDone)
-        {
-            fgClearFinallyTargetBit(leaveBlk->bbJumpDest);
-        }
-        else
-        {
-            fgClearAllFinallyTargetBits();
-            fgNeedToAddFinallyTargetBits = true;
-        }
+        fgClearFinallyTargetBit(leaveBlk->bbJumpDest);
 #endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
     }
 }
index 54a9510..52f641c 100644 (file)
@@ -21,7 +21,7 @@ void Compiler::fgInit()
 #endif // DEBUG
 
     /* We haven't yet computed the bbPreds lists */
-    fgComputePredsDone = false;
+    fgPredsComputed = false;
 
     /* We haven't yet computed the edge weight */
     fgEdgeWeightsComputed    = false;
@@ -296,7 +296,7 @@ bool Compiler::fgEnsureFirstBBisScratch()
     // But if we call this early, before fgLinkBasicBlocks,
     // defer and let it handle adding the implicit ref.
     //
-    block->bbRefs = fgComputePredsDone ? 1 : 0;
+    block->bbRefs = fgPredsComputed ? 1 : 0;
 
     fgFirstBBScratch = fgFirstBB;
 
@@ -322,7 +322,7 @@ bool Compiler::fgFirstBBisScratch()
     {
         assert(fgFirstBBScratch == fgFirstBB);
         assert(fgFirstBBScratch->bbFlags & BBF_INTERNAL);
-        if (fgComputePredsDone)
+        if (fgPredsComputed)
         {
             assert(fgFirstBBScratch->countOfInEdges() == 1);
         }
@@ -396,6 +396,7 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw
     noway_assert(oldSwitchBlock != nullptr);
     noway_assert(newSwitchBlock != nullptr);
     noway_assert(oldSwitchBlock->bbJumpKind == BBJ_SWITCH);
+    assert(fgPredsComputed);
 
     // Walk the switch's jump table, updating the predecessor for each branch.
     for (BasicBlock* const bJump : oldSwitchBlock->SwitchTargets())
@@ -406,18 +407,12 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw
         // fgRemoveRefPred()/fgAddRefPred() will do the right thing: the second and
         // subsequent duplicates will simply subtract from and add to the duplicate
         // count (respectively).
-        if (bJump->countOfInEdges() > 0)
-        {
-            //
-            // Remove the old edge [oldSwitchBlock => bJump]
-            //
-            fgRemoveRefPred(bJump, oldSwitchBlock);
-        }
-        else
-        {
-            // bJump->countOfInEdges() must not be zero after preds are calculated.
-            assert(!fgComputePredsDone);
-        }
+
+        //
+        // Remove the old edge [oldSwitchBlock => bJump]
+        //
+        assert(bJump->countOfInEdges() > 0);
+        fgRemoveRefPred(bJump, oldSwitchBlock);
 
         //
         // Create the new edge [newSwitchBlock => bJump]
@@ -463,6 +458,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne
     noway_assert(newTarget != nullptr);
     noway_assert(oldTarget != nullptr);
     noway_assert(blockSwitch->bbJumpKind == BBJ_SWITCH);
+    assert(fgPredsComputed);
 
     // For the jump targets values that match oldTarget of our BBJ_SWITCH
     // replace predecessor 'blockSwitch' with 'newTarget'
@@ -480,10 +476,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne
         {
             // Remove the old edge [oldTarget from blockSwitch]
             //
-            if (fgComputePredsDone)
-            {
-                fgRemoveAllRefPreds(oldTarget, blockSwitch);
-            }
+            fgRemoveAllRefPreds(oldTarget, blockSwitch);
 
             //
             // Change the jumpTab entry to branch to the new location
@@ -493,12 +486,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne
             //
             // Create the new edge [newTarget from blockSwitch]
             //
-            flowList* newEdge = nullptr;
-
-            if (fgComputePredsDone)
-            {
-                newEdge = fgAddRefPred(newTarget, blockSwitch);
-            }
+            flowList* const newEdge = fgAddRefPred(newTarget, blockSwitch);
 
             // Now set the correct value of newEdge->flDupCount
             // and replace any other jumps in jumpTab[] that go to oldTarget.
@@ -517,10 +505,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne
                     //
                     // Increment the flDupCount
                     //
-                    if (fgComputePredsDone)
-                    {
-                        newEdge->flDupCount++;
-                    }
+                    newEdge->flDupCount++;
                 }
                 i++; // Check the next entry in jumpTab[]
             }
@@ -554,6 +539,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne
 void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, BasicBlock* oldTarget)
 {
     assert(block != nullptr);
+    assert(fgPredsComputed);
 
     switch (block->bbJumpKind)
     {
@@ -567,12 +553,8 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, Bas
             if (block->bbJumpDest == oldTarget)
             {
                 block->bbJumpDest = newTarget;
-
-                if (fgComputePredsDone)
-                {
-                    fgRemoveRefPred(oldTarget, block);
-                    fgAddRefPred(newTarget, block);
-                }
+                fgRemoveRefPred(oldTarget, block);
+                fgAddRefPred(newTarget, block);
             }
             break;
 
@@ -588,12 +570,8 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, Bas
                 {
                     jumpTab[i] = newTarget;
                     changed    = true;
-
-                    if (fgComputePredsDone)
-                    {
-                        fgRemoveRefPred(oldTarget, block);
-                        fgAddRefPred(newTarget, block);
-                    }
+                    fgRemoveRefPred(oldTarget, block);
+                    fgAddRefPred(newTarget, block);
                 }
             }
 
@@ -2840,7 +2818,7 @@ void Compiler::fgLinkBasicBlocks()
 
     // Pred lists now established.
     //
-    fgComputePredsDone = true;
+    fgPredsComputed = true;
 }
 
 //------------------------------------------------------------------------
@@ -4746,11 +4724,8 @@ BasicBlock* Compiler::fgSplitBlockAtBeginning(BasicBlock* curr)
 BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
 {
     assert(curr->KindIs(BBJ_COND, BBJ_SWITCH, BBJ_ALWAYS));
-
-    if (fgComputePredsDone)
-    {
-        assert(fgGetPredForBlock(succ, curr) != nullptr);
-    }
+    assert(fgPredsComputed);
+    assert(fgGetPredForBlock(succ, curr) != nullptr);
 
     BasicBlock* newBlock;
     if (succ == curr->bbNext)
@@ -5246,13 +5221,20 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
     }
 }
 
-/*****************************************************************************
- *
- *  Function called to connect to block that previously had a fall through
- */
-
+//------------------------------------------------------------------------
+// fgConnectFallThrough: fix flow from a block that previously had a fall through
+//
+// Arguments:
+//   bSrc - source of fall through (may be null?)
+//   bDst - target of fall through
+//
+// Returns:
+//   Newly inserted block after bSrc that jumps to bDst,
+//   or nullptr if bSrc already falls through to bDst
+//
 BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
 {
+    assert(fgPredsComputed);
     BasicBlock* jmpBlk = nullptr;
 
     /* If bSrc is non-NULL */
@@ -5269,14 +5251,8 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
                 case BBJ_NONE:
                     bSrc->bbJumpKind = BBJ_ALWAYS;
                     bSrc->bbJumpDest = bDst;
-#ifdef DEBUG
-                    if (verbose)
-                    {
-                        printf("Block " FMT_BB " ended with a BBJ_NONE, Changed to an unconditional jump to " FMT_BB
-                               "\n",
-                               bSrc->bbNum, bSrc->bbJumpDest->bbNum);
-                    }
-#endif
+                    JITDUMP("Block " FMT_BB " ended with a BBJ_NONE, Changed to an unconditional jump to " FMT_BB "\n",
+                            bSrc->bbNum, bSrc->bbJumpDest->bbNum);
                     break;
 
                 case BBJ_CALLFINALLY:
@@ -5284,11 +5260,8 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
 
                     // Add a new block after bSrc which jumps to 'bDst'
                     jmpBlk = fgNewBBafter(BBJ_ALWAYS, bSrc, true);
+                    fgAddRefPred(jmpBlk, bSrc, fgGetPredForBlock(bDst, bSrc));
 
-                    if (fgComputePredsDone)
-                    {
-                        fgAddRefPred(jmpBlk, bSrc, fgGetPredForBlock(bDst, bSrc));
-                    }
                     // Record the loop number in the new block
                     jmpBlk->bbNatLoopNum = bSrc->bbNatLoopNum;
 
@@ -5296,9 +5269,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
                     //
                     if (fgHaveValidEdgeWeights && fgHaveProfileWeights())
                     {
-                        noway_assert(fgComputePredsDone);
-
-                        flowList* newEdge = fgGetPredForBlock(jmpBlk, bSrc);
+                        flowList* const newEdge = fgGetPredForBlock(jmpBlk, bSrc);
 
                         jmpBlk->bbWeight = (newEdge->edgeWeightMin() + newEdge->edgeWeightMax()) / 2;
                         if (bSrc->bbWeight == BB_ZERO_WEIGHT)
@@ -5339,22 +5310,10 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
 
                     jmpBlk->bbJumpDest = bDst;
 
-                    if (fgComputePredsDone)
-                    {
-                        fgReplacePred(bDst, bSrc, jmpBlk);
-                    }
-                    else
-                    {
-                        jmpBlk->bbFlags |= BBF_IMPORTED;
-                    }
+                    fgReplacePred(bDst, bSrc, jmpBlk);
 
-#ifdef DEBUG
-                    if (verbose)
-                    {
-                        printf("Added an unconditional jump to " FMT_BB " after block " FMT_BB "\n",
-                               jmpBlk->bbJumpDest->bbNum, bSrc->bbNum);
-                    }
-#endif // DEBUG
+                    JITDUMP("Added an unconditional jump to " FMT_BB " after block " FMT_BB "\n",
+                            jmpBlk->bbJumpDest->bbNum, bSrc->bbNum);
                     break;
 
                 default:
@@ -5371,14 +5330,9 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
                 (bSrc->bbJumpDest == bSrc->bbNext))
             {
                 bSrc->bbJumpKind = BBJ_NONE;
-#ifdef DEBUG
-                if (verbose)
-                {
-                    printf("Changed an unconditional jump from " FMT_BB " to the next block " FMT_BB
-                           " into a BBJ_NONE block\n",
-                           bSrc->bbNum, bSrc->bbNext->bbNum);
-                }
-#endif // DEBUG
+                JITDUMP("Changed an unconditional jump from " FMT_BB " to the next block " FMT_BB
+                        " into a BBJ_NONE block\n",
+                        bSrc->bbNum, bSrc->bbNext->bbNum);
             }
         }
     }
@@ -5404,20 +5358,17 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
 //
 bool Compiler::fgRenumberBlocks()
 {
+    assert(fgPredsComputed);
+
     // If we renumber the blocks the dominator information will be out-of-date
     if (fgDomsComputed)
     {
         noway_assert(!"Can't call Compiler::fgRenumberBlocks() when fgDomsComputed==true");
     }
 
-#ifdef DEBUG
-    if (verbose)
-    {
-        printf("\n*************** Before renumbering the basic blocks\n");
-        fgDispBasicBlocks();
-        fgDispHandlerTab();
-    }
-#endif // DEBUG
+    JITDUMP("\n*************** Before renumbering the basic blocks\n");
+    JITDUMPEXEC(fgDispBasicBlocks());
+    JITDUMPEXEC(fgDispHandlerTab());
 
     bool     renumbered  = false;
     bool     newMaxBBNum = false;
@@ -5450,29 +5401,20 @@ bool Compiler::fgRenumberBlocks()
 
     // If we renumbered, then we may need to reorder some pred lists.
     //
-    if (renumbered && fgComputePredsDone)
+    if (renumbered)
     {
         for (BasicBlock* const block : Blocks())
         {
             block->ensurePredListOrder(this);
         }
+        JITDUMP("\n*************** After renumbering the basic blocks\n");
+        JITDUMPEXEC(fgDispBasicBlocks());
+        JITDUMPEXEC(fgDispHandlerTab());
     }
-
-#ifdef DEBUG
-    if (verbose)
+    else
     {
-        printf("\n*************** After renumbering the basic blocks\n");
-        if (renumbered)
-        {
-            fgDispBasicBlocks();
-            fgDispHandlerTab();
-        }
-        else
-        {
-            printf("=============== No blocks renumbered!\n");
-        }
+        JITDUMP("=============== No blocks renumbered!\n");
     }
-#endif // DEBUG
 
     // Now update the BlockSet epoch, which depends on the block numbers.
     // If any blocks have been renumbered then create a new BlockSet epoch.
index 1bce589..7fab034 100644 (file)
@@ -1043,7 +1043,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
         fprintf(fgxFile, ">");
     }
 
-    if (fgComputePredsDone)
+    if (fgPredsComputed)
     {
         unsigned    edgeNum = 1;
         BasicBlock* bTarget;
@@ -1177,7 +1177,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
                 }
             }
 
-            if (fgComputePredsDone)
+            if (fgPredsComputed)
             {
                 // Already emitted pred edges above.
                 //
@@ -2275,7 +2275,7 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock,
         maxBlockNumWidth, "----");
     printf("BBnum %*sBBid ref try hnd %s     weight  %*s%s  lp [IL range]     [jump]%*s    [EH region]         [flags]\n",
         padWidth, "",
-        (fgComputePredsDone     ? "preds      "
+        (fgPredsComputed    ? "preds      "
                                 : "           "),
         ((ibcColWidth > 0) ? ibcColWidth - 3 : 0), "",  // Subtract 3 for the width of "IBC", printed next.
         ((ibcColWidth > 0)      ? "IBC"
@@ -2495,7 +2495,7 @@ private:
 //   the number of incoming edges for the block.
 unsigned BBPredsChecker::CheckBBPreds(BasicBlock* block, unsigned curTraversalStamp)
 {
-    if (!comp->fgComputePredsDone)
+    if (!comp->fgPredsComputed)
     {
         assert(block->bbPreds == nullptr);
         return 0;
@@ -2876,7 +2876,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
 
         if (checkBBRefs)
         {
-            assert(fgComputePredsDone);
+            assert(fgPredsComputed);
         }
 
         BBPredsChecker checker(this);
index ff2a171..01a5c5b 100644 (file)
@@ -38,7 +38,7 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
 #endif // FEATURE_EH_FUNCLETS
 
     // We need to update the bbPreds lists.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     if (compHndBBtabCount == 0)
     {
@@ -293,7 +293,7 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
 #endif // FEATURE_EH_FUNCLETS
 
     // We need to update the bbPreds lists.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     bool enableRemoveEmptyTry = true;
 
@@ -630,7 +630,7 @@ PhaseStatus Compiler::fgCloneFinally()
 #endif // FEATURE_EH_FUNCLETS
 
     // We need to update the bbPreds lists.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     bool enableCloning = true;
 
@@ -1643,7 +1643,7 @@ PhaseStatus Compiler::fgMergeFinallyChains()
 #endif // FEATURE_EH_FUNCLETS
 
     // We need to update the bbPreds lists.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     if (compHndBBtabCount == 0)
     {
@@ -2007,7 +2007,7 @@ PhaseStatus Compiler::fgTailMergeThrows()
 
     // This transformation requires block pred lists to be built
     // so that flow can be safely updated.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     struct ThrowHelper
     {
index d745386..2f976e4 100644 (file)
@@ -101,16 +101,10 @@ flowList* Compiler::fgAddRefPred(BasicBlock* block,
 {
     assert(block != nullptr);
     assert(blockPred != nullptr);
+    assert(fgPredsComputed ^ initializingPreds);
 
     block->bbRefs++;
 
-    if (!fgComputePredsDone && !initializingPreds)
-    {
-        // Why is someone trying to update the preds list when the preds haven't been created?
-        // Ignore them! This can happen when fgMorph is called before the preds list is created.
-        return nullptr;
-    }
-
     // Keep the predecessor list in lowest to highest bbNum order. This allows us to discover the loops in
     // optFindNaturalLoops from innermost to outermost.
     //
@@ -262,19 +256,10 @@ flowList* Compiler::fgRemoveRefPred(BasicBlock* block, BasicBlock* blockPred)
 {
     noway_assert(block != nullptr);
     noway_assert(blockPred != nullptr);
-
     noway_assert(block->countOfInEdges() > 0);
+    assert(fgPredsComputed);
     block->bbRefs--;
 
-    // Do nothing if we haven't calculated the predecessor list yet.
-    // Yes, this does happen.
-    // For example the predecessor lists haven't been created yet when we do fgMorph.
-    // But fgMorph calls fgFoldConditional, which in turn calls fgRemoveRefPred.
-    if (!fgComputePredsDone)
-    {
-        return nullptr;
-    }
-
     flowList** ptrToPred;
     flowList*  pred = fgGetPredForBlock(block, blockPred, &ptrToPred);
     noway_assert(pred != nullptr);
@@ -318,7 +303,7 @@ flowList* Compiler::fgRemoveAllRefPreds(BasicBlock* block, BasicBlock* blockPred
 {
     assert(block != nullptr);
     assert(blockPred != nullptr);
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
     assert(block->countOfInEdges() > 0);
 
     flowList** ptrToPred;
index 77c4752..f55400d 100644 (file)
@@ -230,7 +230,7 @@ void Compiler::fgUpdateChangedFlowGraph(FlowGraphUpdates updates)
 //
 void Compiler::fgComputeReachabilitySets()
 {
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
 #ifdef DEBUG
     fgReachabilitySetsValid = false;
@@ -548,7 +548,7 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock)
 //
 PhaseStatus Compiler::fgComputeReachability()
 {
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
     fgComputeReturnBlocks();
 
index 5b8e294..bc05a27 100644 (file)
@@ -4450,7 +4450,7 @@ void Compiler::fgDebugCheckProfileWeights()
 {
     // Optionally check profile data, if we have any.
     //
-    const bool enabled = (JitConfig.JitProfileChecks() > 0) && fgHaveProfileWeights() && fgComputePredsDone;
+    const bool enabled = (JitConfig.JitProfileChecks() > 0) && fgHaveProfileWeights() && fgPredsComputed;
     if (!enabled)
     {
         return;
index f5e84d5..b2d01c7 100644 (file)
@@ -1558,7 +1558,7 @@ void Compiler::fgAddSyncMethodEnterExit()
     assert(!fgFuncletsCreated);
 
     // We need to update the bbPreds lists.
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
 
 #if !FEATURE_EH
     // If we don't support EH, we can't add the EH needed by synchronized methods.
@@ -3304,7 +3304,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block)
 //
 void Compiler::fgCreateFuncletPrologBlocks()
 {
-    noway_assert(fgComputePredsDone);
+    noway_assert(fgPredsComputed);
     noway_assert(!fgDomsComputed); // this function doesn't maintain the dom sets
     assert(!fgFuncletsCreated);
 
@@ -3475,6 +3475,7 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock()
     // Since we may need to create a new transition block
     // we assert that it is OK to create new blocks.
     //
+    assert(fgPredsComputed);
     assert(fgSafeBasicBlockCreation);
     assert(fgFirstColdBlock == nullptr);
 
@@ -3676,8 +3677,6 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock()
                         transitionBlock->bbJumpDest = firstColdBlock;
                         transitionBlock->inheritWeight(firstColdBlock);
 
-                        noway_assert(fgComputePredsDone);
-
                         // Update the predecessor list for firstColdBlock
                         fgReplacePred(firstColdBlock, prevToFirstColdBlock, transitionBlock);
 
index 3b1c907..1d21937 100644 (file)
@@ -279,7 +279,7 @@ private:
         //
         virtual void ChainFlow()
         {
-            assert(compiler->fgComputePredsDone);
+            assert(compiler->fgPredsComputed);
 
             // currBlock
             compiler->fgRemoveRefPred(remainderBlock, currBlock);
@@ -1377,7 +1377,7 @@ private:
         //
         virtual void ChainFlow() override
         {
-            assert(compiler->fgComputePredsDone);
+            assert(compiler->fgPredsComputed);
 
             // currBlock
             compiler->fgRemoveRefPred(remainderBlock, currBlock);
index 822a90b..450e3c8 100644 (file)
@@ -1609,189 +1609,6 @@ EHblkDsc* Compiler::fgAddEHTableEntry(unsigned XTnum)
 
 #endif // FEATURE_EH_FUNCLETS
 
-#if !FEATURE_EH
-
-/*****************************************************************************
- *  fgRemoveEH: To facilitate the bring-up of new platforms without having to
- *  worry about fully implementing EH, we want to simply remove EH constructs
- *  from the IR. This works because a large percentage of our tests contain
- *  EH constructs but don't actually throw exceptions. This function removes
- *  'catch', 'filter', 'filter-handler', and 'fault' clauses completely.
- *  It requires that the importer has created the EH table, and that normal
- *  EH well-formedness tests have been done, and 'leave' opcodes have been
- *  imported.
- *
- *  It currently does not handle 'finally' clauses, so tests that include
- *  'finally' will NYI(). To handle 'finally', we would need to inline the
- *  'finally' clause IL at each exit from a finally-protected 'try', or
- *  else call the 'finally' clause, like normal.
- *
- *  Walk the EH table from beginning to end. If a table entry is nested within
- *  a handler, we skip it, as we'll delete its code when we get to the enclosing
- *  handler. If a clause is enclosed within a 'try', or has no nesting, then we delete
- *  it (and its range of code blocks). We don't need to worry about cleaning up
- *  the EH table entries as we remove the individual handlers (such as calling
- *  fgRemoveEHTableEntry()), as we'll null out the entire table at the end.
- *
- *  This function assumes FEATURE_EH_FUNCLETS is defined.
- */
-void Compiler::fgRemoveEH()
-{
-#ifdef DEBUG
-    if (verbose)
-        printf("\n*************** In fgRemoveEH()\n");
-#endif // DEBUG
-
-    if (compHndBBtabCount == 0)
-    {
-        JITDUMP("No EH to remove\n\n");
-        return;
-    }
-
-#ifdef DEBUG
-    if (verbose)
-    {
-        printf("\n*************** Before fgRemoveEH()\n");
-        fgDispBasicBlocks();
-        fgDispHandlerTab();
-        printf("\n");
-    }
-#endif // DEBUG
-
-    // Make sure we're early in compilation, so we don't need to update lots of data structures.
-    assert(!fgComputePredsDone);
-    assert(!fgDomsComputed);
-    assert(!fgFuncletsCreated);
-    assert(fgFirstFuncletBB == nullptr); // this should follow from "!fgFuncletsCreated"
-    assert(!optLoopTableValid);
-
-    unsigned  XTnum;
-    EHblkDsc* HBtab;
-
-    for (XTnum = 0, HBtab = compHndBBtab; XTnum < compHndBBtabCount; XTnum++, HBtab++)
-    {
-        if (HBtab->ebdEnclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX)
-        {
-            // This entry is nested within some other handler. So, don't delete the
-            // EH entry here; let the enclosing handler delete it. Note that for this
-            // EH entry, both the 'try' and handler portions are fully nested within
-            // the enclosing handler region, due to proper nesting rules.
-            continue;
-        }
-
-        if (HBtab->HasCatchHandler() || HBtab->HasFilter() || HBtab->HasFaultHandler())
-        {
-            // Remove all the blocks associated with the handler. Note that there is no
-            // fall-through into the handler, or fall-through out of the handler, so
-            // just deleting the blocks is sufficient. Note, however, that for every
-            // BBJ_EHCATCHRET we delete, we need to fix up the reference count of the
-            // block it points to (by subtracting one from its reference count).
-            // Note that the blocks for a filter immediately precede the blocks for its associated filter-handler.
-
-            BasicBlock* blkBeg  = HBtab->HasFilter() ? HBtab->ebdFilter : HBtab->ebdHndBeg;
-            BasicBlock* blkLast = HBtab->ebdHndLast;
-
-            // Splice out the range of blocks from blkBeg to blkLast (inclusive).
-            fgUnlinkRange(blkBeg, blkLast);
-
-            BasicBlock* blk;
-
-            // Walk the unlinked blocks and marked them as having been removed.
-            for (blk = blkBeg; blk != blkLast->bbNext; blk = blk->bbNext)
-            {
-                blk->bbFlags |= BBF_REMOVED;
-
-                if (blk->bbJumpKind == BBJ_EHCATCHRET)
-                {
-                    assert(blk->bbJumpDest->bbRefs > 0);
-                    blk->bbJumpDest->bbRefs -= 1;
-                }
-            }
-
-            // Walk the blocks of the 'try' and clear data that makes them appear to be within a 'try'.
-            for (blk = HBtab->ebdTryBeg; blk != HBtab->ebdTryLast->bbNext; blk = blk->bbNext)
-            {
-                blk->clearTryIndex();
-                blk->bbFlags &= ~BBF_TRY_BEG;
-            }
-
-            // If we are deleting a range of blocks whose last block is
-            // the 'last' block of an enclosing try/hnd region, we need to
-            // fix up the EH table. We only care about less nested
-            // EH table entries, since we've already deleted everything up to XTnum.
-
-            unsigned  XTnum2;
-            EHblkDsc* HBtab2;
-            for (XTnum2 = XTnum + 1, HBtab2 = compHndBBtab + XTnum2; XTnum2 < compHndBBtabCount; XTnum2++, HBtab2++)
-            {
-                // Handle case where deleted range is at the end of a 'try'.
-                if (HBtab2->ebdTryLast == blkLast)
-                {
-                    fgSetTryEnd(HBtab2, blkBeg->bbPrev);
-                }
-                // Handle case where deleted range is at the end of a handler.
-                // (This shouldn't happen, though, because we don't delete handlers
-                // nested within other handlers; we wait until we get to the
-                // enclosing handler.)
-                if (HBtab2->ebdHndLast == blkLast)
-                {
-                    unreached();
-                }
-            }
-        }
-        else
-        {
-            // It must be a 'finally'. We still need to call the finally. Note that the
-            // 'finally' can be "called" from multiple locations (e.g., the 'try' block
-            // can have multiple 'leave' instructions, each leaving to different targets,
-            // and each going through the 'finally'). We could inline the 'finally' at each
-            // LEAVE site within a 'try'. If the 'try' exits at all (that is, no infinite loop),
-            // there will be at least one since there is no "fall through" at the end of
-            // the 'try'.
-
-            assert(HBtab->HasFinallyHandler());
-
-            NYI("remove finally blocks");
-        }
-    } /* end of the for loop over XTnum */
-
-#ifdef DEBUG
-    // Make sure none of the remaining blocks have any EH.
-
-    for (BasicBlock* const blk : Blocks())
-    {
-        assert(!blk->hasTryIndex());
-        assert(!blk->hasHndIndex());
-        assert((blk->bbFlags & BBF_TRY_BEG) == 0);
-        assert((blk->bbFlags & BBF_FUNCLET_BEG) == 0);
-        assert((blk->bbFlags & BBF_REMOVED) == 0);
-        assert(blk->bbCatchTyp == BBCT_NONE);
-    }
-#endif // DEBUG
-
-    // Delete the EH table
-
-    compHndBBtab      = nullptr;
-    compHndBBtabCount = 0;
-    // Leave compHndBBtabAllocCount alone.
-
-    // Renumber the basic blocks
-    JITDUMP("\nRenumbering the basic blocks for fgRemoveEH\n");
-    fgRenumberBlocks();
-
-#ifdef DEBUG
-    if (verbose)
-    {
-        printf("\n*************** After fgRemoveEH()\n");
-        fgDispBasicBlocks();
-        fgDispHandlerTab();
-        printf("\n");
-    }
-#endif
-}
-
-#endif // !FEATURE_EH
-
 /*****************************************************************************
  *
  *  Sort the EH table if necessary.
@@ -4169,7 +3986,7 @@ void Compiler::verCheckNestingLevel(EHNodeDsc* root)
 
 void Compiler::fgClearFinallyTargetBit(BasicBlock* block)
 {
-    assert(fgComputePredsDone);
+    assert(fgPredsComputed);
     assert((block->bbFlags & BBF_FINALLY_TARGET) != 0);
 
     for (BasicBlock* const predBlock : block->PredBlocks())
index f3db2e8..efe8a77 100644 (file)
@@ -27,13 +27,6 @@ PhaseStatus Compiler::fgMorphInit()
 {
     bool madeChanges = false;
 
-#if !FEATURE_EH
-    // If we aren't yet supporting EH in a compiler bring-up, remove as many EH handlers as possible, so
-    // we can pass tests that contain try/catch EH, but don't actually throw any exceptions.
-    fgRemoveEH();
-    madeChanges = true;
-#endif // !FEATURE_EH
-
     // We could allow ESP frames. Just need to reserve space for
     // pushing EBP if the method becomes an EBP-frame after an edit.
     // Note that requiring a EBP Frame disallows double alignment.  Thus if we change this
index a6490d1..9edb34f 100644 (file)
@@ -1599,6 +1599,8 @@ void SsaBuilder::Build()
 
 void SsaBuilder::SetupBBRoot()
 {
+    assert(m_pCompiler->fgPredsComputed);
+
     // Allocate a bbroot, if necessary.
     // We need a unique block to be the root of the dominator tree.
     // This can be violated if the first block is in a try, or if it is the first block of
@@ -1635,10 +1637,7 @@ void SsaBuilder::SetupBBRoot()
     m_pCompiler->fgInsertBBbefore(m_pCompiler->fgFirstBB, bbRoot);
 
     assert(m_pCompiler->fgFirstBB == bbRoot);
-    if (m_pCompiler->fgComputePredsDone)
-    {
-        m_pCompiler->fgAddRefPred(oldFirst, bbRoot);
-    }
+    m_pCompiler->fgAddRefPred(oldFirst, bbRoot);
 }
 
 #ifdef DEBUG