Use a simpler representation for array addresses after morph (#64581)

* Do not copy flags in ADDR/COMMA opt

We only need to update the side effects, and
gtUpdateNodeSideEffects already takes care of that.

Will avoid copying GTF_NO_CSE unnecessarily.

No diffs.

* Add GT_ARR_ADDR

* Generate ARR_ADDR in morph

* gtSetEvalOrder tuning

* Delete the ArrayInfo map

* Delete GTF_IND_ARR_INDEX

* Make ARR_ADDR non-null

This preserves the non-faultness annotation for the parent
indir when ADDR(IND(ARR_ADDR)) -> ARR_ADDR folding happends.

* Simplify the array address parsing

We don't need the field sequence or the complex parsing.

A few diffs because previos code didn't handle COMMAs.
(The new code doesn't either, but they are skipped
automatically by the optComputeLoopSideEffectsOfBlock).

* Clean ParseArrayAddress up

Move it to GenTreeArrAddr, stop using ArrayInfo.

* More ParseArrayAddress cleanup

FldSeq no loger expected or needed. We will be
attaching it to ARR_ADDR directly in the future.

* Delete index labeling

And associated code. No longer needed, and we
will use a different mechanism for ARR_ADDR.

No diffs.

* Delete the last pseudo-field

No longer needed...

* Delete ArrayInfo

* Fix formatting

src/coreclr/jit/assertionprop.cpp

@@ -3225,19 +3225,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc*        curAssertion,
             }
             else
             {
-                bool isArrIndex = ((tree->gtFlags & GTF_VAR_ARR_INDEX) != 0);
-
                 assert(varTypeIsIntegralOrI(tree));
-
                 newTree->BashToConst(curAssertion->op2.u1.iconVal, genActualType(tree));
-
-                // If we're doing an array index address, assume any constant propagated contributes to the index.
-                if (isArrIndex)
-                {
-                    newTree->AsIntCon()->gtFieldSeq =
-                        GetFieldSeqStore()->CreateSingleton(FieldSeqStore::ConstantIndexPseudoField);
-                }
-                newTree->gtFlags &= ~GTF_VAR_ARR_INDEX;
             }
             break;
 

src/coreclr/jit/compiler.cpp

