Fold field offsets into array addresses

src/coreclr/jit/gentree.cpp

@@ -8495,7 +8495,8 @@ GenTree* Compiler::gtCloneExpr(
             case GT_ARR_ADDR:
                 copy = new (this, GT_ARR_ADDR)
                     GenTreeArrAddr(tree->AsArrAddr()->Addr(), tree->AsArrAddr()->GetElemType(),
-                                   tree->AsArrAddr()->GetElemClassHandle(), tree->AsArrAddr()->GetFirstElemOffset());
+                                   tree->AsArrAddr()->GetElemClassHandle(), tree->AsArrAddr()->GetElemSize(),
+                                   tree->AsArrAddr()->GetFirstElemOffset(), tree->AsArrAddr()->GetFieldOffset());
                 break;
 
             case GT_ARR_LENGTH:
@@ -10630,6 +10631,11 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_
                 {
                     printf("%s[]", varTypeName(elemType));
                 }
+
+                if (tree->OperIs(GT_ARR_ADDR) && (tree->AsArrAddr()->GetFieldOffset() != 0))
+                {
+                    printf(" [+%u]", tree->AsArrAddr()->GetFieldOffset());
+                }
             }
 
             if (tree->OperIsLocal())
@@ -18128,13 +18134,9 @@ void GenTreeArrAddr::ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum*
     }
 
     // OK, new we have to figure out if any part of the "offset" is a constant contribution to the index.
-    target_ssize_t elemOffset = GetFirstElemOffset();
-    unsigned       elemSizeUn = (GetElemType() == TYP_STRUCT) ? comp->typGetObjLayout(GetElemClassHandle())->GetSize()
-                                                        : genTypeSize(GetElemType());
-
-    assert(FitsIn<target_ssize_t>(elemSizeUn));
-    target_ssize_t elemSize         = static_cast<target_ssize_t>(elemSizeUn);
-    target_ssize_t constIndexOffset = offset - elemOffset;
+    assert(FitsIn<target_ssize_t>(GetElemSize()));
+    target_ssize_t elemSize         = static_cast<target_ssize_t>(GetElemSize());
+    target_ssize_t constIndexOffset = offset - (GetFirstElemOffset() + GetFieldOffset());
 
     // This should be divisible by the element size...
     assert((constIndexOffset % elemSize) == 0);
@@ -18241,25 +18243,11 @@ void GenTreeArrAddr::ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum*
                 GenTree*       nonConst = nullptr;
                 if (tree->AsOp()->gtOp1->IsCnsIntOrI())
                 {
-                    // If the other arg is an int constant, and is a "not-a-field", choose
-                    // that as the multiplier, thus preserving constant index offsets...
-                    if (tree->AsOp()->gtOp2->OperGet() == GT_CNS_INT &&
-                        tree->AsOp()->gtOp2->AsIntCon()->gtFieldSeq == nullptr)
-                    {
-                        assert(!tree->AsOp()->gtOp2->AsIntCon()->ImmedValNeedsReloc(comp));
-                        // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had
-                        // target_ssize_t type.
-                        subMul   = (target_ssize_t)tree->AsOp()->gtOp2->AsIntConCommon()->IconValue();
-                        nonConst = tree->AsOp()->gtOp1;
-                    }
-                    else
-                    {
-                        assert(!tree->AsOp()->gtOp1->AsIntCon()->ImmedValNeedsReloc(comp));
-                        // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had
-                        // target_ssize_t type.
-                        subMul   = (target_ssize_t)tree->AsOp()->gtOp1->AsIntConCommon()->IconValue();
-                        nonConst = tree->AsOp()->gtOp2;
-                    }
+                    assert(!tree->AsOp()->gtOp1->AsIntCon()->ImmedValNeedsReloc(comp));
+                    // TODO-CrossBitness: we wouldn't need the cast below if GenTreeIntConCommon::gtIconVal had
+                    // target_ssize_t type.
+                    subMul   = (target_ssize_t)tree->AsOp()->gtOp1->AsIntConCommon()->IconValue();
+                    nonConst = tree->AsOp()->gtOp2;
                 }
                 else if (tree->AsOp()->gtOp2->IsCnsIntOrI())
                 {
@@ -18323,36 +18311,6 @@ void GenTreeArrAddr::ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum*
     }
 }
 
