JIT: build basic block pred lists before morph

src/coreclr/src/jit/codegencommon.cpp

@@ -1375,6 +1375,12 @@ bool CodeGen::genCreateAddrMode(GenTree*  addr,
 
     if (op2->IsIntCnsFitsInI32() && (op2->gtType != TYP_REF) && FitsIn<INT32>(cns + op2->AsIntConCommon()->IconValue()))
     {
+        // Don't build address modes out of non-foldable constants
+        if (!op2->AsIntConCommon()->ImmedValCanBeFolded(compiler, addr->OperGet()))
+        {
+            return false;
+        }
+
         /* We're adding a constant */
 
         cns += op2->AsIntConCommon()->IconValue();

src/coreclr/src/jit/compiler.cpp

@@ -4622,23 +4622,11 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
     }
     EndPhase(PHASE_GS_COOKIE);
 
-    /* Compute bbNum, bbRefs and bbPreds */
-
-    JITDUMP("\nRenumbering the basic blocks for fgComputePred\n");
-    fgRenumberBlocks();
-
-    noway_assert(!fgComputePredsDone); // This is the first time full (not cheap) preds will be computed.
-    fgComputePreds();
-    EndPhase(PHASE_COMPUTE_PREDS);
-
     /* If we need to emit GC Poll calls, mark the blocks that need them now.  This is conservative and can
      * be optimized later. */
     fgMarkGCPollBlocks();
     EndPhase(PHASE_MARK_GC_POLL_BLOCKS);
 
-    /* From this point on the flowgraph information such as bbNum,
-     * bbRefs or bbPreds has to be kept updated */
-
     // Compute the block and edge weights
     fgComputeBlockAndEdgeWeights();
     EndPhase(PHASE_COMPUTE_EDGE_WEIGHTS);

src/coreclr/src/jit/compiler.hpp

@@ -2729,13 +2729,31 @@ inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block)
 */
 inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
 {
-    // If we're converting a BBJ_CALLFINALLY block to a BBJ_THROW block,
+    JITDUMP("Converting " FMT_BB " to BBJ_THROW\n", block->bbNum);
+
+    // Ordering of the following operations matters.
+    // First, note if we are looking at the first block of a call always pair.
+    const bool isCallAlwaysPair = block->isBBCallAlwaysPair();
+
+    // Scrub this block from the pred lists of any successors
+    fgRemoveBlockAsPred(block);
+
+    // Update jump kind after the scrub.
+    block->bbJumpKind = BBJ_THROW;
+
+    // Any block with a throw is rare
+    block->bbSetRunRarely();
+
+    // If we've converted a BBJ_CALLFINALLY block to a BBJ_THROW block,
     // then mark the subsequent BBJ_ALWAYS block as unreferenced.
-    if (block->isBBCallAlwaysPair())
+    //
+    // Must do this after we update bbJumpKind of block.
+    if (isCallAlwaysPair)
     {
         BasicBlock* leaveBlk = block->bbNext;
         noway_assert(leaveBlk->bbJumpKind == BBJ_ALWAYS);
 
+        // leaveBlk is now unreachable, so scrub the pred lists.
         leaveBlk->bbFlags &= ~BBF_DONT_REMOVE;
         leaveBlk->bbRefs  = 0;
         leaveBlk->bbPreds = nullptr;
@@ -2757,9 +2775,6 @@ inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block)
         }
 #endif // defined(FEATURE_EH_FUNCLETS) && defined(_TARGET_ARM_)
     }
