Separate statements from GenTree, part 1 (dotnet/coreclr#25898)

* Fix some leftovers where we used GenTree instead of GenTreeStmt.

* Separate GT_IL_OFFSET and GT_STMT.

* Add unreached to gtHashValue for GT_STMT.

it has an assert that tree is not GT_STMT: https://github.com/dotnet/coreclr/blob/dotnet/coreclr@65a50887b9a460d7833f0ab476fab9027f1e8166/src/jit/gentree.cpp#L1837 the assert was added before 2010.

* Do not use gtCloneExpr for stms.

* Fix optUnrollLoops.

The was a wrong change in cbf3704a8421ef0b093d68a93d867c9a768dc9d.


Commit migrated from https://github.com/dotnet/coreclr/commit/86fd1ea08c3d05855992df64fbb09929cb47e068

src/coreclr/src/jit/block.cpp

@@ -700,9 +700,9 @@ GenTreeStmt* BasicBlock::lastStmt() const
         return nullptr;
     }
 
-    GenTree* result = bbTreeList->gtPrev;
-    assert(result && result->gtNext == nullptr);
-    return result->AsStmt();
+    GenTreeStmt* result = bbTreeList->AsStmt()->gtPrevStmt;
+    assert(result != nullptr && result->gtNext == nullptr);
+    return result;
 }
 
 //------------------------------------------------------------------------

src/coreclr/src/jit/codegenlinear.cpp

@@ -414,7 +414,7 @@ void CodeGen::genCodeForBBlist()
             // Do we have a new IL offset?
             if (node->OperGet() == GT_IL_OFFSET)
             {
-                GenTreeStmt* ilOffset = node->AsStmt();
+                GenTreeILOffset* ilOffset = node->AsILOffset();
                 genEnsureCodeEmitted(currentILOffset);
                 currentILOffset = ilOffset->gtStmtILoffsx;
                 genIPmappingAdd(currentILOffset, firstMapping);

src/coreclr/src/jit/compiler.cpp

@@ -9941,13 +9941,13 @@ int cLeafIR(Compiler* comp, GenTree* tree)
 
         case GT_IL_OFFSET:
 
-            if (tree->gtStmt.gtStmtILoffsx == BAD_IL_OFFSET)
+            if (tree->gtILOffset.gtStmtILoffsx == BAD_IL_OFFSET)
             {
                 chars += printf("?");
             }
             else
             {
-                chars += printf("0x%x", jitGetILoffs(tree->gtStmt.gtStmtILoffsx));
+                chars += printf("0x%x", jitGetILoffs(tree->gtILOffset.gtStmtILoffsx));
             }
             break;
 

src/coreclr/src/jit/compiler.h

@@ -2625,6 +2625,12 @@ public:
         return gtCloneExpr(tree, addFlags, varNum, varVal, varNum, varVal);
     }
 
+    GenTreeStmt* gtCloneStmt(GenTreeStmt* stmt)
+    {
+        GenTree* exprClone = gtCloneExpr(stmt->gtStmtExpr);
+        return gtNewStmt(exprClone, stmt->gtStmtILoffsx);
+    }
+
     // Internal helper for cloning a call
     GenTreeCall* gtCloneExprCallHelper(GenTreeCall* call,
                                        unsigned     addFlags   = 0,

src/coreclr/src/jit/earlyprop.cpp

@@ -607,8 +607,8 @@ void Compiler::optFoldNullCheck(GenTree* tree)
                                                 // Then walk the statement list in reverse execution order
                                                 // until we get to the statement containing the null check.
                                                 // We only need to check the side effects at the root of each statement.
-                                                GenTree* curStmt = compCurStmt->gtPrev;
-                                                currentTree      = curStmt->gtStmt.gtStmtExpr;
+                                                GenTreeStmt* curStmt = compCurStmt->getPrevStmt();
+                                                currentTree          = curStmt->gtStmtExpr;
                                                 while (canRemoveNullCheck && (currentTree != defParent))
                                                 {
                                                     if ((nodesWalked++ > maxNodesWalked) ||
@@ -618,9 +618,9 @@ void Compiler::optFoldNullCheck(GenTree* tree)
                                                     }
                                                     else
                                                     {
-                                                        curStmt = curStmt->gtStmt.gtPrevStmt;
+                                                        curStmt = curStmt->getPrevStmt();
                                                         assert(curStmt != nullptr);
-                                                        currentTree = curStmt->gtStmt.gtStmtExpr;
+                                                        currentTree = curStmt->gtStmtExpr;
                                                     }
                                                 }
 
@@ -638,8 +638,7 @@ void Compiler::optFoldNullCheck(GenTree* tree)
                                                         additionNode->gtFlags & (GTF_EXCEPT | GTF_DONT_CSE);
 
                                                     // Re-morph the statement.
-                                                    fgMorphBlockStmt(compCurBB,
-                                                                     curStmt->AsStmt() DEBUGARG("optFoldNullCheck"));
+                                                    fgMorphBlockStmt(compCurBB, curStmt DEBUGARG("optFoldNullCheck"));
                                                 }
                                             }
                                         }

