More Struct-related Refactorings

- Make GT_OBJ and GT_NULLCHECK derive from GenTreeIndir, as they are also indirections.
- Extract the ordering-related methods from gentree and optcse, as they will need to be used slightly differently for struct assignments.
- Make fgMorphImplicitByRefArgs take a pointer to the tree, as it may need to replace the tree with one that a simple CopyFrom() will not work for.
- In gtSetEvalOrder(), in the addressing mode code, replace "adr" with "base" and "op1" with "addr", as the existing names are quite confusing.
- Other minor refactorings to reduce future diffs.

7 files changed, 247 insertions, 184 deletions

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index 324a19de62..033837e172 100644
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -2006,12 +2006,16 @@ public:
     GenTreePtr              gtWalkOpEffectiveVal(GenTreePtr op);
 #endif
 
-    void                    gtPrepareCost   (GenTree *      tree);
-    bool                    gtIsLikelyRegVar(GenTree *      tree);
+    void                    gtPrepareCost   (GenTree*       tree);
+    bool                    gtIsLikelyRegVar(GenTree*       tree);
 
     unsigned                gtSetEvalOrderAndRestoreFPstkLevel(GenTree *      tree);
 
-    unsigned                gtSetEvalOrder  (GenTree *      tree);
+    // Returns true iff the secondNode can be swapped with firstNode.
+    bool                    gtCanSwapOrder  (GenTree*       firstNode,
+                                             GenTree*       secondNode);
+
+    unsigned                gtSetEvalOrder  (GenTree*       tree);
 
 #if FEATURE_STACK_FP_X87
     bool                    gtFPstLvlRedo;
@@ -4772,7 +4776,7 @@ private:
     fgWalkResult        fgMorphStructField(GenTreePtr tree, fgWalkData *fgWalkPre);
     fgWalkResult        fgMorphLocalField(GenTreePtr tree, fgWalkData *fgWalkPre);
     void                fgMarkImplicitByRefArgs();
-    bool                fgMorphImplicitByRefArgs(GenTreePtr tree, fgWalkData *fgWalkPre);
+    bool                fgMorphImplicitByRefArgs(GenTree** pTree, fgWalkData *fgWalkPre);
     static fgWalkPreFn  fgMarkAddrTakenLocalsPreCB;
     static fgWalkPostFn fgMarkAddrTakenLocalsPostCB;
     void                fgMarkAddressExposedLocals();
@@ -5405,8 +5409,9 @@ protected :
     //
     void                optCSE_GetMaskData (GenTreePtr tree, optCSE_MaskData* pMaskData);
 
-    // Given a binary tree node return true if it is safe to swap the order of evaluation for op1 and op2
-    bool                optCSE_canSwap (GenTreePtr tree);
+    // Given a binary tree node return true if it is safe to swap the order of evaluation for op1 and op2.
+    bool                optCSE_canSwap(GenTree* firstNode, GenTree* secondNode);
+    bool                optCSE_canSwap(GenTree* tree);
 
     static fgWalkPostFn optPropagateNonCSE;
     static fgWalkPreFn  optHasNonCSEChild;
diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index ed2e6f2377..fa2c396ecc 100644
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -6771,13 +6771,23 @@ bool                Compiler::fgIsCommaThrow(GenTreePtr tree,
     return false;
 }
 