-//------------------------------------------------------------------------
-// IsArrayAddr: Is "this" an expression for an array address?
-//
-// Recognizes the following patterns:
-//    this: ARR_ADDR
-//    this: ADD(ARR_ADDR, CONST)
-//
-// Arguments:
-//    pArrAddr - [out] parameter for the found ARR_ADDR node
-//
-// Return Value:
-//    Whether "this" matches the pattern denoted above.
-//
-bool GenTree::IsArrayAddr(GenTreeArrAddr** pArrAddr)
-{
-    GenTree* addr = this;
-    if (addr->OperIs(GT_ADD) && addr->AsOp()->gtGetOp2()->IsCnsIntOrI())
-    {
-        addr = addr->AsOp()->gtGetOp1();
-    }
-
-    if (addr->OperIs(GT_ARR_ADDR))
-    {
-        *pArrAddr = addr->AsArrAddr();
-        return true;
-    }
-
-    return false;
-}
-
 //------------------------------------------------------------------------
 // Create: Create or retrieve a field sequence for the given field handle.
 //

src/coreclr/jit/gentree.h

@@ -192,6 +192,9 @@ struct GuardedDevirtualizationCandidateInfo;
 struct HandleHistogramProfileCandidateInfo;
 struct LateDevirtualizationInfo;
 
+template <typename T>
+unsigned genTypeSize(T value);
+
 typedef unsigned short AssertionIndex;
 
 static const AssertionIndex NO_ASSERTION_INDEX = 0;
@@ -2010,8 +2013,6 @@ struct GenTree
 
     bool IsFieldAddr(Compiler* comp, GenTree** pBaseAddr, FieldSeq** pFldSeq, ssize_t* pOffset);
 
-    bool IsArrayAddr(GenTreeArrAddr** pArrAddr);
-
     // These are only used for dumping.
     // The GetRegNum() is only valid in LIR, but the dumping methods are not easily
     // modified to check this.
@@ -6425,17 +6426,27 @@ struct GenTreeArrAddr : GenTreeUnOp
     CORINFO_CLASS_HANDLE m_elemClassHandle; // The array element class. Currently only used for arrays of TYP_STRUCT.
     var_types            m_elemType;        // The normalized (TYP_SIMD != TYP_STRUCT) array element type.
     uint8_t              m_firstElemOffset; // Offset to the first element of the array.
+    unsigned             m_elemSize;        // Size of the array element type.
+    unsigned             m_fieldOffset;     // Compound offset of the element's fields (if any).
 
 public:
-    GenTreeArrAddr(GenTree* addr, var_types elemType, CORINFO_CLASS_HANDLE elemClassHandle, uint8_t firstElemOffset)
+    GenTreeArrAddr(GenTree*             addr,
+                   var_types            elemType,
+                   CORINFO_CLASS_HANDLE elemClassHandle,
+                   unsigned             elemSize,
+                   uint8_t              firstElemOffset,
+                   unsigned             fieldOffset)
         : GenTreeUnOp(GT_ARR_ADDR, TYP_BYREF, addr DEBUGARG(/* largeNode */ false))
         , m_elemClassHandle(elemClassHandle)
         , m_elemType(elemType)
         , m_firstElemOffset(firstElemOffset)
+        , m_elemSize(elemSize)
+        , m_fieldOffset(fieldOffset)
     {
         assert(addr->TypeIs(TYP_BYREF));
         assert(((elemType == TYP_STRUCT) && (elemClassHandle != NO_CLASS_HANDLE)) ||
                (elemClassHandle == NO_CLASS_HANDLE));
+        assert((elemSize == genTypeSize(elemType)) || ((elemSize != 0) && (elemType == TYP_STRUCT)));
     }
 
 #if DEBUGGABLE_GENTREE
@@ -6459,11 +6470,26 @@ struct GenTreeArrAddr : GenTreeUnOp
         return m_elemType;
     }
 
+    unsigned GetElemSize() const
+    {
+        return m_elemSize;
+    }
+
     uint8_t GetFirstElemOffset() const
     {
         return m_firstElemOffset;
     }
 
+    unsigned GetFieldOffset() const
+    {
+        return m_fieldOffset;
+    }
+
+    void AddOffset(ssize_t offset)
+    {
+        m_fieldOffset = (m_fieldOffset + static_cast<size_t>(offset)) % m_elemSize;
+    }
+
     void ParseArrayAddress(Compiler* comp, GenTree** pArr, ValueNum* pInxVN);
 
 private:

src/coreclr/jit/morph.cpp

@@ -4686,7 +4686,7 @@ GenTree* Compiler::fgMorphIndexAddr(GenTreeIndexAddr* indexAddr)
     }
 
     // TODO-Throughput: bash the INDEX_ADDR to ARR_ADDR here instead of creating a new node.
