Remove BBF_TRY_BEG

The flag duplicates information already present in the EH table.

Also delete some remnants of verification from "impVerifyEHBlock".

src/coreclr/jit/block.cpp

@@ -399,10 +399,6 @@ void BasicBlock::dspFlags()
     {
         printf("failV ");
     }
-    if (bbFlags & BBF_TRY_BEG)
-    {
-        printf("try ");
-    }
     if (bbFlags & BBF_RUN_RARELY)
     {
         printf("rare ");

src/coreclr/jit/block.h

@@ -371,7 +371,7 @@ enum BasicBlockFlags : unsigned __int64
     BBF_IMPORTED             = MAKE_BBFLAG( 4), // BB byte-code has been imported
     BBF_INTERNAL             = MAKE_BBFLAG( 5), // BB has been added by the compiler
     BBF_FAILED_VERIFICATION  = MAKE_BBFLAG( 6), // BB has verification exception
-    BBF_TRY_BEG              = MAKE_BBFLAG( 7), // BB starts a 'try' block
+//  BBF_UNUSED               = MAKE_BBFLAG( 7),
     BBF_FUNCLET_BEG          = MAKE_BBFLAG( 8), // BB is the beginning of a funclet
     BBF_CLONED_FINALLY_BEGIN = MAKE_BBFLAG( 9), // First block of a cloned finally region
     BBF_CLONED_FINALLY_END   = MAKE_BBFLAG(10), // Last block of a cloned finally region

src/coreclr/jit/compiler.h

@@ -4259,7 +4259,7 @@ class Compiler
 
     void impReimportMarkBlock(BasicBlock* block);
 
-    void impVerifyEHBlock(BasicBlock* block, bool isTryStart);
+    void impVerifyEHBlock(BasicBlock* block);
 
     void impImportBlockPending(BasicBlock* block);
 

src/coreclr/jit/fgbasic.cpp

@@ -3726,10 +3726,6 @@ void Compiler::fgFindBasicBlocks()
             }
         }
 
-        /* Mark the initial block and last blocks in the 'try' region */
-
-        tryBegBB->bbFlags |= BBF_TRY_BEG;
-
         /*  Prevent future optimizations of removing the first block   */
         /*  of a TRY block and the first block of an exception handler */
 
@@ -4599,9 +4595,8 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr)
     newBlock->bbFlags = curr->bbFlags;
 
     // Remove flags that the new block can't have.
-    newBlock->bbFlags &=
-        ~(BBF_TRY_BEG | BBF_LOOP_HEAD | BBF_LOOP_CALL0 | BBF_LOOP_CALL1 | BBF_FUNCLET_BEG | BBF_LOOP_PREHEADER |
-          BBF_KEEP_BBJ_ALWAYS | BBF_PATCHPOINT | BBF_BACKWARD_JUMP_TARGET | BBF_LOOP_ALIGN);
+    newBlock->bbFlags &= ~(BBF_LOOP_HEAD | BBF_LOOP_CALL0 | BBF_LOOP_CALL1 | BBF_FUNCLET_BEG | BBF_LOOP_PREHEADER |
+                           BBF_KEEP_BBJ_ALWAYS | BBF_PATCHPOINT | BBF_BACKWARD_JUMP_TARGET | BBF_LOOP_ALIGN);
 
     // Remove the GC safe bit on the new block. It seems clear that if we split 'curr' at the end,
     // such that all the code is left in 'curr', and 'newBlock' just gets the control flow, then

src/coreclr/jit/fgdiagnostic.cpp

@@ -907,13 +907,15 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
             if (displayBlockFlags)
             {
                 // Don't display the `[` `]` unless we're going to display something.
-                const BasicBlockFlags allDisplayedBlockFlags = BBF_TRY_BEG | BBF_FUNCLET_BEG | BBF_RUN_RARELY |
-                                                               BBF_LOOP_HEAD | BBF_LOOP_PREHEADER | BBF_LOOP_ALIGN;
-                if (block->bbFlags & allDisplayedBlockFlags)
+                const bool            isTryEntryBlock = bbIsTryBeg(block);
+                const BasicBlockFlags allDisplayedBlockFlags =
+                    BBF_FUNCLET_BEG | BBF_RUN_RARELY | BBF_LOOP_HEAD | BBF_LOOP_PREHEADER | BBF_LOOP_ALIGN;
+
+                if (isTryEntryBlock || ((block->bbFlags & allDisplayedBlockFlags) != 0))
                 {
                     // Display a very few, useful, block flags
                     fprintf(fgxFile, " [");
-                    if (block->bbFlags & BBF_TRY_BEG)
+                    if (isTryEntryBlock)
                     {
                         fprintf(fgxFile, "T");
                     }
@@ -2171,7 +2173,7 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 *
         /* brace matching editor workaround to compensate for the preceding line: } */
     }
 
-    if (flags & BBF_TRY_BEG)
+    if (bbIsTryBeg(block))
     {
         // Output a brace for every try region that this block opens
 

src/coreclr/jit/fgehopt.cpp

@@ -224,12 +224,6 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
                 }
             }
 