src/coreclr/src/jit/flowgraph.cpp

@@ -791,14 +791,14 @@ GenTreeStmt* Compiler::fgInsertStmtBefore(BasicBlock* block, GenTreeStmt* insert
     if (insertionPoint == block->bbTreeList)
     {
         // We're inserting before the first statement in the block.
-        GenTree* list = block->bbTreeList;
-        GenTree* last = list->gtPrev;
+        GenTreeStmt* first = block->firstStmt();
+        GenTreeStmt* last  = block->lastStmt();
 
-        stmt->gtNext = list;
+        stmt->gtNext = first;
         stmt->gtPrev = last;
 
         block->bbTreeList = stmt;
-        list->gtPrev      = stmt;
+        first->gtPrev     = stmt;
     }
     else
     {
@@ -9426,7 +9426,7 @@ BasicBlock* Compiler::fgSplitBlockAfterStatement(BasicBlock* curr, GenTreeStmt*
     if (stmt != nullptr)
     {
         newBlock->bbTreeList = stmt->gtNext;
-        if (newBlock->bbTreeList)
+        if (newBlock->bbTreeList != nullptr)
         {
             newBlock->bbTreeList->gtPrev = curr->bbTreeList->gtPrev;
         }
@@ -9492,10 +9492,10 @@ BasicBlock* Compiler::fgSplitBlockAfterNode(BasicBlock* curr, GenTree* node)
         {
             if ((*riter)->gtOper == GT_IL_OFFSET)
             {
-                GenTreeStmt* stmt = (*riter)->AsStmt();
-                if (stmt->gtStmtILoffsx != BAD_IL_OFFSET)
+                GenTreeILOffset* ilOffset = (*riter)->AsILOffset();
+                if (ilOffset->gtStmtILoffsx != BAD_IL_OFFSET)
                 {
-                    splitPointILOffset = jitGetILoffs(stmt->gtStmtILoffsx);
+                    splitPointILOffset = jitGetILoffs(ilOffset->gtStmtILoffsx);
                     break;
                 }
             }
@@ -9998,8 +9998,6 @@ void Compiler::fgRemoveStmt(BasicBlock* block, GenTreeStmt* stmt)
 {
     assert(fgOrder == FGOrderTree);
 
-    GenTreeStmt* tree = block->firstStmt();
-
 #ifdef DEBUG
     if (verbose &&
         stmt->gtStmtExpr->gtOper != GT_NOP) // Don't print if it is a GT_NOP. Too much noise from the inliner.
@@ -10029,8 +10027,8 @@ void Compiler::fgRemoveStmt(BasicBlock* block, GenTreeStmt* stmt)
         }
         else
         {
-            block->bbTreeList         = tree->gtNext;
-            block->bbTreeList->gtPrev = tree->gtPrev;
+            block->bbTreeList         = firstStmt->gtNext;
+            block->bbTreeList->gtPrev = firstStmt->gtPrev;
         }
     }
     else if (stmt == block->lastStmt()) // Is it the last statement in the list?
@@ -10042,10 +10040,10 @@ void Compiler::fgRemoveStmt(BasicBlock* block, GenTreeStmt* stmt)
     {
         assert(stmt->gtPrevStmt != nullptr && stmt->gtNext != nullptr);
 
-        tree = stmt->gtPrevStmt;
+        GenTreeStmt* prev = stmt->gtPrevStmt;
 
-        tree->gtNext         = stmt->gtNext;
-        stmt->gtNext->gtPrev = tree;
+        prev->gtNext         = stmt->gtNext;
+        stmt->gtNext->gtPrev = prev;
     }
 
     noway_assert(!optValnumCSE_phase);
@@ -10299,15 +10297,15 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
     }
     else
     {
-        GenTree* blkNonPhi1   = block->FirstNonPhiDef();
-        GenTree* bNextNonPhi1 = bNext->FirstNonPhiDef();
-        GenTree* blkFirst     = block->firstStmt();
-        GenTree* bNextFirst   = bNext->firstStmt();
+        GenTreeStmt* blkNonPhi1   = block->FirstNonPhiDef();
+        GenTreeStmt* bNextNonPhi1 = bNext->FirstNonPhiDef();
+        GenTreeStmt* blkFirst     = block->firstStmt();
+        GenTreeStmt* bNextFirst   = bNext->firstStmt();
 
         // Does the second have any phis?
         if (bNextFirst != nullptr && bNextFirst != bNextNonPhi1)
         {
-            GenTree* bNextLast = bNextFirst->gtPrev;
+            GenTreeStmt* bNextLast = bNextFirst->gtPrevStmt;
             assert(bNextLast->gtNext == nullptr);
 
             // Does "blk" have phis?
@@ -10316,28 +10314,28 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
                 // Yes, has phis.
                 // Insert after the last phi of "block."
                 // First, bNextPhis after last phi of block.
-                GenTree* blkLastPhi;
+                GenTreeStmt* blkLastPhi;
                 if (blkNonPhi1 != nullptr)
                 {
-                    blkLastPhi = blkNonPhi1->gtPrev;
+                    blkLastPhi = blkNonPhi1->gtPrevStmt;
                 }
                 else
                 {
-                    blkLastPhi = blkFirst->gtPrev;
+                    blkLastPhi = blkFirst->gtPrevStmt;
                 }
 
                 blkLastPhi->gtNext = bNextFirst;
                 bNextFirst->gtPrev = blkLastPhi;
 
                 // Now, rest of "block" after last phi of "bNext".
-                GenTree* bNextLastPhi = nullptr;
+                GenTreeStmt* bNextLastPhi = nullptr;
                 if (bNextNonPhi1 != nullptr)
                 {
-                    bNextLastPhi = bNextNonPhi1->gtPrev;
+                    bNextLastPhi = bNextNonPhi1->gtPrevStmt;
                 }
                 else
                 {
-                    bNextLastPhi = bNextFirst->gtPrev;
+                    bNextLastPhi = bNextFirst->gtPrevStmt;
                 }
 
                 bNextLastPhi->gtNext = blkNonPhi1;
@@ -10363,19 +10361,19 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
                 if (blkFirst != nullptr) // If "block" has no statements, fusion will work fine...
                 {
                     // First, bNextPhis at start of block.
-                    GenTree* blkLast  = blkFirst->gtPrev;
-                    block->bbTreeList = bNextFirst;
+                    GenTreeStmt* blkLast = blkFirst->gtPrevStmt;
+                    block->bbTreeList    = bNextFirst;
                     // Now, rest of "block" (if it exists) after last phi of "bNext".
-                    GenTree* bNextLastPhi = nullptr;
+                    GenTreeStmt* bNextLastPhi = nullptr;
                     if (bNextNonPhi1 != nullptr)
                     {
                         // There is a first non phi, so the last phi is before it.
-                        bNextLastPhi = bNextNonPhi1->gtPrev;
+                        bNextLastPhi = bNextNonPhi1->gtPrevStmt;
                     }
                     else
                     {
                         // All the statements are phi defns, so the last one is the prev of the first.
-                        bNextLastPhi = bNextFirst->gtPrev;
+                        bNextLastPhi = bNextFirst->gtPrevStmt;
                     }
                     bNextFirst->gtPrev   = blkLast;
                     bNextLastPhi->gtNext = blkFirst;
@@ -10723,7 +10721,7 @@ void Compiler::fgUnreachableBlock(BasicBlock* block)
         // TODO-Cleanup: I'm not sure why this happens -- if the block is unreachable, why does it have phis?
         // Anyway, remove any phis.
 
-        GenTree* firstNonPhi = block->FirstNonPhiDef();
+        GenTreeStmt* firstNonPhi = block->FirstNonPhiDef();
         if (block->bbTreeList != firstNonPhi)
         {
             if (firstNonPhi != nullptr)
@@ -10733,7 +10731,7 @@ void Compiler::fgUnreachableBlock(BasicBlock* block)
             block->bbTreeList = firstNonPhi;
         }
 
-        for (GenTreeStmt* stmt = block->firstStmt(); stmt; stmt = stmt->gtNextStmt)
+        for (GenTreeStmt* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
         {
             fgRemoveStmt(block, stmt);
         }
@@ -14634,7 +14632,7 @@ bool Compiler::fgOptimizeBranchToNext(BasicBlock* block, BasicBlock* bNext, Basi
     {
         /* remove the conditional statement at the end of block */
         noway_assert(block->bbJumpKind == BBJ_COND);
-        noway_assert(block->bbTreeList);
+        noway_assert(block->bbTreeList != nullptr);
 
 #ifdef DEBUG
         if (verbose)
@@ -14929,7 +14927,7 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
     for (GenTreeStmt* curStmt = bDest->firstStmt(); curStmt != nullptr; curStmt = curStmt->getNextStmt())
     {
         // Clone/substitute the expression.
-        GenTreeStmt* stmt = gtCloneExpr(curStmt)->AsStmt();
+        GenTreeStmt* stmt = gtCloneStmt(curStmt);
 
         // cloneExpr doesn't handle everything.
         if (stmt == nullptr)
@@ -21083,7 +21081,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
     // Make sure the one return BB is not changed.
     if (genReturnBB != nullptr)
     {
-        assert(genReturnBB->bbTreeList);
+        assert(genReturnBB->bbTreeList != nullptr);
         assert(genReturnBB->IsLIR() || genReturnBB->bbTreeList->gtOper == GT_STMT);
         assert(genReturnBB->IsLIR() || genReturnBB->bbTreeList->gtType == TYP_VOID);
     }
@@ -22962,7 +22960,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
         if (InlineeCompiler->fgFirstBB->bbJumpKind == BBJ_RETURN)
         {
             // Inlinee contains just one BB. So just insert its statement list to topBlock.
-            if (InlineeCompiler->fgFirstBB->bbTreeList)
+            if (InlineeCompiler->fgFirstBB->bbTreeList != nullptr)
             {
                 stmtAfter = fgInsertStmtListAfter(iciBlock, stmtAfter, InlineeCompiler->fgFirstBB->firstStmt());
 
@@ -23020,24 +23018,10 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
         bottomBlock->bbFlags |= originalFlags & BBF_SPLIT_GAINED;
     }
 
-    //
-    // Split statements between topBlock and bottomBlock
-    //
-    GenTree* topBlock_Begin;
-    GenTree* topBlock_End;
-    GenTree* bottomBlock_Begin;
-    GenTree* bottomBlock_End;
-
-    topBlock_Begin    = nullptr;
-    topBlock_End      = nullptr;
-    bottomBlock_Begin = nullptr;
-    bottomBlock_End   = nullptr;
-
-    //
+    // Split statements between topBlock and bottomBlock.
     // First figure out bottomBlock_Begin
-    //
-
-    bottomBlock_Begin = stmtAfter->gtNext;
+    GenTreeStmt* bottomBlock_Begin;
+    bottomBlock_Begin = stmtAfter->gtNextStmt;
 
     if (topBlock->bbTreeList == nullptr)
     {
@@ -23049,7 +23033,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     }
     else if (topBlock->bbTreeList == bottomBlock_Begin)
     {
-        noway_assert(bottomBlock_Begin);
+        noway_assert(bottomBlock_Begin != nullptr);
 
         // topBlock contains at least one statement before the split.
         // And the split is before the first statement.
@@ -23059,7 +23043,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     }
     else if (bottomBlock_Begin == nullptr)
     {
-        noway_assert(topBlock->bbTreeList);
+        noway_assert(topBlock->bbTreeList != nullptr);
 
         // topBlock contains at least one statement before the split.
         // And the split is at the end of the topBlock.
@@ -23069,16 +23053,16 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     }
     else
     {
-        noway_assert(topBlock->bbTreeList);
-        noway_assert(bottomBlock_Begin);
+        noway_assert(topBlock->bbTreeList != nullptr);
+        noway_assert(bottomBlock_Begin != nullptr);
 
         // This is the normal case where both blocks should contain at least one statement.
-        topBlock_Begin = topBlock->bbTreeList;
-        noway_assert(topBlock_Begin);
-        topBlock_End = bottomBlock_Begin->gtPrev;
-        noway_assert(topBlock_End);
-        bottomBlock_End = topBlock->lastStmt();
-        noway_assert(bottomBlock_End);
+        GenTreeStmt* topBlock_Begin = topBlock->firstStmt();
+        noway_assert(topBlock_Begin != nullptr);
+        GenTreeStmt* topBlock_End = bottomBlock_Begin->gtPrevStmt;
+        noway_assert(topBlock_End != nullptr);
+        GenTreeStmt* bottomBlock_End = topBlock->lastStmt();
+        noway_assert(bottomBlock_End != nullptr);
 
         // Break the linkage between 2 blocks.
         topBlock_End->gtNext = nullptr;

src/coreclr/src/jit/gentree.cpp

@@ -329,6 +329,7 @@ void GenTree::InitNodeSize()
     static_assert_no_msg(sizeof(GenTreeObj)          <= TREE_NODE_SZ_LARGE); // *** large node
     static_assert_no_msg(sizeof(GenTreeBlk)          <= TREE_NODE_SZ_SMALL);
     static_assert_no_msg(sizeof(GenTreeRetExpr)      <= TREE_NODE_SZ_LARGE); // *** large node
+    static_assert_no_msg(sizeof(GenTreeILOffset)     <= TREE_NODE_SZ_SMALL);
     static_assert_no_msg(sizeof(GenTreeStmt)         <= TREE_NODE_SZ_LARGE); // *** large node
     static_assert_no_msg(sizeof(GenTreeClsVar)       <= TREE_NODE_SZ_SMALL);
     static_assert_no_msg(sizeof(GenTreeArgPlace)     <= TREE_NODE_SZ_SMALL);
@@ -2079,12 +2080,6 @@ AGAIN:
             }
             break;
 
-        case GT_STMT:
-            temp = tree->gtStmt.gtStmtExpr;
-            assert(temp);
-            hash = genTreeHashAdd(hash, gtHashValue(temp));
-            break;
-
         case GT_ARR_ELEM:
 
             hash = genTreeHashAdd(hash, gtHashValue(tree->gtArrElem.gtArrObj));
@@ -7316,8 +7311,7 @@ GenTree* Compiler::gtCloneExpr(
     switch (oper)
     {
         case GT_STMT:
-            copy = gtCloneExpr(tree->gtStmt.gtStmtExpr, addFlags, deepVarNum, deepVarVal);
-            copy = gtNewStmt(copy, tree->gtStmt.gtStmtILoffsx);
+            NO_WAY("Use gtCloneStmt instead");
             goto DONE;
 
         case GT_CALL:
@@ -10468,13 +10462,13 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack)
 
         case GT_IL_OFFSET:
             printf(" IL offset: ");
-            if (tree->gtStmt.gtStmtILoffsx == BAD_IL_OFFSET)
+            if (tree->gtILOffset.gtStmtILoffsx == BAD_IL_OFFSET)
             {
                 printf("???");
             }
             else
             {
-                printf("0x%x", jitGetILoffs(tree->gtStmt.gtStmtILoffsx));
+                printf("0x%x", jitGetILoffs(tree->gtILOffset.gtStmtILoffsx));
             }
             break;
 

src/coreclr/src/jit/gentree.h

@@ -4886,6 +4886,29 @@ struct GenTreeRetExpr : public GenTree
 
 class InlineContext;
 
+struct GenTreeILOffset : public GenTree
+{
+    IL_OFFSETX gtStmtILoffsx; // instr offset (if available)
+#ifdef DEBUG
+    IL_OFFSET gtStmtLastILoffs; // instr offset at end of stmt
+#endif
+
+    GenTreeILOffset(IL_OFFSETX offset)
+        : GenTree(GT_IL_OFFSET, TYP_VOID)
+        , gtStmtILoffsx(offset)
+#ifdef DEBUG
+        , gtStmtLastILoffs(BAD_IL_OFFSET)
+#endif
+    {
+    }
+
+#if DEBUGGABLE_GENTREE
+    GenTreeILOffset() : GenTree(GT_IL_OFFSET, TYP_VOID)
+    {
+    }
+#endif
+};
+
 struct GenTreeStmt : public GenTree
 {
     GenTree*       gtStmtExpr;      // root of the expression tree

src/coreclr/src/jit/gtstructs.h

@@ -89,7 +89,8 @@ GTSTRUCT_1(ArrElem     , GT_ARR_ELEM)
 GTSTRUCT_1(ArrOffs     , GT_ARR_OFFSET)
 GTSTRUCT_1(ArrIndex    , GT_ARR_INDEX) 
 GTSTRUCT_1(RetExpr     , GT_RET_EXPR) 
-GTSTRUCT_2(Stmt        , GT_STMT, GT_IL_OFFSET) 
+GTSTRUCT_1(Stmt        , GT_STMT)
+GTSTRUCT_1(ILOffset    , GT_IL_OFFSET)
 GTSTRUCT_2(CopyOrReload, GT_COPY, GT_RELOAD)
 GTSTRUCT_2(ClsVar      , GT_CLS_VAR, GT_CLS_VAR_ADDR) 
 GTSTRUCT_1(ArgPlace    , GT_ARGPLACE) 

src/coreclr/src/jit/importer.cpp

@@ -2557,11 +2557,11 @@ BasicBlock* Compiler::impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_H
     if ((hndBlk->bbFlags & (BBF_IMPORTED | BBF_INTERNAL | BBF_DONT_REMOVE | BBF_HAS_LABEL | BBF_JMP_TARGET)) ==
         (BBF_IMPORTED | BBF_INTERNAL | BBF_DONT_REMOVE | BBF_HAS_LABEL | BBF_JMP_TARGET))
     {
-        GenTree* tree = hndBlk->bbTreeList;
+        GenTreeStmt* stmt = hndBlk->firstStmt();
 
-        if (tree != nullptr && tree->gtOper == GT_STMT)
+        if (stmt != nullptr)
         {
-            tree = tree->gtStmt.gtStmtExpr;
+            GenTree* tree = stmt->gtStmtExpr;
             assert(tree != nullptr);
 
             if ((tree->gtOper == GT_ASG) && (tree->gtOp.gtOp1->gtOper == GT_LCL_VAR) &&
@@ -9211,10 +9211,10 @@ void Compiler::impImportLeave(BasicBlock* block)
     assert(block->bbJumpKind == BBJ_LEAVE);
     assert(fgBBs == (BasicBlock**)0xCDCD || fgLookupBB(jmpAddr) != NULL); // should be a BB boundary
 
-    BasicBlock* step         = DUMMY_INIT(NULL);
-    unsigned    encFinallies = 0; // Number of enclosing finallies.
-    GenTree*    endCatches   = NULL;
-    GenTree*    endLFin      = NULL; // The statement tree to indicate the end of locally-invoked finally.
+    BasicBlock*  step         = DUMMY_INIT(NULL);
+    unsigned     encFinallies = 0; // Number of enclosing finallies.
+    GenTree*     endCatches   = NULL;
+    GenTreeStmt* endLFinStmt  = NULL; // The statement tree to indicate the end of locally-invoked finally.
 
     unsigned  XTnum;
     EHblkDsc* HBtab;
@@ -9269,7 +9269,8 @@ void Compiler::impImportLeave(BasicBlock* block)
 
             BasicBlock* callBlock;
 
-            assert(!encFinallies == !endLFin); // if we have finallies, we better have an endLFin tree, and vice-versa
+            assert(!encFinallies ==
+                   !endLFinStmt); // if we have finallies, we better have an endLFin tree, and vice-versa
 
             if (encFinallies == 0)
             {
@@ -9316,17 +9317,17 @@ void Compiler::impImportLeave(BasicBlock* block)
 
                 if (endCatches)
                 {
-                    lastStmt         = gtNewStmt(endCatches);
-                    endLFin->gtNext  = lastStmt;
-                    lastStmt->gtPrev = endLFin;
+                    lastStmt            = gtNewStmt(endCatches);
+                    endLFinStmt->gtNext = lastStmt;
+                    lastStmt->gtPrev    = endLFinStmt;
                 }
                 else
                 {
-                    lastStmt = endLFin->AsStmt();
+                    lastStmt = endLFinStmt;
                 }
 
                 // note that this sets BBF_IMPORTED on the block
-                impEndTreeList(callBlock, endLFin->AsStmt(), lastStmt);
+                impEndTreeList(callBlock, endLFinStmt, lastStmt);
             }
 
             step = fgNewBBafter(BBJ_ALWAYS, callBlock, true);
@@ -9346,8 +9347,8 @@ void Compiler::impImportLeave(BasicBlock* block)
             assert(finallyNesting <= compHndBBtabCount);
 
             callBlock->bbJumpDest = HBtab->ebdHndBeg; // This callBlock will call the "finally" handler.
-            endLFin               = new (this, GT_END_LFIN) GenTreeVal(GT_END_LFIN, TYP_VOID, finallyNesting);
-            endLFin               = gtNewStmt(endLFin);
+            GenTree* endLFin      = new (this, GT_END_LFIN) GenTreeVal(GT_END_LFIN, TYP_VOID, finallyNesting);
+            endLFinStmt           = gtNewStmt(endLFin);
             endCatches            = NULL;
 
             encFinallies++;
@@ -9358,7 +9359,7 @@ void Compiler::impImportLeave(BasicBlock* block)
 
     /* Append any remaining endCatches, if any */
 
-    assert(!encFinallies == !endLFin);
+    assert(!encFinallies == !endLFinStmt);
 
     if (encFinallies == 0)
     {
@@ -9407,16 +9408,16 @@ void Compiler::impImportLeave(BasicBlock* block)
 
         if (endCatches)
         {
-            lastStmt         = gtNewStmt(endCatches);
-            endLFin->gtNext  = lastStmt;
-            lastStmt->gtPrev = endLFin;
+            lastStmt            = gtNewStmt(endCatches);
+            endLFinStmt->gtNext = lastStmt;
+            lastStmt->gtPrev    = endLFinStmt;
         }
         else
         {
-            lastStmt = endLFin->AsStmt();
+            lastStmt = endLFinStmt;
         }
 
-        impEndTreeList(finalStep, endLFin->AsStmt(), lastStmt);
+        impEndTreeList(finalStep, endLFinStmt, lastStmt);
 
         finalStep->bbJumpDest = leaveTarget; // this is the ultimate destination of the LEAVE
 

src/coreclr/src/jit/indirectcalltransformer.cpp

@@ -351,7 +351,7 @@ private:
         virtual void CreateThen()
         {
             thenBlock                       = CreateAndInsertBasicBlock(BBJ_ALWAYS, checkBlock);
-            GenTreeStmt* copyOfOriginalStmt = compiler->gtCloneExpr(stmt)->AsStmt();
+            GenTreeStmt* copyOfOriginalStmt = compiler->gtCloneStmt(stmt);
             compiler->fgInsertStmtAtEnd(thenBlock, copyOfOriginalStmt);
         }
 
@@ -411,7 +411,7 @@ private:
         //    created call node.
         GenTreeStmt* CreateFatCallStmt(GenTree* actualCallAddress, GenTree* hiddenArgument)
         {
-            GenTreeStmt* fatStmt = compiler->gtCloneExpr(stmt)->AsStmt();
+            GenTreeStmt* fatStmt = compiler->gtCloneStmt(stmt);
             GenTree*     fatTree = fatStmt->gtStmtExpr;
             GenTreeCall* fatCall = GetCall(fatStmt);
             fatCall->gtCallAddr  = actualCallAddress;

src/coreclr/src/jit/morph.cpp

@@ -15192,7 +15192,7 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
 
     if (block->bbJumpKind == BBJ_COND)
     {
-        noway_assert(block->bbTreeList && block->bbTreeList->gtPrev);
+        noway_assert(block->bbTreeList != nullptr && block->bbTreeList->gtPrev != nullptr);
 
         GenTreeStmt* lastStmt = block->lastStmt();
 
@@ -15400,7 +15400,7 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
     }
     else if (block->bbJumpKind == BBJ_SWITCH)
     {
-        noway_assert(block->bbTreeList && block->bbTreeList->gtPrev);
+        noway_assert(block->bbTreeList != nullptr && block->bbTreeList->gtPrev != nullptr);
 
         GenTreeStmt* lastStmt = block->lastStmt();
 

src/coreclr/src/jit/optimizer.cpp

@@ -1302,7 +1302,7 @@ bool Compiler::optRecordLoop(BasicBlock*   head,
             do
             {
                 block = block->bbNext;
-                for (GenTreeStmt* stmt = block->firstStmt(); stmt; stmt = stmt->gtNextStmt)
+                for (GenTreeStmt* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
                 {
                     if (stmt->gtStmtExpr == incr)
                     {
@@ -3476,8 +3476,8 @@ void Compiler::optUnrollLoops()
 
     bool change = false;
 
-    // Visit loops from highest to lowest number to vist them in innermost
-    // to outermost order
+    // Visit loops from highest to lowest number to visit them in innermost
+    // to outermost order.
     for (unsigned lnum = optLoopCount - 1; lnum != ~0U; --lnum)
     {
         // This is necessary due to an apparent analysis limitation since
@@ -3489,12 +3489,6 @@ void Compiler::optUnrollLoops()
         BasicBlock* head;
         BasicBlock* bottom;
 
-        GenTree* loop;
-        GenTree* test;
-        GenTree* incr;
-        GenTree* phdr;
-        GenTree* init;
-
         bool       dupCond;
         int        lval;
         int        lbeg;         // initial value for iterator
@@ -3610,28 +3604,23 @@ void Compiler::optUnrollLoops()
             continue;
         }
 
-        /* Locate the pre-header and initialization and increment/test statements */
+        // Locate/initialize the increment/test statements.
+        GenTreeStmt* initStmt = head->lastStmt();
+        noway_assert((initStmt != nullptr) && (initStmt->gtNext == nullptr));
 
-        phdr = head->bbTreeList;
-        noway_assert(phdr);
-        loop = bottom->bbTreeList;
-        noway_assert(loop);
+        GenTreeStmt* testStmt = bottom->lastStmt();
+        noway_assert((testStmt != nullptr) && (testStmt->gtNext == nullptr));
+        GenTreeStmt* incrStmt = testStmt->gtPrevStmt;
+        noway_assert(incrStmt != nullptr);
 
-        init = head->lastStmt();
-        noway_assert(init && (init->gtNext == nullptr));
-        test = bottom->lastStmt();
-        noway_assert(test && (test->gtNext == nullptr));
-        incr = test->gtPrev;
-        noway_assert(incr);
-
-        if (init->gtFlags & GTF_STMT_CMPADD)
+        if ((initStmt->gtFlags & GTF_STMT_CMPADD) != 0)
         {
             /* Must be a duplicated loop condition */
-            noway_assert(init->gtStmt.gtStmtExpr->gtOper == GT_JTRUE);
+            noway_assert(initStmt->gtStmtExpr->gtOper == GT_JTRUE);
 
-            dupCond = true;
-            init    = init->gtPrev;
-            noway_assert(init);
+            dupCond  = true;
+            initStmt = initStmt->gtPrevStmt;
+            noway_assert(initStmt != nullptr);
         }
         else
         {
@@ -3652,12 +3641,7 @@ void Compiler::optUnrollLoops()
             continue;
         }
 
-        noway_assert(init->gtOper == GT_STMT);
-        init = init->gtStmt.gtStmtExpr;
-        noway_assert(test->gtOper == GT_STMT);
-        test = test->gtStmt.gtStmtExpr;
-        noway_assert(incr->gtOper == GT_STMT);
-        incr = incr->gtStmt.gtStmtExpr;
+        GenTree* incr = incrStmt->gtStmtExpr;
 
         // Don't unroll loops we don't understand.
         if (incr->gtOper != GT_ASG)
@@ -3666,6 +3650,9 @@ void Compiler::optUnrollLoops()
         }
         incr = incr->gtOp.gtOp2;
 
+        GenTree* init = initStmt->gtStmtExpr;
+        GenTree* test = testStmt->gtStmtExpr;
+
         /* Make sure everything looks ok */
         if ((init->gtOper != GT_ASG) || (init->gtOp.gtOp1->gtOper != GT_LCL_VAR) ||
             (init->gtOp.gtOp1->gtLclVarCommon.gtLclNum != lvar) || (init->gtOp.gtOp2->gtOper != GT_CNS_INT) ||
@@ -3675,7 +3662,7 @@ void Compiler::optUnrollLoops()
             (incr->gtOp.gtOp1->gtLclVarCommon.gtLclNum != lvar) || (incr->gtOp.gtOp2->gtOper != GT_CNS_INT) ||
             (incr->gtOp.gtOp2->gtIntCon.gtIconVal != iterInc) ||
 
-            (test->gtOper != GT_JTRUE))
+            (testStmt->gtStmtExpr->gtOper != GT_JTRUE))
         {
             noway_assert(!"Bad precondition in Compiler::optUnrollLoops()");
             continue;
@@ -3703,9 +3690,9 @@ void Compiler::optUnrollLoops()
                     ++loopRetCount;
                 }
 
-                /* Visit all the statements in the block */
+                // Visit all the statements in the block.
 
-                for (GenTreeStmt* stmt = block->firstStmt(); stmt; stmt = stmt->gtNextStmt)
+                for (GenTreeStmt* stmt = block->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
                 {
                     /* Calculate gtCostSz */
                     gtSetStmtInfo(stmt);
@@ -3863,21 +3850,19 @@ void Compiler::optUnrollLoops()
 
             if (head->bbJumpKind == BBJ_COND)
             {
-                phdr = head->bbTreeList;
-                noway_assert(phdr);
-                test = phdr->gtPrev;
+                GenTreeStmt* preHeaderStmt = head->firstStmt();
+                noway_assert(preHeaderStmt != nullptr);
+                testStmt = preHeaderStmt->gtPrevStmt;
 
-                noway_assert(test && (test->gtNext == nullptr));
-                noway_assert(test->gtOper == GT_STMT);
-                noway_assert(test->gtStmt.gtStmtExpr->gtOper == GT_JTRUE);
+                noway_assert((testStmt != nullptr) && (testStmt->gtNext == nullptr));
+                noway_assert(testStmt->gtStmtExpr->gtOper == GT_JTRUE);
 
-                init = test->gtPrev;
-                noway_assert(init && (init->gtNext == test));
-                noway_assert(init->gtOper == GT_STMT);
+                initStmt = testStmt->gtPrevStmt;
+                noway_assert((initStmt != nullptr) && (initStmt->gtNext == testStmt));
 
-                init->gtNext     = nullptr;
-                phdr->gtPrev     = init;
-                head->bbJumpKind = BBJ_NONE;
+                initStmt->gtNext      = nullptr;
+                preHeaderStmt->gtPrev = initStmt;
+                head->bbJumpKind      = BBJ_NONE;
                 head->bbFlags &= ~BBF_NEEDS_GCPOLL;
             }
             else
@@ -3891,7 +3876,7 @@ void Compiler::optUnrollLoops()
             {
                 printf("Whole unrolled loop:\n");
 
-                gtDispTree(init);
+                gtDispTree(initStmt->gtStmtExpr);
                 printf("\n");
                 fgDumpTrees(head->bbNext, insertAfter);
             }
@@ -4024,7 +4009,7 @@ static GenTreeStmt* optFindLoopTermTest(BasicBlock* bottom)
 {
     GenTreeStmt* testStmt = bottom->firstStmt();
 
-    assert(testStmt);
+    assert(testStmt != nullptr);
 
     GenTreeStmt* result = testStmt->getPrevStmt();
 
@@ -6031,9 +6016,9 @@ bool Compiler::optIsVarAssigned(BasicBlock* beg, BasicBlock* end, GenTree* skip,
 
     for (;;)
     {
-        noway_assert(beg);
+        noway_assert(beg != nullptr);
 
-        for (GenTreeStmt* stmt = beg->firstStmt(); stmt; stmt = stmt->gtNextStmt)
+        for (GenTreeStmt* stmt = beg->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
         {
             if (fgWalkTreePre(&stmt->gtStmtExpr, optIsVarAssgCB, &desc))
             {
@@ -6235,18 +6220,18 @@ void Compiler::optPerformHoistExpr(GenTree* origExpr, unsigned lnum)
 
     /* simply append the statement at the end of the preHead's list */
 
-    GenTree* treeList = preHead->bbTreeList;
+    GenTreeStmt* firstStmt = preHead->firstStmt();
 
-    if (treeList)
+    if (firstStmt != nullptr)
     {
         /* append after last statement */
 
-        GenTree* last = treeList->gtPrev;
-        assert(last->gtNext == nullptr);
+        GenTreeStmt* lastStmt = preHead->lastStmt();
+        assert(lastStmt->gtNext == nullptr);
 
-        last->gtNext      = hoistStmt;
-        hoistStmt->gtPrev = last;
-        treeList->gtPrev  = hoistStmt;
+        lastStmt->gtNext  = hoistStmt;
+        hoistStmt->gtPrev = lastStmt;
+        firstStmt->gtPrev = hoistStmt;
     }
     else
     {

src/coreclr/src/jit/rationalize.cpp

@@ -957,9 +957,8 @@ void Rationalizer::DoPhase()
             if (statement->gtStmtILoffsx != BAD_IL_OFFSET)
             {
                 assert(!statement->IsPhiDefnStmt());
-                statement->SetOper(GT_IL_OFFSET);
-
-                BlockRange().InsertBefore(statement->gtStmtList, statement);
+                GenTreeILOffset* ilOffset = new (comp, GT_IL_OFFSET) GenTreeILOffset(statement->gtStmtILoffsx);
+                BlockRange().InsertBefore(statement->gtStmtList, ilOffset);
             }
 
             m_block = block;

src/coreclr/src/jit/ssabuilder.cpp

@@ -99,8 +99,8 @@ void Compiler::fgResetForSsa()
         }
         if (blk->bbTreeList != nullptr)
         {
-            GenTree* last   = blk->bbTreeList->gtPrev;
-            blk->bbTreeList = blk->FirstNonPhiDef();
+            GenTreeStmt* last = blk->lastStmt();
+            blk->bbTreeList   = blk->FirstNonPhiDef();
             if (blk->bbTreeList != nullptr)
             {
                 blk->bbTreeList->gtPrev = last;