-
-    block->bbJumpKind = BBJ_THROW;
-    block->bbSetRunRarely(); // any block with a throw is rare
 }
 
 /*****************************************************************************

src/coreclr/src/jit/flowgraph.cpp

@@ -431,9 +431,6 @@ BasicBlock* Compiler::fgNewBasicBlock(BBjumpKinds jumpKind)
 
 void Compiler::fgEnsureFirstBBisScratch()
 {
-    // This method does not update predecessor lists and so must only be called before they are computed.
-    assert(!fgComputePredsDone);
-
     // Have we already allocated a scratch block?
 
     if (fgFirstBBisScratch())
@@ -453,6 +450,15 @@ void Compiler::fgEnsureFirstBBisScratch()
             block->inheritWeight(fgFirstBB);
         }
 
+        // The first block has an implicit ref count which we must
+        // remove. Note the ref count could be greater that one, if
+        // the first block is not scratch and is targeted by a
+        // branch.
+        assert(fgFirstBB->bbRefs >= 1);
+        fgFirstBB->bbRefs--;
+
+        // The new scratch bb will fall through to the old first bb
+        fgAddRefPred(fgFirstBB, block);
         fgInsertBBbefore(fgFirstBB, block);
     }
     else
@@ -466,6 +472,9 @@ void Compiler::fgEnsureFirstBBisScratch()
 
     block->bbFlags |= (BBF_INTERNAL | BBF_IMPORTED);
 
+    // This new first BB has an implicit ref, and no others.
+    block->bbRefs = 1;
+
     fgFirstBBScratch = fgFirstBB;
 
 #ifdef DEBUG
@@ -8244,7 +8253,7 @@ void Compiler::fgConvertSyncReturnToLeave(BasicBlock* block)
     // Convert the BBJ_RETURN to BBJ_ALWAYS, jumping to genReturnBB.
     block->bbJumpKind = BBJ_ALWAYS;
     block->bbJumpDest = genReturnBB;
-    block->bbJumpDest->bbRefs++;
+    fgAddRefPred(genReturnBB, block);
 
 #ifdef DEBUG
     if (verbose)
@@ -8531,7 +8540,7 @@ class MergedReturns
         newReturnBB->bbRefs     = 1; // bbRefs gets update later, for now it should be 1
         comp->fgReturnCount++;
 
-        newReturnBB->bbFlags |= BBF_INTERNAL;
+        newReturnBB->bbFlags |= (BBF_INTERNAL | BBF_JMP_TARGET);
 
         noway_assert(newReturnBB->bbNext == nullptr);
 
@@ -13102,6 +13111,7 @@ void Compiler::fgComputeBlockAndEdgeWeights()
     const bool usingProfileWeights = fgIsUsingProfileWeights();
     const bool isOptimizing        = opts.OptimizationEnabled();
 
+    fgModified             = false;
     fgHaveValidEdgeWeights = false;
     fgCalledCount          = BB_UNITY_WEIGHT;
 

src/coreclr/src/jit/gentree.cpp

@@ -5483,6 +5483,18 @@ bool GenTree::OperMayThrow(Compiler* comp)
         case GT_ARR_ELEM:
             return comp->fgAddrCouldBeNull(this->AsArrElem()->gtArrObj);
 
+        case GT_FIELD:
+        {
+            GenTree* fldObj = this->AsField()->gtFldObj;
+
+            if (fldObj != nullptr)
+            {
+                return comp->fgAddrCouldBeNull(fldObj);
+            }
+
+            return false;
+        }
+
         case GT_ARR_BOUNDS_CHECK:
         case GT_ARR_INDEX:
         case GT_ARR_OFFSET:

src/coreclr/src/jit/morph.cpp

@@ -7232,6 +7232,11 @@ GenTree* Compiler::fgMorphPotentialTailCall(GenTreeCall* call)
     }
 #endif
 
+    // This block is not longer any block's predecessor. If we end up
+    // converting this tail call to a branch, we'll add appropriate
+    // successor information then.
+    fgRemoveBlockAsPred(compCurBB);
+
 #if !FEATURE_TAILCALL_OPT_SHARED_RETURN
     // We enable shared-ret tail call optimization for recursive calls even if
     // FEATURE_TAILCALL_OPT_SHARED_RETURN is not defined.
@@ -8033,6 +8038,7 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa
     fgFirstBB->bbFlags |= BBF_DONT_REMOVE;
     block->bbJumpKind = BBJ_ALWAYS;
     block->bbJumpDest = fgFirstBB->bbNext;
+    block->bbJumpDest->bbFlags |= BBF_JMP_TARGET;
     fgAddRefPred(block->bbJumpDest, block);
     block->bbFlags &= ~BBF_HAS_JMP;
 }
@@ -14877,12 +14883,6 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
             /* Unconditional throw - transform the basic block into a BBJ_THROW */
             fgConvertBBToThrowBB(block);
 