-            if (block == firstTryBlock)
-            {
-                assert((block->bbFlags & BBF_TRY_BEG) != 0);
-                block->bbFlags &= ~BBF_TRY_BEG;
-            }
-
             if (block == lastTryBlock)
             {
                 break;
@@ -500,12 +494,6 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
                 }
             }
 
-            if (block == firstTryBlock)
-            {
-                assert((block->bbFlags & BBF_TRY_BEG) != 0);
-                block->bbFlags &= ~BBF_TRY_BEG;
-            }
-
             if (block == lastTryBlock)
             {
                 break;
@@ -2101,7 +2089,7 @@ PhaseStatus Compiler::fgTailMergeThrows()
         // Workaround: don't consider try entry blocks as candidates
         // for merging; if the canonical throw is later in the same try,
         // we'll create invalid flow.
-        if ((block->bbFlags & BBF_TRY_BEG) != 0)
+        if (bbIsTryBeg(block))
         {
             continue;
         }

src/coreclr/jit/fgopt.cpp

@@ -1671,7 +1671,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
                     fgSetTryBeg(HBtab, newTryEntry);
 
                     // Try entry blocks get specially marked and have special protection.
-                    HBtab->ebdTryBeg->bbFlags |= BBF_DONT_REMOVE | BBF_TRY_BEG;
+                    HBtab->ebdTryBeg->bbFlags |= BBF_DONT_REMOVE;
 
                     // We are keeping this try region
                     removeTryRegion = false;
@@ -2040,8 +2040,8 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
 
     // Make sure the second block is not the start of a TRY block or an exception handler
 
+    noway_assert(!bbIsTryBeg(bNext));
     noway_assert(bNext->bbCatchTyp == BBCT_NONE);
-    noway_assert((bNext->bbFlags & BBF_TRY_BEG) == 0);
     noway_assert((bNext->bbFlags & BBF_DONT_REMOVE) == 0);
 
     /* both or none must have an exception handler */
@@ -6400,10 +6400,9 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase)
                 goto REPEAT;
             }
 
-            /* Remove unreachable or empty blocks - do not consider blocks marked BBF_DONT_REMOVE or genReturnBB block
-             * These include first and last block of a TRY, exception handlers and RANGE_CHECK_FAIL THROW blocks */
-
-            if ((block->bbFlags & BBF_DONT_REMOVE) == BBF_DONT_REMOVE || block == genReturnBB)
+            // Remove unreachable or empty blocks - do not consider blocks marked BBF_DONT_REMOVE
+            // These include first and last block of a TRY, exception handlers and THROW blocks.
+            if ((block->bbFlags & BBF_DONT_REMOVE) != 0)
             {
                 bPrev = block;
                 continue;
@@ -6421,8 +6420,8 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase)
             }
 #endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
 