-
+//------------------------------------------------------------------------
+// fgIsIndirOfAddrOfLocal: Determine whether "tree" is an indirection of a local.
+//
+// Arguments:
+//    tree - The tree node under consideration
+//
+// Return Value:
+//    If "tree" is a indirection (GT_IND, GT_BLK, or GT_OBJ) whose arg is an ADDR,
+//    whose arg in turn is a LCL_VAR, return that LCL_VAR node, else nullptr.
+//
+// static
 GenTreePtr          Compiler::fgIsIndirOfAddrOfLocal(GenTreePtr tree)
 {
     GenTreePtr res = nullptr;
-    if (tree->OperGet() == GT_OBJ || tree->OperIsIndir())
+    if (tree->OperIsIndir())
     {
-        GenTreePtr addr = tree->gtOp.gtOp1;
+        GenTreePtr addr = tree->AsIndir()->Addr();
 
         // Post rationalization, we can have Indir(Lea(..) trees. Therefore to recognize
         // Indir of addr of a local, skip over Lea in Indir(Lea(base, index, scale, offset))
diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 30f25b2c57..67440f25e6 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -2978,6 +2978,71 @@ bool                Compiler::gtIsLikelyRegVar(GenTree * tree)
     return true;
 }
 
+//------------------------------------------------------------------------
+// gtCanSwapOrder: Returns true iff the secondNode can be swapped with firstNode.
+//
+// Arguments:
+//    firstNode  - An operand of a tree that can have GTF_REVERSE_OPS set.
+//    secondNode - The other operand of the tree.
+//
+// Return Value:
+//    Returns a boolean indicating whether it is safe to reverse the execution
+//    order of the two trees, considering any exception, global effects, or
+//    ordering constraints.
+//
+bool
+Compiler::gtCanSwapOrder(GenTree* firstNode, GenTree*  secondNode)
+{
+    // Relative of order of global / side effects can't be swapped.
+
+    bool    canSwap = true;
+
+    if (optValnumCSE_phase)
+    {
+        canSwap = optCSE_canSwap(firstNode, secondNode);
+    }
+            
+    // We cannot swap in the presence of special side effects such as GT_CATCH_ARG.
+
+    if (canSwap &&
+        (firstNode->gtFlags & GTF_ORDER_SIDEEFF))
+    {
+        canSwap = false;
+    }
+
+    // When strict side effect order is disabled we allow GTF_REVERSE_OPS to be set
+    // when one or both sides contains a GTF_CALL or GTF_EXCEPT.
+    // Currently only the C and C++ languages allow non strict side effect order.
+
+    unsigned strictEffects = GTF_GLOB_EFFECT;
+
+    if (canSwap &&
+        (firstNode->gtFlags & strictEffects))
+    {
+        // op1 has side efects that can't be reordered.
+        // Check for some special cases where we still may be able to swap.
+
+        if (secondNode->gtFlags & strictEffects)
+        {
+            // op2 has also has non reorderable side effects - can't swap.
+            canSwap = false;
+        }
+        else
+        {
+            // No side effects in op2 - we can swap iff op1 has no way of modifying op2,
+            // i.e. through byref assignments or calls or op2 is a constant.
+
+            if (firstNode->gtFlags & strictEffects & GTF_PERSISTENT_SIDE_EFFECTS)
+            {
+                // We have to be conservative - can swap iff op2 is constant.
+                if (!secondNode->OperIsConst())
+                    canSwap = false;
+            }
+        }
+    }
+    return canSwap;
+}
+
 /*****************************************************************************
  *
  *  Given a tree, figure out the order in which its sub-operands should be
@@ -3543,17 +3608,18 @@ COMMON_CNS:
                     unsigned        mul;
 #endif
                     unsigned        cns;
-                    GenTreePtr      adr;
+                    GenTreePtr      base;
                     GenTreePtr      idx;
 
                     /* See if we can form a complex addressing mode? */
 
-                    if  (codeGen->genCreateAddrMode(op1,             // address
+                    GenTreePtr      addr = op1;
+                    if  (codeGen->genCreateAddrMode(addr,             // address
                                            0,               // mode
                                            false,           // fold
                                            RBM_NONE,        // reg mask
                                            &rev,            // reverse ops
-                                           &adr,            // base addr
+                                           &base,           // base addr
                                            &idx,            // index val
 #if SCALED_ADDR_MODES
                                            &mul,            // scaling
@@ -3564,17 +3630,17 @@ COMMON_CNS:
                         // We can form a complex addressing mode, so mark each of the interior
                         // nodes with GTF_ADDRMODE_NO_CSE and calculate a more accurate cost.
 
-                        op1->gtFlags |= GTF_ADDRMODE_NO_CSE;
+                        addr->gtFlags |= GTF_ADDRMODE_NO_CSE;
 #ifdef _TARGET_XARCH_
                         // addrmodeCount is the count of items that we used to form 
                         // an addressing mode.  The maximum value is 4 when we have
-                        // all of these:   { adr, idx, cns, mul }
+                        // all of these:   { base, idx, cns, mul }
                         //
                         unsigned addrmodeCount = 0;
-                        if (adr)
+                        if (base)
                         {
-                            costEx += adr->gtCostEx;
-                            costSz += adr->gtCostSz;
+                            costEx += base->gtCostEx;
+                            costSz += base->gtCostSz;
                             addrmodeCount++;
                         }
 
@@ -3604,7 +3670,7 @@ COMMON_CNS:
                         //                      /   \       --
                         //                  GT_ADD  'cns'   -- reduce this interior GT_ADD by (-2,-2)
                         //                  /   \           --
-                        //               'adr'  GT_LSL      -- reduce this interior GT_LSL by (-1,-1)
+                        //               'base'  GT_LSL     -- reduce this interior GT_LSL by (-1,-1)
                         //                      /   \       --
                         //                   'idx'  'mul'
                         //
@@ -3614,7 +3680,7 @@ COMMON_CNS:
                             //
                             addrmodeCount--;
 
-                            GenTreePtr tmp = op1;
+                            GenTreePtr tmp = addr;
                             while (addrmodeCount > 0)
                             {
                                 // decrement the gtCosts for the interior GT_ADD or GT_LSH node by the remaining addrmodeCount
@@ -3627,7 +3693,7 @@ COMMON_CNS:
                                     GenTreePtr tmpOp2 = tmp->gtGetOp2();
                                     assert(tmpOp2 != nullptr);
 
-                                    if ((tmpOp1 != adr) && (tmpOp1->OperGet() == GT_ADD))
+                                    if ((tmpOp1 != base) && (tmpOp1->OperGet() == GT_ADD))
                                     {
                                         tmp = tmpOp1;
                                     }
@@ -3653,11 +3719,11 @@ COMMON_CNS:
                             }
                         }
 #elif defined _TARGET_ARM_
-                        if (adr)
+                        if (base)
                         {
-                            costEx += adr->gtCostEx;
-                            costSz += adr->gtCostSz;
-                            if ((adr->gtOper == GT_LCL_VAR) &&
+                            costEx += base->gtCostEx;
+                            costSz += base->gtCostSz;
+                            if ((base->gtOper == GT_LCL_VAR) &&
                                 ((idx==NULL) || (cns==0)))
                             {
                                 costSz -= 1;
@@ -3691,10 +3757,10 @@ COMMON_CNS:
                             }
                         }
 #elif defined _TARGET_ARM64_
-                        if (adr)
+                        if (base)
                         {
-                            costEx += adr->gtCostEx;
-                            costSz += adr->gtCostSz;
+                            costEx += base->gtCostEx;
+                            costSz += base->gtCostSz;
                         }
 
                         if (idx)
@@ -3715,62 +3781,62 @@ COMMON_CNS:
 #error "Unknown _TARGET_"
 #endif
 
-                        assert(op1->gtOper == GT_ADD);
-                        assert(!op1->gtOverflow());
+                        assert(addr->gtOper == GT_ADD);
+                        assert(!addr->gtOverflow());
                         assert(op2 == NULL);
                         assert(mul != 1);
 
                         // If we have an addressing mode, we have one of:
-                        //   [adr             + cns]
-                        //   [      idx * mul      ]  // mul >= 2, else we would use adr instead of idx
-                        //   [      idx * mul + cns]  // mul >= 2, else we would use adr instead of idx
-                        //   [adr + idx * mul      ]  // mul can be 0, 2, 4, or 8
-                        //   [adr + idx * mul + cns]  // mul can be 0, 2, 4, or 8
+                        //   [base             + cns]
+                        //   [       idx * mul      ]  // mul >= 2, else we would use base instead of idx
+                        //   [       idx * mul + cns]  // mul >= 2, else we would use base instead of idx
+                        //   [base + idx * mul      ]  // mul can be 0, 2, 4, or 8
+                        //   [base + idx * mul + cns]  // mul can be 0, 2, 4, or 8
                         // Note that mul == 0 is semantically equivalent to mul == 1.
                         // Note that cns can be zero.
 #if SCALED_ADDR_MODES
-                        assert((adr != NULL) || (idx != NULL && mul >= 2));
+                        assert((base != NULL) || (idx != NULL && mul >= 2));
 #else
-                        assert(adr != NULL);
+                        assert(base != NULL);
 #endif
 
-                        INDEBUG(GenTreePtr op1Save = op1);
+                        INDEBUG(GenTreePtr op1Save = addr);
 
-                        /* Walk op1 looking for non-overflow GT_ADDs */
-                        gtWalkOp(&op1, &op2, adr, false);
+                        /* Walk addr looking for non-overflow GT_ADDs */
+                        gtWalkOp(&addr, &op2, base, false);
 
-                        // op1 and op2 are now children of the root GT_ADD of the addressing mode
-                        assert(op1 != op1Save);
+                        // addr and op2 are now children of the root GT_ADD of the addressing mode
+                        assert(addr != op1Save);
                         assert(op2 != NULL);
 
-                        /* Walk op1 looking for non-overflow GT_ADDs of constants */
-                        gtWalkOp(&op1, &op2, NULL, true);
+                        /* Walk addr looking for non-overflow GT_ADDs of constants */
+                        gtWalkOp(&addr, &op2, NULL, true);
 
                         // TODO-Cleanup: It seems very strange that we might walk down op2 now, even though the prior
                         //           call to gtWalkOp() may have altered op2.
 
                         /* Walk op2 looking for non-overflow GT_ADDs of constants */
-                        gtWalkOp(&op2, &op1, NULL, true);
+                        gtWalkOp(&op2, &addr, NULL, true);
 
                         // OK we are done walking the tree
-                        // Now assert that op1 and op2 correspond with adr and idx
+                        // Now assert that addr and op2 correspond with base and idx
                         // in one of the several acceptable ways.
 
-                        // Note that sometimes op1/op2 is equal to idx/adr
-                        // and other times op1/op2 is a GT_COMMA node with
-                        // an effective value that is idx/adr
+                        // Note that sometimes addr/op2 is equal to idx/base
+                        // and other times addr/op2 is a GT_COMMA node with
+                        // an effective value that is idx/base
 
                         if (mul > 1)
                         {
-                            if ((op1 != adr) && (op1->gtOper == GT_LSH))
+                            if ((addr != base) && (addr->gtOper == GT_LSH))
                             {
-                                op1->gtFlags |= GTF_ADDRMODE_NO_CSE;
-                                if (op1->gtOp.gtOp1->gtOper == GT_MUL)
+                                addr->gtFlags |= GTF_ADDRMODE_NO_CSE;
+                                if (addr->gtOp.gtOp1->gtOper == GT_MUL)
                                 {
-                                    op1->gtOp.gtOp1->gtFlags |= GTF_ADDRMODE_NO_CSE;
+                                    addr->gtOp.gtOp1->gtFlags |= GTF_ADDRMODE_NO_CSE;
                                 }
-                                assert((adr == NULL) || (op2 == adr) || (op2->gtEffectiveVal() == adr->gtEffectiveVal()) ||
-                                       (gtWalkOpEffectiveVal(op2) == gtWalkOpEffectiveVal(adr)));
+                                assert((base == NULL) || (op2 == base) || (op2->gtEffectiveVal() == base->gtEffectiveVal()) ||
+                                       (gtWalkOpEffectiveVal(op2) == gtWalkOpEffectiveVal(base)));
                             }
                             else
                             {
@@ -3785,7 +3851,7 @@ COMMON_CNS:
                                     op2op1->gtFlags |= GTF_ADDRMODE_NO_CSE;
                                     op2op1 = op2op1->gtOp.gtOp1;
                                 }
-                                assert(op1->gtEffectiveVal() == adr);
+                                assert(addr->gtEffectiveVal() == base);
                                 assert(op2op1 == idx);
                             }
                         }
@@ -3793,24 +3859,24 @@ COMMON_CNS:
                         {
                             assert(mul == 0);
 
-                            if      ((op1 == idx) || (op1->gtEffectiveVal() == idx))
+                            if ((addr == idx) || (addr->gtEffectiveVal() == idx))
                             {
                                 if (idx != NULL)
                                 {
-                                    if ((op1->gtOper == GT_MUL) || (op1->gtOper == GT_LSH))
+                                    if ((addr->gtOper == GT_MUL) || (addr->gtOper == GT_LSH))
                                     {
-                                        if ((op1->gtOp.gtOp1->gtOper == GT_NOP) || 
-                                            (op1->gtOp.gtOp1->gtOper == GT_MUL && op1->gtOp.gtOp1->gtOp.gtOp1->gtOper == GT_NOP))
+                                        if ((addr->gtOp.gtOp1->gtOper == GT_NOP) || 
+                                            (addr->gtOp.gtOp1->gtOper == GT_MUL && addr->gtOp.gtOp1->gtOp.gtOp1->gtOper == GT_NOP))
                                         {
-                                            op1->gtFlags |= GTF_ADDRMODE_NO_CSE;
-                                            if (op1->gtOp.gtOp1->gtOper == GT_MUL)
-                                                op1->gtOp.gtOp1->gtFlags |= GTF_ADDRMODE_NO_CSE;
+                                            addr->gtFlags |= GTF_ADDRMODE_NO_CSE;
+                                            if (addr->gtOp.gtOp1->gtOper == GT_MUL)
+                                                addr->gtOp.gtOp1->gtFlags |= GTF_ADDRMODE_NO_CSE;
                                         }
                                     }
                                 }
-                                assert((op2 == adr) || (op2->gtEffectiveVal() == adr));
+                                assert((op2 == base) || (op2->gtEffectiveVal() == base));
                             }
-                            else if ((op1 == adr) || (op1->gtEffectiveVal() == adr))
+                            else if ((addr == base) || (addr->gtEffectiveVal() == base))
                             {
                                 if (idx != NULL)
                                 {
@@ -3831,7 +3897,7 @@ COMMON_CNS:
                             }
                             else
                             {
-                                // op1 isn't adr or idx. Is this possible? Or should there be an assert?
+                                // addr isn't base or idx. Is this possible? Or should there be an assert?
                             }
                         }
                         goto DONE;
@@ -4302,60 +4368,7 @@ COMMON_CNS:
 
         if (tryToSwap)
         {
-            /* Relative of order of global / side effects can't be swapped */
-
-            bool    canSwap = true;
-
-            if (optValnumCSE_phase)
-            {
-                canSwap = optCSE_canSwap(tree);
-            }
-            
-            /* We cannot swap in the presence of special side effects such as GT_CATCH_ARG */
-
-            if (canSwap &&
-                (opA->gtFlags & GTF_ORDER_SIDEEFF))
-            {
-                canSwap = false;
-            }
-
-            /*  When strict side effect order is disabled we allow
-             *  GTF_REVERSE_OPS to be set when one or both sides contains
-             *  a GTF_CALL or GTF_EXCEPT.
-             *  Currently only the C and C++ languages
-             *  allow non strict side effect order
-             */
-            unsigned strictEffects = GTF_GLOB_EFFECT;
-
-            if (canSwap &&
-                (opA->gtFlags & strictEffects))
-            {
-                /*  op1 has side efects, that can't be reordered.
-                 *  Check for some special cases where we still
-                 *  may be able to swap
-                 */
-
-                if (opB->gtFlags & strictEffects)
-                {
-                    /* op2 has also has non reorderable side effects - can't swap */
-                    canSwap = false;
-                }
-                else
-                {
-                    /* No side effects in op2 - we can swap iff
-                     *  op1 has no way of modifying op2,
-                     *  i.e. through byref assignments or calls
-                     *       unless op2 is a constant
-                     */
-
-                    if (opA->gtFlags & strictEffects & GTF_PERSISTENT_SIDE_EFFECTS)
-                    {
-                        /* We have to be conservative - can swap iff op2 is constant */
-                        if (!opB->OperIsConst())
-                            canSwap = false;
-                    }
-                }
-            }
+            bool canSwap = gtCanSwapOrder(opA, opB);
 
             if  (canSwap)
             {
@@ -12191,7 +12204,7 @@ void                Compiler::gtExtractSideEffList(GenTreePtr expr, GenTreePtr *
             // Special case - GT_ADDR of GT_IND nodes of TYP_STRUCT
             // have to be kept together
 
-            if (oper == GT_ADDR && op1->gtOper == GT_IND && op1->gtType == TYP_STRUCT)
+            if (oper == GT_ADDR && op1->OperIsIndir() && op1->gtType == TYP_STRUCT)
             {
                 *pList = gtBuildCommaList(*pList, expr);
 
@@ -13388,7 +13401,7 @@ bool GenTree::isContainedIndir() const
 
 bool GenTree::isIndirAddrMode()
 { 
-    return isIndir() && gtOp.gtOp1->OperIsAddrMode() && gtOp.gtOp1->isContained(); 
+    return isIndir() && AsIndir()->Addr()->OperIsAddrMode() && AsIndir()->Addr()->isContained(); 
 }
 
 bool GenTree::isIndir() const
@@ -14041,7 +14054,7 @@ void GenTree::ParseArrayAddressWork(Compiler* comp, ssize_t inputMul, GenTreePtr
 
 bool GenTree::ParseArrayElemForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqNode** pFldSeq)
 {
-    if (OperGet() == GT_IND)
+    if (OperIsIndir())
     {
         if (gtFlags & GTF_IND_ARR_INDEX)
         {
@@ -14051,7 +14064,7 @@ bool GenTree::ParseArrayElemForm(Compiler* comp, ArrayInfo* arrayInfo, FieldSeqN
         }
 
         // Otherwise...
-        GenTreePtr addr = gtOp.gtOp1;
+        GenTreePtr addr = AsIndir()->Addr();
         return addr->ParseArrayElemAddrForm(comp, arrayInfo, pFldSeq);
     }
     else
diff --git a/src/jit/gentree.h b/src/jit/gentree.h
index 716d38553e..b9b97efde2 100644
--- a/src/jit/gentree.h
+++ b/src/jit/gentree.h
@@ -1200,7 +1200,7 @@ public:
     static
     bool            OperIsIndir(genTreeOps gtOper)
     {
-        return  gtOper == GT_IND || gtOper == GT_STOREIND || gtOper == GT_NULLCHECK;
+        return  gtOper == GT_IND || gtOper == GT_STOREIND || gtOper == GT_NULLCHECK || gtOper == GT_OBJ;
     }
 
     bool            OperIsIndir() const
@@ -3316,27 +3316,6 @@ protected:
 #endif // DEBUGGABLE_GENTREE
 };
 
-// gtObj  -- 'object' (GT_OBJ). */
-
-struct GenTreeObj: public GenTreeUnOp
-{
-    // The address of the block.
-    GenTreePtr&     Addr()          { return gtOp1; }
-
-    CORINFO_CLASS_HANDLE gtClass;   // the class of the object
-
-    GenTreeObj(var_types type, GenTreePtr addr, CORINFO_CLASS_HANDLE cls) : 
-        GenTreeUnOp(GT_OBJ, type, addr),
-        gtClass(cls)
-        {
-            gtFlags |= GTF_GLOB_REF; // An Obj is always a global reference.
-        }
-
-#if DEBUGGABLE_GENTREE
-    GenTreeObj() : GenTreeUnOp() {}
-#endif
-};
-
 // Represents a CpObj MSIL Node.
 struct GenTreeCpObj : public GenTreeBlkOp
 {
@@ -3545,6 +3524,25 @@ protected:
 #endif
 };
 
+// gtObj  -- 'object' (GT_OBJ). */
+
+struct GenTreeObj: public GenTreeIndir
+{
+    CORINFO_CLASS_HANDLE gtClass;   // the class of the object
+
+    GenTreeObj(var_types type, GenTreePtr addr, CORINFO_CLASS_HANDLE cls) : 
+        GenTreeIndir(GT_OBJ, type, addr, nullptr),
+        gtClass(cls)
+        {
+            // By default, an OBJ is assumed to be a global reference.
+            gtFlags |= GTF_GLOB_REF;
+        }
+
+#if DEBUGGABLE_GENTREE
+    GenTreeObj() : GenTreeIndir() {}
+#endif
+};
+
 // Read-modify-write status of a RMW memory op rooted at a storeInd 
 enum  RMWStatus {    
     STOREIND_RMW_STATUS_UNKNOWN,  // RMW status of storeInd unknown
diff --git a/src/jit/gtstructs.h b/src/jit/gtstructs.h
index 2f0b3a3936..06c5b9816f 100644
--- a/src/jit/gtstructs.h
+++ b/src/jit/gtstructs.h
@@ -90,7 +90,7 @@ GTSTRUCT_1(AddrMode    , GT_LEA)
 GTSTRUCT_1(Qmark       , GT_QMARK)
 GTSTRUCT_1(PhiArg      , GT_PHI_ARG)
 GTSTRUCT_1(StoreInd    , GT_STOREIND)
-GTSTRUCT_2(Indir       , GT_STOREIND, GT_IND)
+GTSTRUCT_N(Indir       , GT_STOREIND, GT_IND, GT_NULLCHECK, GT_OBJ)
 GTSTRUCT_1(PutArgStk   , GT_PUTARG_STK)
 GTSTRUCT_1(PhysReg     , GT_PHYSREG)
 GTSTRUCT_3(BlkOp       , GT_COPYBLK, GT_INITBLK, GT_COPYOBJ)
diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp
index e24e8ae7cd..95d25e30ea 100755
--- a/src/jit/morph.cpp
+++ b/src/jit/morph.cpp
@@ -4795,29 +4795,30 @@ Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call,
     // See if we need to insert a copy at all
     // Case 1: don't need a copy if it is the last use of a local.  We can't determine that all of the time
     // but if there is only one use and no loops, the use must be last.
-    if (argx->gtOper == GT_OBJ)
+    GenTreeLclVarCommon* lcl = nullptr;
+    if ((argx->OperGet() == GT_OBJ) && argx->AsObj()->Addr()->OperIsLocal())
+    {
+        lcl = argx->AsObj()->Addr()->AsLclVarCommon();
+    }
+    if (lcl != nullptr)
     {
-        GenTree* lcl = argx->gtOp.gtOp1;
-        if (lcl->OperIsLocal())
+        unsigned varNum = lcl->AsLclVarCommon()->GetLclNum();
+        if (lvaIsImplicitByRefLocal(varNum))
         {
-            unsigned varNum = lcl->AsLclVarCommon()->GetLclNum();
-            if (lvaIsImplicitByRefLocal(varNum))
+            LclVarDsc* varDsc = &lvaTable[varNum];
+            // JIT_TailCall helper has an implicit assumption that all tail call arguments live
+            // on the caller's frame. If an argument lives on the caller caller's frame, it may get
+            // overwritten if that frame is reused for the tail call. Therefore, we should always copy
+            // struct parameters if they are passed as arguments to a tail call.
+            if (!call->IsTailCallViaHelper() && (varDsc->lvRefCnt == 1) && !fgMightHaveLoop())
             {
-                LclVarDsc* varDsc = &lvaTable[varNum];
-                // JIT_TailCall helper has an implicit assumption that all tail call arguments live
-                // on the caller's frame. If an argument lives on the caller caller's frame, it may get
-                // overwritten if that frame is reused for the tail call. Therefore, we should always copy
-                // struct parameters if they are passed as arguments to a tail call.
-                if (!call->IsTailCallViaHelper() && (varDsc->lvRefCnt == 1) && !fgMightHaveLoop())
-                {
-                    varDsc->lvRefCnt = 0;
-                    args->gtOp.gtOp1 = lcl;
-                    fgArgTabEntryPtr fp = Compiler::gtArgEntryByNode(call, argx);
-                    fp->node = lcl;
+                varDsc->lvRefCnt = 0;
+                args->gtOp.gtOp1 = lcl;
+                fgArgTabEntryPtr fp = Compiler::gtArgEntryByNode(call, argx);
+                fp->node = lcl;
 
-                    JITDUMP("did not have to make outgoing copy for V%2d", varNum);
-                    return;
-                }
+                JITDUMP("did not have to make outgoing copy for V%2d", varNum);
+                return;
             }
         }
     }
@@ -5912,13 +5913,14 @@ GenTreePtr          Compiler::fgMorphField(GenTreePtr tree, MorphAddrContext* ma
                 lclNum = objRef->gtLclVarCommon.gtLclNum;
             }
 
-            // Create the "nullchk" node
-            nullchk = gtNewOperNode(GT_NULLCHECK,
-                                    TYP_BYTE,  // Make it TYP_BYTE so we only deference it for 1 byte.
-                                    gtNewLclvNode(lclNum, objRefType));
+            // Create the "nullchk" node.
+            // Make it TYP_BYTE so we only deference it for 1 byte.
+            GenTreePtr lclVar = gtNewLclvNode(lclNum, objRefType);
+            nullchk = new(this, GT_NULLCHECK) GenTreeIndir(GT_NULLCHECK, TYP_BYTE, lclVar, nullptr);
+                                    
             nullchk->gtFlags |= GTF_DONT_CSE;     // Don't try to create a CSE for these TYP_BYTE indirections
 
-            /* An indirection will cause a GPF if the address is null */
+            // An indirection will cause a GPF if the address is null.
             nullchk->gtFlags |= GTF_EXCEPT;
 
             if (asg)
@@ -6423,12 +6425,15 @@ bool                Compiler::fgCanFastTailCall(GenTreeCall* callee)
             }
 
             // Get the size of the struct and see if it is register passable.
+            CORINFO_CLASS_HANDLE objClass = nullptr;
+
             if (argx->OperGet() == GT_OBJ)
             {
+                objClass = argx->AsObj()->gtClass;
 #if defined(_TARGET_AMD64_) || defined(_TARGET_ARM64_)
 
                 unsigned typeSize = 0;
-                hasMultiByteArgs = !VarTypeIsMultiByteAndCanEnreg(argx->TypeGet(), argx->gtObj.gtClass, &typeSize, false);
+                hasMultiByteArgs = !VarTypeIsMultiByteAndCanEnreg(argx->TypeGet(), objClass, &typeSize, false);
 
 #if defined(FEATURE_UNIX_AMD64_STRUCT_PASSING) || defined(_TARGET_ARM64_)
                 // On System V/arm64 the args could be a 2 eightbyte struct that is passed in two registers.
@@ -16166,7 +16171,7 @@ void                Compiler::fgMarkImplicitByRefArgs()
  *  Morph irregular parameters
  *    for x64 and ARM64 this means turning them into byrefs, adding extra indirs.
  */
-bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr tree, fgWalkData* fgWalkPre)
+bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr *pTree, fgWalkData* fgWalkPre)
 {
 #if !defined(_TARGET_AMD64_) && !defined(_TARGET_ARM64_)
 
@@ -16174,6 +16179,7 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr tree, fgWalkData* fgWalkPre)
 
 #else // _TARGET_AMD64_ || _TARGET_ARM64_
 
+    GenTree* tree = *pTree;
     assert((tree->gtOper == GT_LCL_VAR) ||
            ((tree->gtOper == GT_ADDR) && (tree->gtOp.gtOp1->gtOper == GT_LCL_VAR)));
 
@@ -16201,6 +16207,9 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr tree, fgWalkData* fgWalkPre)
     // We are overloading the lvRefCnt field here because real ref counts have not been set.
     lclVarDsc->lvRefCnt++;
 
+    // This is no longer a def of the lclVar, even if it WAS a def of the struct.
+    lclVarTree->gtFlags &= ~(GTF_LIVENESS_MASK);
+
     if (isAddr)
     {
         // change &X into just plain X
@@ -16245,6 +16254,7 @@ bool Compiler::fgMorphImplicitByRefArgs(GenTreePtr tree, fgWalkData* fgWalkPre)
 #endif // DEBUG
     }
 
+    *pTree = tree;
     return true;
 
 #endif // _TARGET_AMD64_ || _TARGET_ARM64_
@@ -16446,7 +16456,7 @@ Compiler::fgWalkResult      Compiler::fgMarkAddrTakenLocalsPreCB(GenTreePtr* pTr
         // If we have ADDR(lcl), and "lcl" is an implicit byref parameter, fgMorphImplicitByRefArgs will
         // convert to just "lcl".  This is never an address-context use, since the local is already a
         // byref after this transformation.
-        if (tree->gtOp.gtOp1->OperGet() == GT_LCL_VAR && comp->fgMorphImplicitByRefArgs(tree, fgWalkPre))
+        if (tree->gtOp.gtOp1->OperGet() == GT_LCL_VAR && comp->fgMorphImplicitByRefArgs(pTree, fgWalkPre))
         {
             // Push something to keep the PostCB, which will pop it, happy.
             axcStack->Push(AXC_None);
@@ -16547,7 +16557,7 @@ Compiler::fgWalkResult      Compiler::fgMarkAddrTakenLocalsPreCB(GenTreePtr* pTr
     case GT_LCL_VAR:
         // On some architectures, some arguments are passed implicitly by reference.
         // Modify the trees to reflect that, if this local is one of those.
-        if (comp->fgMorphImplicitByRefArgs(tree, fgWalkPre))
+        if (comp->fgMorphImplicitByRefArgs(pTree, fgWalkPre))
         {
             // We can't be in an address context; the ADDR(lcl), where lcl is an implicit byref param, was
             // handled earlier.  (And we can't have added anything to this address, since it was implicit.)
diff --git a/src/jit/optcse.cpp b/src/jit/optcse.cpp
index fa19445aa5..c424e7e178 100644
--- a/src/jit/optcse.cpp
+++ b/src/jit/optcse.cpp
@@ -313,17 +313,24 @@ void                Compiler::optCSE_GetMaskData(GenTreePtr tree, optCSE_MaskDat
 }
 
 
-// Given a binary tree node return true if it is safe to swap the order of evaluation for op1 and op2
-// It only considers the locations of the CSE defs and uses for op1 and op2 to decide this
+//------------------------------------------------------------------------
+// optCSE_canSwap: Determine if the execution order of two nodes can be swapped.
 //
-bool                Compiler::optCSE_canSwap(GenTreePtr tree)
+// Arguments:
+//    op1 - The first node
+//    op2 - The second node
+//
+// Return Value:
+//    Return true iff it safe to swap the execution order of 'op1' and 'op2',
+//    considering only the locations of the CSE defs and uses.
+//
+// Assumptions:
+//    'op1' currently occurse before 'op2' in the execution order.
+//
+bool                Compiler::optCSE_canSwap(GenTree* op1, GenTree* op2)
 {
-    assert((tree->OperKind() & GTK_SMPOP) != 0);
-
-    GenTreePtr      op1 = tree->gtOp.gtOp1;
-    GenTreePtr      op2 = tree->gtGetOp2();
-
-    assert(op1 != nullptr);  // We must have a binary treenode with non-null op1 and op2
+    // op1 and op2 must be non-null.
+    assert(op1 != nullptr);
     assert(op2 != nullptr);
 
     bool canSwap = true;   // the default result unless proven otherwise.
@@ -341,7 +348,7 @@ bool                Compiler::optCSE_canSwap(GenTreePtr tree)
     }
     else
     {
-        // We also cannot swap if op2 contains a CSE def that is used by op1
+        // We also cannot swap if op2 contains a CSE def that is used by op1.
         if ((op2MaskData.CSE_defMask & op1MaskData.CSE_useMask) != 0)
         {
             canSwap = false;
@@ -351,6 +358,26 @@ bool                Compiler::optCSE_canSwap(GenTreePtr tree)
     return canSwap;
 }
 
+//------------------------------------------------------------------------
+// optCSE_canSwap: Determine if the execution order of a node's operands can be swapped.
+//
+// Arguments:
+//    tree - The node of interest
+//
+// Return Value:
+//    Return true iff it safe to swap the execution order of the operands of 'tree',
+//    considering only the locations of the CSE defs and uses.
+//
+bool                Compiler::optCSE_canSwap(GenTreePtr tree)
+{
+    // We must have a binary treenode with non-null op1 and op2
+    assert((tree->OperKind() & GTK_SMPOP) != 0);
+
+    GenTreePtr      op1 = tree->gtOp.gtOp1;
+    GenTreePtr      op2 = tree->gtGetOp2();
+
+    return optCSE_canSwap(op1, op2);
+}
 
 /*****************************************************************************
  *