@@ -1916,7 +1916,6 @@ void Compiler::compInit(ArenaAllocator*       pAlloc,
     m_blockToEHPreds     = nullptr;
     m_fieldSeqStore      = nullptr;
     m_zeroOffsetFieldMap = nullptr;
-    m_arrayInfoMap       = nullptr;
     m_refAnyClass        = nullptr;
     for (MemoryKind memoryKind : allMemoryKinds())
     {
@@ -9161,10 +9160,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
                 {
                     chars += printf("[VAR_DEATH]");
                 }
-                if (tree->gtFlags & GTF_VAR_ARR_INDEX)
-                {
-                    chars += printf("[VAR_ARR_INDEX]");
-                }
 #if defined(DEBUG)
                 if (tree->gtDebugFlags & GTF_DEBUG_VAR_CSE_REF)
                 {
@@ -9310,8 +9305,14 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
                 }
                 break;
 
-            case GT_CNS_INT:
+            case GT_ARR_ADDR:
+                if (tree->gtFlags & GTF_ARR_ADDR_NONNULL)
+                {
+                    chars += printf("[ARR_ADDR_NONNULL]");
+                }
+                break;
 
+            case GT_CNS_INT:
             {
                 GenTreeFlags handleKind = (tree->gtFlags & GTF_ICON_HDL_MASK);
 

src/coreclr/jit/compiler.h

@@ -1377,27 +1377,6 @@ class LinearScanInterface
 
 LinearScanInterface* getLinearScanAllocator(Compiler* comp);
 
-// Information about arrays: their element type and size, and the offset of the first element.
-// We label GT_IND's that are array indices with GTF_IND_ARR_INDEX, and, for such nodes,
-// associate an array info via the map retrieved by GetArrayInfoMap().  This information is used,
-// for example, in value numbering of array index expressions.
-struct ArrayInfo
-{
-    var_types            m_elemType;
-    CORINFO_CLASS_HANDLE m_elemStructType;
-    unsigned             m_elemSize;
-    unsigned             m_elemOffset;
-
-    ArrayInfo() : m_elemType(TYP_UNDEF), m_elemStructType(nullptr), m_elemSize(0), m_elemOffset(0)
-    {
-    }
-
-    ArrayInfo(var_types elemType, unsigned elemSize, unsigned elemOffset, CORINFO_CLASS_HANDLE elemStructType)
-        : m_elemType(elemType), m_elemStructType(elemStructType), m_elemSize(elemSize), m_elemOffset(elemOffset)
-    {
-    }
-};
-
 // This enumeration names the phases into which we divide compilation.  The phases should completely
 // partition a compilation.
 enum Phases
@@ -5604,6 +5583,8 @@ class Compiler
     // Does value-numbering for an intrinsic tree.
     void fgValueNumberIntrinsic(GenTree* tree);
 
+    void fgValueNumberArrIndexAddr(GenTreeArrAddr* arrAddr);
+
 #ifdef FEATURE_SIMD
     // Does value-numbering for a GT_SIMD tree
     void fgValueNumberSimd(GenTreeSIMD* tree);
@@ -11220,52 +11201,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
     // CoreRT. Such case is handled same as the default case.
     void fgAddFieldSeqForZeroOffset(GenTree* op1, FieldSeqNode* fieldSeq);
 
-    typedef JitHashTable<const GenTree*, JitPtrKeyFuncs<GenTree>, ArrayInfo> NodeToArrayInfoMap;
-    NodeToArrayInfoMap* m_arrayInfoMap;
-
-    NodeToArrayInfoMap* GetArrayInfoMap()
-    {
-        Compiler* compRoot = impInlineRoot();
-        if (compRoot->m_arrayInfoMap == nullptr)
-        {
-            // Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation.
-            CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap));
-            compRoot->m_arrayInfoMap = new (ialloc) NodeToArrayInfoMap(ialloc);
-        }
-        return compRoot->m_arrayInfoMap;
-    }
-
-    //-----------------------------------------------------------------------------------------------------------------
-    // Compiler::TryGetArrayInfo:
-    //    Given an indirection node, checks to see whether or not that indirection represents an array access, and
-    //    if so returns information about the array.
-    //
-    // Arguments:
-    //    indir           - The `GT_IND` node.
-    //    arrayInfo (out) - Information about the accessed array if this function returns true. Undefined otherwise.
-    //
-    // Returns:
-    //    True if the `GT_IND` node represents an array access; false otherwise.
-    bool TryGetArrayInfo(GenTreeIndir* indir, ArrayInfo* arrayInfo)
-    {
-        if ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0)
-        {
-            return false;
-        }
-
-        if (indir->gtOp1->OperIs(GT_INDEX_ADDR))
-        {
-            GenTreeIndexAddr* const indexAddr = indir->gtOp1->AsIndexAddr();
-            *arrayInfo = ArrayInfo(indexAddr->gtElemType, indexAddr->gtElemSize, indexAddr->gtElemOffset,
-                                   indexAddr->gtStructElemClass);
-            return true;
-        }
-
-        bool found = GetArrayInfoMap()->Lookup(indir, arrayInfo);
-        assert(found);
-        return true;
-    }
-
     NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];
 
     // In some cases, we want to assign intermediate SSA #'s to memory states, and know what nodes create those memory
@@ -11283,8 +11218,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
         Compiler* compRoot = impInlineRoot();
         if (compRoot->m_memorySsaMap[memoryKind] == nullptr)
         {
-            // Create a CompAllocator that labels sub-structure with CMK_ArrayInfoMap, and use that for allocation.
-            CompAllocator ialloc(getAllocator(CMK_ArrayInfoMap));
+            // Create a CompAllocator that labels sub-structure with CMK_MemorySsaMap, and use that for allocation.
+            CompAllocator ialloc(getAllocator(CMK_MemorySsaMap));
             compRoot->m_memorySsaMap[memoryKind] = new (ialloc) NodeToUnsignedMap(ialloc);
         }
         return compRoot->m_memorySsaMap[memoryKind];
@@ -11552,6 +11487,7 @@ class GenTreeVisitor
             case GT_RETURN:
             case GT_RETFILT:
             case GT_RUNTIMELOOKUP:
