JIT: remove GTF_INX_REFARR_LAYOUT (dotnet#33098)

When morphing `GT_INDEX` nodes, we were inadvertently also setting
`GTF_IND_NONFAULTING` for the `GT_IND` subtree for ref type arrays, because
`GTF_IND_NONFAULTING` has the same value as `GTF_INX_REFARR_LAYOUT`.

This turns out to be safe since in general there is an upstream bounds check to
cover the null check from the indexing operation, so the fact that we were
claiming the `GT_IND` can't fault is ok.

A no diff change would remove the `GTF_INX_REFARR_LAYOUT` flag and then modify
`fgMorphArrayIndex` to set `GTF_IND_NONFAULTING` for ref type arrays with bounds
checks:
```
    // If there's a bounds check, the the indir won't fault.
    if (bndsChk && (tree->gtType == TYP_REF))
    {
        tree->gtFlags |= GTF_IND_NONFAULTING;
    }

    tree->gtFlags |= GTF_EXCEPT;
```

But there's no good reason to limit the above change to ref type arrays and no
good reason to OR in `GTF_EXCEPT` when there are bounds checks.

Closes dotnet#32647.

src/coreclr/src/jit/compiler.cpp

@@ -8653,10 +8653,6 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
 
             case GT_INDEX:
 
-                if (tree->gtFlags & GTF_INX_REFARR_LAYOUT)
-                {
-                    chars += printf("[INX_REFARR_LAYOUT]");
-                }
                 if (tree->gtFlags & GTF_INX_STRING_LAYOUT)
                 {
                     chars += printf("[INX_STRING_LAYOUT]");

src/coreclr/src/jit/gentree.cpp

@@ -9778,18 +9778,6 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, __in __in_z _
 
             case GT_INDEX:
             case GT_INDEX_ADDR:
-
-                if ((tree->gtFlags & (GTF_IND_VOLATILE | GTF_IND_UNALIGNED)) == 0) // We prefer printing V or U over R
-                {
-                    if (tree->gtFlags & GTF_INX_REFARR_LAYOUT)
-                    {
-                        printf("R");
-                        --msgLength;
-                        break;
-                    } // R means RefArray
-                }
-                __fallthrough;
-
             case GT_FIELD:
             case GT_CLS_VAR:
                 if (tree->gtFlags & GTF_IND_VOLATILE)

src/coreclr/src/jit/gentree.h

@@ -806,7 +806,6 @@ struct GenTree
 #define GTF_FLD_INITCLASS           0x20000000 // GT_FIELD/GT_CLS_VAR -- field access requires preceding class/static init helper
 
 #define GTF_INX_RNGCHK              0x80000000 // GT_INDEX/GT_INDEX_ADDR -- the array reference should be range-checked.
-#define GTF_INX_REFARR_LAYOUT       0x20000000 // GT_INDEX -- TODO: Delete, no longer necessary (https://github.com/dotnet/runtime/issues/32647)
 #define GTF_INX_STRING_LAYOUT       0x40000000 // GT_INDEX -- this uses the special string array layout
 
 #define GTF_IND_TGT_NOT_HEAP        0x80000000 // GT_IND   -- the target is not on the heap
@@ -4635,11 +4634,6 @@ struct GenTreeIndex : public GenTreeOp
             gtFlags |= GTF_INX_RNGCHK;
         }
 
-        if (type == TYP_REF)
-        {
-            gtFlags |= GTF_INX_REFARR_LAYOUT;
-        }
-
         gtFlags |= GTF_EXCEPT | GTF_GLOB_REF;
     }
 #if DEBUGGABLE_GENTREE

src/coreclr/src/jit/importer.cpp

@@ -9100,14 +9100,11 @@ GenTree* Compiler::impFixupStructReturnType(GenTree* op, CORINFO_CLASS_HANDLE re
     {
         GenTree* op1 = op->AsObj()->Addr();
 
-        // We will fold away OBJ/ADDR
-        // except for OBJ/ADDR/INDEX
-        //     as the array type influences the array element's offset
-        //     Later in this method we change op->gtType to info.compRetNativeType
-        //     This is not correct when op is a GT_INDEX as the starting offset
-        //     for the array elements 'elemOffs' is different for an array of
-        //     TYP_REF than an array of TYP_STRUCT (which simply wraps a TYP_REF)
-        //     Also refer to the GTF_INX_REFARR_LAYOUT flag
+        // We will fold away OBJ/ADDR, except for OBJ/ADDR/INDEX
+        //
+        // In the latter case the OBJ type may have a different type
+        // than the array element type, and we need to preserve the
+        // array element type for now.
         //
         if ((op1->gtOper == GT_ADDR) && (op1->AsOp()->gtOp1->gtOper != GT_INDEX))
         {

src/coreclr/src/jit/morph.cpp

@@ -5314,8 +5314,8 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree)
     }
 #endif // FEATURE_SIMD
 
-    // Set up the the array length's offset into lenOffs
-    // And    the the first element's offset into elemOffs
+    // Set up the array length's offset into lenOffs
+    // And    the first element's offset into elemOffs
     ssize_t lenOffs;
     ssize_t elemOffs;
     if (tree->gtFlags & GTF_INX_STRING_LAYOUT)
@@ -5552,8 +5552,15 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree)
     // This is an array index expression.
     tree->gtFlags |= GTF_IND_ARR_INDEX;
 