-            /* Remove 'block' from the predecessor list of 'block->bbNext' */
-            fgRemoveRefPred(block->bbNext, block);
-
-            /* Remove 'block' from the predecessor list of 'block->bbJumpDest' */
-            fgRemoveRefPred(block->bbJumpDest, block);
-
 #ifdef DEBUG
             if (verbose)
             {
@@ -15096,19 +15096,6 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
             /* Unconditional throw - transform the basic block into a BBJ_THROW */
             fgConvertBBToThrowBB(block);
 
-            /* update the flow graph */
-
-            unsigned     jumpCnt = block->bbJumpSwt->bbsCount;
-            BasicBlock** jumpTab = block->bbJumpSwt->bbsDstTab;
-
-            for (unsigned val = 0; val < jumpCnt; val++, jumpTab++)
-            {
-                BasicBlock* curJump = *jumpTab;
-
-                /* Remove 'block' from the predecessor list of 'curJump' */
-                fgRemoveRefPred(curJump, block);
-            }
-
 #ifdef DEBUG
             if (verbose)
             {
@@ -15207,22 +15194,30 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
     return result;
 }
 
-//*****************************************************************************
+//------------------------------------------------------------------------
+// fgMorphBlockStmt: morph a single statement in a block.
 //
-// Morphs a single statement in a block.
-// Can be called anytime, unlike fgMorphStmts() which should only be called once.
+// Arguments:
+//    block - block containing the statement
+//    stmt - statement to morph
+//    msg - string to identify caller in a dump
 //
-// Returns true  if 'stmt' was removed from the block.
-// Returns false if 'stmt' is still in the block (even if other statements were removed).
+// Returns:
+//    true if 'stmt' was removed from the block.
+//  s false if 'stmt' is still in the block (even if other statements were removed).
+//
+// Notes:
+//   Can be called anytime, unlike fgMorphStmts() which should only be called once.
 //
-
 bool Compiler::fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(const char* msg))
 {
     assert(block != nullptr);
     assert(stmt != nullptr);
 
-    compCurBB   = block;
-    compCurStmt = stmt;
+    // Reset some ambient state
+    fgRemoveRestOfBlock = false;
+    compCurBB           = block;
+    compCurStmt         = stmt;
 
     GenTree* morph = fgMorphTree(stmt->GetRootNode());
 
@@ -15315,10 +15310,7 @@ bool Compiler::fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(cons
         }
 
         // The rest of block has been removed and we will always throw an exception.
-
-        // Update succesors of block
-        fgRemoveBlockAsPred(block);
-
+        //
         // For compDbgCode, we prepend an empty BB as the firstBB, it is BBJ_NONE.
         // We should not convert it to a ThrowBB.
         if ((block != fgFirstBB) || ((fgFirstBB->bbFlags & BBF_INTERNAL) == 0))
@@ -15682,6 +15674,7 @@ void Compiler::fgMorphBlocks()
                     {
                         block->bbJumpKind = BBJ_ALWAYS;
                         block->bbJumpDest = genReturnBB;
+                        fgAddRefPred(genReturnBB, block);
                         fgReturnCount--;
                     }
                     if (genReturnLocal != BAD_VAR_NUM)
@@ -16642,6 +16635,25 @@ void Compiler::fgMorph()
 
     EndPhase(PHASE_CLONE_FINALLY);
 
+    /* Compute bbNum, bbRefs and bbPreds */
+
+    JITDUMP("\nRenumbering the basic blocks for fgComputePreds\n");
+    fgRenumberBlocks();
+
+    noway_assert(!fgComputePredsDone); // This is the first time full (not cheap) preds will be computed.
+    fgComputePreds();
+
+    // Run an early flow graph simplification pass
+    if (opts.OptimizationEnabled())
+    {
+        fgUpdateFlowGraph();
+    }
+
+    EndPhase(PHASE_COMPUTE_PREDS);
+
+    /* From this point on the flowgraph information such as bbNum,
+     * bbRefs or bbPreds has to be kept updated */
+
     /* For x64 and ARM64 we need to mark irregular parameters */
     lvaRefCountState = RCS_EARLY;
     fgResetImplicitByRefRefCount();

src/coreclr/src/jit/optimizer.cpp

@@ -4073,6 +4073,13 @@ void Compiler::fgOptWhileLoop(BasicBlock* block)
         return;
     }
 
+    // block can't be the scratch bb, since we prefer to keep flow
+    // out of the scratch bb as BBJ_ALWAYS or BBJ_NONE.
+    if (fgBBisScratch(block))
+    {
+        return;
+    }
+
     // It has to be a forward jump
     //  TODO-CQ: Check if we can also optimize the backwards jump as well.
     //