-    addr = new (this, GT_ARR_ADDR) GenTreeArrAddr(addr, elemTyp, elemStructType, elemOffs);
+    addr = new (this, GT_ARR_ADDR) GenTreeArrAddr(addr, elemTyp, elemStructType, elemSize, elemOffs, 0);
 
     if (indexAddr->IsNotNull())
     {
@@ -12492,7 +12492,7 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add)
 
     // Fold "((x + icon1) + (y + icon2))" to ((x + y) + (icon1 + icon2))".
     // Be careful not to create a byref pointer that may point outside of the ref object.
-    // Only do this in global morph as we don't recompute the VN for "(x + y)", the new "op2".
+    // Only do this in global morph as we don't recompute the VN for "(x + y)", the new "op1".
     if (op1->OperIs(GT_ADD) && op2->OperIs(GT_ADD) && !op1->gtOverflow() && !op2->gtOverflow() &&
         op1->AsOp()->gtGetOp2()->IsCnsIntOrI() && op2->AsOp()->gtGetOp2()->IsCnsIntOrI() &&
         !varTypeIsGC(op1->AsOp()->gtGetOp1()) && !varTypeIsGC(op2->AsOp()->gtGetOp1()) && fgGlobalMorph)
@@ -12561,6 +12561,25 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add)
             }
         }
 
+        // Sink offset addition below "ARR_ADDR"s to find potentially optimizable commutative folding.
+        if (fgGlobalMorph && op2->IsCnsIntOrI())
+        {
+            GenTree* effectiveOp1 = op1->gtEffectiveVal();
+            if (effectiveOp1->OperIs(GT_ARR_ADDR))
+            {
+                GenTreeArrAddr* addr   = effectiveOp1->AsArrAddr();
+                ssize_t         offset = op2->AsIntCon()->IconValue();
+
+                add->SetAllEffectsFlags(addr);
+                add->gtOp1            = addr->gtOp1;
+                GenTree* optimizedAdd = fgOptimizeCommutativeArithmetic(add);
+                addr->gtOp1           = optimizedAdd;
+                addr->AddOffset(offset);
+
+                return op1;
+            }
+        }
+
         // - a + b = > b - a
         // ADD((NEG(a), b) => SUB(b, a)
 

src/coreclr/jit/optimizer.cpp

@@ -8473,17 +8473,14 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                     }
                     else
                     {
-                        GenTreeArrAddr* arrAddr  = nullptr;
-                        GenTree*        baseAddr = nullptr;
-                        FieldSeq*       fldSeq   = nullptr;
-                        ssize_t         offset   = 0;
+                        GenTree*  baseAddr = nullptr;
+                        FieldSeq* fldSeq   = nullptr;
+                        ssize_t   offset   = 0;
 
-                        if (arg->IsArrayAddr(&arrAddr))
+                        if (arg->OperIs(GT_ARR_ADDR))
                         {
-                            // We will not collect "fldSeq" -- any modification to an S[], at
-                            // any field of "S", will lose all information about the array type.
                             CORINFO_CLASS_HANDLE elemTypeEq =
-                                EncodeElemType(arrAddr->GetElemType(), arrAddr->GetElemClassHandle());
+                                EncodeElemType(arg->AsArrAddr()->GetElemType(), arg->AsArrAddr()->GetElemClassHandle());
                             AddModifiedElemTypeAllContainingLoops(mostNestedLoop, elemTypeEq);
                             // Conservatively assume byrefs may alias this array element
                             memoryHavoc |= memoryKindSet(ByrefExposed);

src/coreclr/jit/valuenum.cpp

@@ -9173,7 +9173,7 @@ void Compiler::fgValueNumberArrIndexAddr(GenTreeArrAddr* arrAddr)
 
     ValueNum arrVN    = vnStore->VNNormalValue(arr->GetVN(VNK_Liberal));
     inxVN             = vnStore->VNNormalValue(inxVN);
-    ValueNum offsetVN = vnStore->VNForIntPtrCon(0);
+    ValueNum offsetVN = vnStore->VNForIntPtrCon(arrAddr->GetFieldOffset());
 
     ValueNum     arrAddrVN  = vnStore->VNForFunc(TYP_BYREF, VNF_PtrToArrElem, elemTypeEqVN, arrVN, inxVN, offsetVN);
     ValueNumPair arrAddrVNP = ValueNumPair(arrAddrVN, arrAddrVN);