+            case GT_ARR_ADDR:
             case GT_KEEPALIVE:
             case GT_INC_SATURATE:
             {

src/coreclr/jit/compiler.hpp

@@ -844,28 +844,22 @@ inline GenTree* Compiler::gtNewOperNode(genTreeOps oper, var_types type, GenTree
 
     if (doSimplifications)
     {
-        // We do some simplifications here.
-        // If this gets to be too many, try a switch...
-        // TODO-Cleanup: With the factoring out of array bounds checks, it should not be the
-        // case that we need to check for the array index case here, but without this check
-        // we get failures (see for example jit\Directed\Languages\Python\test_methods_d.exe)
+        // We do some simplifications here. If this gets to be too many, try a switch...
         if (oper == GT_IND)
         {
             // IND(ADDR(IND(x)) == IND(x)
-            if (op1->gtOper == GT_ADDR)
+            if (op1->OperIs(GT_ADDR))
             {
-                GenTreeUnOp* addr  = op1->AsUnOp();
-                GenTree*     indir = addr->gtGetOp1();
-                if (indir->OperIs(GT_IND) && ((indir->gtFlags & GTF_IND_ARR_INDEX) == 0))
+                GenTree* indir = op1->AsUnOp()->gtGetOp1();
+                if (indir->OperIs(GT_IND))
                 {
                     op1 = indir->AsIndir()->Addr();
                 }
             }
         }
         else if (oper == GT_ADDR)
         {
-            // if "x" is not an array index, ADDR(IND(x)) == x
-            if (op1->gtOper == GT_IND && (op1->gtFlags & GTF_IND_ARR_INDEX) == 0)
+            if (op1->OperIs(GT_IND))
             {
                 return op1->AsOp()->gtOp1;
             }
@@ -4231,6 +4225,7 @@ void GenTree::VisitOperands(TVisitor visitor)
         case GT_ALLOCOBJ:
         case GT_INIT_VAL:
         case GT_RUNTIMELOOKUP:
+        case GT_ARR_ADDR:
         case GT_JTRUE:
         case GT_SWITCH:
         case GT_NULLCHECK:

src/coreclr/jit/compmemkind.h

@@ -36,7 +36,7 @@ CompMemKindMacro(Generic)
 CompMemKindMacro(LocalAddressVisitor)
 CompMemKindMacro(FieldSeqStore)
 CompMemKindMacro(ZeroOffsetFieldMap)
-CompMemKindMacro(ArrayInfoMap)
+CompMemKindMacro(MemorySsaMap)
 CompMemKindMacro(MemoryPhiArg)
 CompMemKindMacro(CSE)
 CompMemKindMacro(GC)

src/coreclr/jit/earlyprop.cpp

@@ -344,16 +344,6 @@ GenTree* Compiler::optEarlyPropRewriteTree(GenTree* tree, LocalNumberToNullCheck
             actualValClone->gtType = tree->gtType;
         }
 
-        // Propagating a constant into an array index expression requires calling
-        // LabelIndex to update the FieldSeq annotations.  EarlyProp may replace
-        // array length expressions with constants, so check if this is an array
-        // length operator that is part of an array index expression.
-        bool isIndexExpr = (tree->OperGet() == GT_ARR_LENGTH && ((tree->gtFlags & GTF_ARRLEN_ARR_IDX) != 0));
-        if (isIndexExpr)
-        {
-            actualValClone->LabelIndex(this);
-        }
-
         // actualValClone has small tree node size, it is safe to use CopyFrom here.
         tree->ReplaceWith(actualValClone, this);
 

src/coreclr/jit/fgdiagnostic.cpp

@@ -2951,8 +2951,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree)
         expectedFlags |= GTF_CALL;
     }
 
-    // We reuse GTF_REVERSE_OPS as GTF_VAR_ARR_INDEX for LCL_VAR nodes.
-    if (((tree->gtFlags & GTF_REVERSE_OPS) != 0) && !tree->OperIs(GT_LCL_VAR))
+    if ((tree->gtFlags & GTF_REVERSE_OPS) != 0)
     {
         assert(tree->OperSupportsReverseOpEvalOrder(this));
     }

src/coreclr/jit/flowgraph.cpp

@@ -948,6 +948,10 @@ bool Compiler::fgAddrCouldBeNull(GenTree* addr)
     {
         return false;
     }
+    else if (addr->OperIs(GT_ARR_ADDR))
+    {
+        return (addr->gtFlags & GTF_ARR_ADDR_NONNULL) == 0;
+    }
     else if (addr->gtOper == GT_LCL_VAR)
     {
         unsigned varNum = addr->AsLclVarCommon()->GetLclNum();