-            noway_assert(!block->bbCatchTyp);
-            noway_assert(!(block->bbFlags & BBF_TRY_BEG));
+            assert(!bbIsTryBeg(block));
+            noway_assert(block->bbCatchTyp == BBCT_NONE);
 
             /* Remove unreachable blocks
              *

src/coreclr/jit/flowgraph.cpp

@@ -1617,7 +1617,7 @@ void Compiler::fgAddSyncMethodEnterExit()
         // EH regions in fgFindBasicBlocks(). Note that the try has no enclosing
         // handler, and the fault has no enclosing try.
 
-        tryBegBB->bbFlags |= BBF_DONT_REMOVE | BBF_TRY_BEG | BBF_IMPORTED;
+        tryBegBB->bbFlags |= BBF_DONT_REMOVE | BBF_IMPORTED;
 
         faultBB->bbFlags |= BBF_DONT_REMOVE | BBF_IMPORTED;
         faultBB->bbCatchTyp = BBCT_FAULT;

src/coreclr/jit/importer.cpp

@@ -11043,30 +11043,17 @@ inline void Compiler::impReimportMarkBlock(BasicBlock* block)
     block->bbFlags &= ~BBF_IMPORTED;
 }
 
-/*****************************************************************************
- *
- *  Filter wrapper to handle only passed in exception code
- *  from it).
- */
-
-void Compiler::impVerifyEHBlock(BasicBlock* block, bool isTryStart)
+void Compiler::impVerifyEHBlock(BasicBlock* block)
 {
     assert(block->hasTryIndex());
     assert(!compIsForInlining());
 
     unsigned  tryIndex = block->getTryIndex();
     EHblkDsc* HBtab    = ehGetDsc(tryIndex);
 
-    if (isTryStart)
+    if (bbIsTryBeg(block) && (block->bbStkDepth != 0))
     {
-        assert(block->bbFlags & BBF_TRY_BEG);
-
-        // The Stack must be empty
-        //
-        if (block->bbStkDepth != 0)
-        {
-            BADCODE("Evaluation stack must be empty on entry into a try block");
-        }
+        BADCODE("Evaluation stack must be empty on entry into a try block");
     }
 
     // Save the stack contents, we'll need to restore it later
@@ -11121,12 +11108,6 @@ void Compiler::impVerifyEHBlock(BasicBlock* block, bool isTryStart)
         // Process the filter block, if we haven't already done so.
         if (HBtab->HasFilter())
         {
-            /* @VERIFICATION : Ideally the end of filter state should get
-               propagated to the catch handler, this is an incompleteness,
-               but is not a security/compliance issue, since the only
-               interesting state is the 'thisInit' state.
-            */
-
             BasicBlock* filterBB = HBtab->ebdFilter;
 
             if (((filterBB->bbFlags & BBF_IMPORTED) == 0) && (impGetPendingBlockMember(filterBB) == 0))
@@ -11203,51 +11184,14 @@ void Compiler::impImportBlock(BasicBlock* block)
     /* Set the current stack state to the merged result */
     verResetCurrentState(block, &verCurrentState);
 
-    /* Now walk the code and import the IL into GenTrees */
-
+    if (block->hasTryIndex())
     {
-        /* @VERIFICATION : For now, the only state propagation from try
-           to it's handler is "thisInit" state (stack is empty at start of try).
-           In general, for state that we track in verification, we need to
-           model the possibility that an exception might happen at any IL
-           instruction, so we really need to merge all states that obtain
-           between IL instructions in a try block into the start states of
-           all handlers.
-
-           However we do not allow the 'this' pointer to be uninitialized when
-           entering most kinds try regions (only try/fault are allowed to have
-           an uninitialized this pointer on entry to the try)
-
-           Fortunately, the stack is thrown away when an exception
-           leads to a handler, so we don't have to worry about that.
-           We DO, however, have to worry about the "thisInit" state.
-           But only for the try/fault case.
-
-           The only allowed transition is from TIS_Uninit to TIS_Init.
-
-           So for a try/fault region for the fault handler block
-           we will merge the start state of the try begin
-           and the post-state of each block that is part of this try region
-        */
-
-        // merge the start state of the try begin
-        //
-        if (block->bbFlags & BBF_TRY_BEG)
-        {
-            impVerifyEHBlock(block, true);
-        }
-
-        impImportBlockCode(block);
-
-        // As discussed above:
-        // merge the post-state of each block that is part of this try region
-        //
-        if (block->hasTryIndex())
-        {
-            impVerifyEHBlock(block, false);
-        }
+        impVerifyEHBlock(block);
     }
 
+    // Now walk the code and import the IL into GenTrees.
+    impImportBlockCode(block);
+
     if (compDonotInline())
     {
         return;

src/coreclr/jit/jiteh.cpp

@@ -2214,7 +2214,7 @@ bool Compiler::fgNormalizeEHCase2()
 
                         // Note that we don't need to clear any flags on the old try start, since it is still a 'try'
                         // start.
-                        newTryStart->bbFlags |= (BBF_TRY_BEG | BBF_DONT_REMOVE | BBF_INTERNAL);
+                        newTryStart->bbFlags |= (BBF_DONT_REMOVE | BBF_INTERNAL);
 
                         if (insertBeforeBlk->bbFlags & BBF_BACKWARD_JUMP_TARGET)
                         {
@@ -3007,7 +3007,6 @@ void Compiler::fgVerifyHandlerTab()
         assert(HBtab->ebdHndBeg != nullptr);
         assert(HBtab->ebdHndLast != nullptr);
 
-        assert(HBtab->ebdTryBeg->bbFlags & BBF_TRY_BEG);
         assert(HBtab->ebdTryBeg->bbFlags & BBF_DONT_REMOVE);
 
         assert(HBtab->ebdHndBeg->bbFlags & BBF_DONT_REMOVE);
@@ -3499,12 +3498,6 @@ void Compiler::fgVerifyHandlerTab()
 #endif // FEATURE_EH_FUNCLETS
         }
 
-        // Only the first block of 'try' regions should have BBF_TRY_BEG set.
-        if (!blockTryBegSet[block->bbNum])
-        {
-            assert((block->bbFlags & BBF_TRY_BEG) == 0);
-        }
-
         // Check for legal block types
         switch (block->GetJumpKind())
         {
@@ -4339,12 +4332,18 @@ bool Compiler::fgRelocateEHRegions()
 
 #endif // !FEATURE_EH_FUNCLETS
 
-/*****************************************************************************
- * We've inserted a new block before 'block' that should be part of the same EH region as 'block'.
- * Update the EH table to make this so. Also, set the new block to have the right EH region data
- * (copy the bbTryIndex, bbHndIndex, and bbCatchTyp from 'block' to the new predecessor, and clear
- * 'bbCatchTyp' from 'block').
- */
+//------------------------------------------------------------------------
+// fgExtendEHRegionBefore: Modify the EH table to account for a new block.
+//
+// We've inserted a new block before 'block' that should be part of the same
+// EH region as 'block'. Update the EH table to make this so. Also, set the
+// new block to have the right EH region data (copy the bbTryIndex, bbHndIndex,
+// and bbCatchTyp from 'block' to the new predecessor, and clear 'bbCatchTyp'
+// from 'block').
+//
+// Arguments:
+//    block - The block before which a new block has been inserted
+//
 void Compiler::fgExtendEHRegionBefore(BasicBlock* block)
 {
     assert(!block->IsFirst());
@@ -4369,13 +4368,7 @@ void Compiler::fgExtendEHRegionBefore(BasicBlock* block)
             }
 #endif // DEBUG
             HBtab->ebdTryBeg = bPrev;
-            bPrev->bbFlags |= BBF_TRY_BEG | BBF_DONT_REMOVE;
-
-            // clear the TryBeg flag unless it begins another try region
-            if (!bbIsTryBeg(block))
-            {
-                block->bbFlags &= ~BBF_TRY_BEG;
-            }
+            bPrev->bbFlags |= BBF_DONT_REMOVE;
         }
 
         if (HBtab->ebdHndBeg == block)
@@ -4387,12 +4380,13 @@ void Compiler::fgExtendEHRegionBefore(BasicBlock* block)
             }
 #endif // DEBUG
 
+            HBtab->ebdHndBeg = bPrev;
+            bPrev->bbFlags |= BBF_DONT_REMOVE;
+
             // The first block of a handler has an artificial extra refcount. Transfer that to the new block.
             noway_assert(block->countOfInEdges() > 0);
             block->bbRefs--;
-
-            HBtab->ebdHndBeg = bPrev;
-            bPrev->bbFlags |= BBF_DONT_REMOVE;
+            bPrev->bbRefs++;
 
 #if defined(FEATURE_EH_FUNCLETS)
             if (fgFuncletsCreated)
@@ -4403,8 +4397,6 @@ void Compiler::fgExtendEHRegionBefore(BasicBlock* block)
             }
 #endif // FEATURE_EH_FUNCLETS
 
-            bPrev->bbRefs++;
-
             // If this is a handler for a filter, the last block of the filter will end with
             // a BBJ_EHFILTERRET block that has a bbJumpDest that jumps to the first block of
             // its handler. So we need to update it to keep things in sync.

src/coreclr/jit/optimizer.cpp

@@ -7989,11 +7989,11 @@ void Compiler::fgSetEHRegionForNewLoopHead(BasicBlock* newHead, BasicBlock* top)
     assert(newHead->NextIs(top));
     assert(!fgIsFirstBlockOfFilterOrHandler(top));
 
-    if ((top->bbFlags & BBF_TRY_BEG) != 0)
+    if (bbIsTryBeg(top))
     {
         // `top` is the beginning of a try block. Figure out the EH region to use.
         assert(top->hasTryIndex());
-        unsigned short newTryIndex = (unsigned short)ehTrueEnclosingTryIndexIL(top->getTryIndex());
+        unsigned newTryIndex = ehTrueEnclosingTryIndexIL(top->getTryIndex());
         if (newTryIndex == EHblkDsc::NO_ENCLOSING_INDEX)
         {
             // No EH try index.
@@ -8107,7 +8107,7 @@ bool Compiler::fgCreateLoopPreHeader(unsigned lnum)
             // Does this existing region have the same EH region index that we will use when we create the pre-header?
             // If not, we want to create a new pre-header with the expected region.
             bool headHasCorrectEHRegion = false;
-            if ((top->bbFlags & BBF_TRY_BEG) != 0)
+            if (bbIsTryBeg(top))
             {
                 assert(top->hasTryIndex());
                 unsigned newTryIndex     = ehTrueEnclosingTryIndexIL(top->getTryIndex());