-    /* An indirection will cause a GPF if the address is null */
-    tree->gtFlags |= GTF_EXCEPT;
+    // If there's a bounds check, the indir won't fault.
+    if (bndsChk)
+    {
+        tree->gtFlags |= GTF_IND_NONFAULTING;
+    }
+    else
+    {
+        tree->gtFlags |= GTF_EXCEPT;
+    }
 
     if (nCSE)
     {
@@ -9210,7 +9217,7 @@ GenTree* Compiler::fgMorphPromoteLocalInitBlock(GenTreeLclVar* destLclNode, GenT
         unsigned   fieldLclNum = destLclVar->lvFieldLclStart + i;
         LclVarDsc* fieldDesc   = lvaGetDesc(fieldLclNum);
         GenTree*   dest        = gtNewLclvNode(fieldLclNum, fieldDesc->TypeGet());
-        // If it had been labeled a "USEASG", assignments to the the individual promoted fields are not.
+        // If it had been labeled a "USEASG", assignments to the individual promoted fields are not.
         dest->gtFlags |= (destLclNode->gtFlags & ~(GTF_NODE_MASK | GTF_VAR_USEASG));
 
         GenTree* src;
@@ -10210,7 +10217,7 @@ GenTree* Compiler::fgMorphCopyBlock(GenTree* tree)
                 noway_assert(destLclNum != BAD_VAR_NUM);
                 unsigned dstFieldLclNum = lvaTable[destLclNum].lvFieldLclStart + i;
                 dstFld                  = gtNewLclvNode(dstFieldLclNum, lvaTable[dstFieldLclNum].TypeGet());
-                // If it had been labeled a "USEASG", assignments to the the individual promoted fields are not.
+                // If it had been labeled a "USEASG", assignments to the individual promoted fields are not.
                 if (destAddr != nullptr)
                 {
                     noway_assert(destAddr->AsOp()->gtOp1->gtOper == GT_LCL_VAR);
@@ -13120,7 +13127,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
                 // all the comma nodes the type of op1.
                 // TODO: the comma flag update below is conservative and can be improved.
                 // For example, if we made the ADDR(IND(x)) == x transformation, we may be able to
-                // get rid of some of the the IND flags on the COMMA nodes (e.g., GTF_GLOB_REF).
+                // get rid of some of the IND flags on the COMMA nodes (e.g., GTF_GLOB_REF).
 
                 while (!commas.Empty())
                 {
@@ -17212,6 +17219,13 @@ void Compiler::fgAddFieldSeqForZeroOffset(GenTree* addr, FieldSeqNode* fieldSeqZ
     // We expect 'addr' to be an address at this point.
     assert(addr->TypeGet() == TYP_BYREF || addr->TypeGet() == TYP_I_IMPL || addr->TypeGet() == TYP_REF);
 
+    // Tunnel through any commas.
+    const bool commaOnly = true;
+    addr                 = addr->gtEffectiveVal(commaOnly);
+
+    // We still expect 'addr' to be an address at this point.
+    assert(addr->TypeGet() == TYP_BYREF || addr->TypeGet() == TYP_I_IMPL || addr->TypeGet() == TYP_REF);
+
     FieldSeqNode* fieldSeqUpdate   = fieldSeqZero;
     GenTree*      fieldSeqNode     = addr;
     bool          fieldSeqRecorded = false;

src/coreclr/src/jit/optcse.cpp

@@ -2773,7 +2773,9 @@ class CSE_Heuristic
             GenTree*      cse = nullptr;
             bool          isDef;
             FieldSeqNode* fldSeq               = nullptr;
-            bool          hasZeroMapAnnotation = m_pCompiler->GetZeroOffsetFieldMap()->Lookup(exp, &fldSeq);
+            bool          commaOnly            = true;
+            GenTree*      effectiveExp         = exp->gtEffectiveVal(commaOnly);
+            const bool    hasZeroMapAnnotation = m_pCompiler->GetZeroOffsetFieldMap()->Lookup(effectiveExp, &fldSeq);
 
             if (IS_CSE_USE(exp->gtCSEnum))
             {
@@ -2984,12 +2986,6 @@ class CSE_Heuristic
                 // Assign the ssa num for the ref use. Note it may be the reserved num.
                 ref->AsLclVarCommon()->SetSsaNum(cseSsaNum);
 
-                // If it has a zero-offset field seq, copy annotation to the ref
-                if (hasZeroMapAnnotation)
-                {
-                    m_pCompiler->fgAddFieldSeqForZeroOffset(ref, fldSeq);
-                }
-
                 /* Create a comma node for the CSE assignment */
                 cse           = m_pCompiler->gtNewOperNode(GT_COMMA, expTyp, origAsg, ref);
                 cse->gtVNPair = ref->gtVNPair; // The comma's value is the same as 'val'