JIT: optimize more array covariant store checks in the importer (#189)

The importer was already optimizing away some array covariant store checks,
for cases where the value being stored was null, or the value being stored
came from the same array.

Change this to only optimize array covariant store checks in the importer
when optimization is enabled. For minopts, invoking the store helper produces
smaller code.

Update `gtGetClassHandle` to obtain the array handle from array newobjs,
and use this to also optimize cases where the destination array is exactly
`object[]` or is `T[]` where `T` is final and not itself subject to special
casting logic. In particular this gets the common case where `T` is `string`.

Closes dotnet/coreclr#6537.

src/coreclr/src/jit/compiler.h

@@ -4129,6 +4129,9 @@ private:
     bool impIsImplicitTailCallCandidate(
         OPCODE curOpcode, const BYTE* codeAddrOfNextOpcode, const BYTE* codeEnd, int prefixFlags, bool isRecursive);
 
+    bool impIsClassExact(CORINFO_CLASS_HANDLE classHnd);
+    bool impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array);
+
     CORINFO_RESOLVED_TOKEN* impAllocateToken(const CORINFO_RESOLVED_TOKEN& token);
 
     /*

src/coreclr/src/jit/gentree.cpp

@@ -17290,6 +17290,24 @@ CORINFO_CLASS_HANDLE Compiler::gtGetHelperCallClassHandle(GenTreeCall* call, boo
             break;
         }
 
+        case CORINFO_HELP_NEWARR_1_DIRECT:
+        case CORINFO_HELP_NEWARR_1_OBJ:
+        case CORINFO_HELP_NEWARR_1_VC:
+        case CORINFO_HELP_NEWARR_1_ALIGN8:
+        case CORINFO_HELP_NEWARR_1_R2R_DIRECT:
+        case CORINFO_HELP_READYTORUN_NEWARR_1:
+        {
+            CORINFO_CLASS_HANDLE arrayHnd = (CORINFO_CLASS_HANDLE)call->compileTimeHelperArgumentHandle;
+
+            if (arrayHnd != NO_CLASS_HANDLE)
+            {
+                objClass    = arrayHnd;
+                *pIsExact   = true;
+                *pIsNonNull = true;
+            }
+            break;
+        }
+
         default:
             break;
     }

src/coreclr/src/jit/importer.cpp

@@ -10239,10 +10239,7 @@ GenTree* Compiler::impOptimizeCastClassOrIsInst(GenTree* op1, CORINFO_RESOLVED_T
             // See if we can sharpen exactness by looking for final classes
             if (!isExact)
             {
-                DWORD flags     = info.compCompHnd->getClassAttribs(fromClass);
-                DWORD flagsMask = CORINFO_FLG_FINAL | CORINFO_FLG_MARSHAL_BYREF | CORINFO_FLG_CONTEXTFUL |
-                                  CORINFO_FLG_VARIANCE | CORINFO_FLG_ARRAY;
-                isExact = ((flags & flagsMask) == CORINFO_FLG_FINAL);
+                isExact = impIsClassExact(fromClass);
             }
 
             // Cast to exact type will fail. Handle case where we have
@@ -10344,14 +10341,8 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree*                op1,
         {
             if (helper == CORINFO_HELP_ISINSTANCEOFCLASS)
             {
-                // Check the class attributes.
-                DWORD flags = info.compCompHnd->getClassAttribs(pResolvedToken->hClass);
-
-                // If the class is final and is not marshal byref, variant or
-                // contextful, the jit can expand the IsInst check inline.
-                DWORD flagsMask =
-                    CORINFO_FLG_FINAL | CORINFO_FLG_MARSHAL_BYREF | CORINFO_FLG_VARIANCE | CORINFO_FLG_CONTEXTFUL;
-                canExpandInline = ((flags & flagsMask) == CORINFO_FLG_FINAL);
+                // If the class is exact, the jit can expand the IsInst check inline.
+                canExpandInline = impIsClassExact(pResolvedToken->hClass);
             }
         }
     }
@@ -10520,12 +10511,8 @@ void Compiler::impImportBlockCode(BasicBlock* block)
     }
 #endif
 
-    unsigned  nxtStmtIndex = impInitBlockLineInfo();
-    IL_OFFSET nxtStmtOffs;
-
-    GenTree*                     arrayNodeFrom;
-    GenTree*                     arrayNodeTo;
-    GenTree*                     arrayNodeToIndex;
+    unsigned                     nxtStmtIndex = impInitBlockLineInfo();
+    IL_OFFSET                    nxtStmtOffs;
     CorInfoHelpFunc              helper;
     CorInfoIsAccessAllowedResult accessAllowedResult;
     CORINFO_HELPER_DESC          calloutHelper;
@@ -11821,45 +11808,21 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
             STELEM_REF_POST_VERIFY:
 
-                arrayNodeTo      = impStackTop(2).val;
-                arrayNodeToIndex = impStackTop(1).val;
-                arrayNodeFrom    = impStackTop().val;
-
-                //
-                // Note that it is not legal to optimize away CORINFO_HELP_ARRADDR_ST in a
-                // lot of cases because of covariance. ie. foo[] can be cast to object[].
-                //
-
-                // Check for assignment to same array, ie. arrLcl[i] = arrLcl[j]
-                // This does not need CORINFO_HELP_ARRADDR_ST
-                if (arrayNodeFrom->OperGet() == GT_INDEX && arrayNodeTo->gtOper == GT_LCL_VAR)
+                if (opts.OptimizationEnabled())
                 {
-                    GenTree* indexFromOp1 = arrayNodeFrom->AsIndex()->Arr();
-                    if (indexFromOp1->OperGet() == GT_LCL_VAR)
+                    GenTree* array = impStackTop(2).val;
+                    GenTree* value = impStackTop().val;
+
+                    // Is this a case where we can skip the covariant store check?
+                    if (impCanSkipCovariantStoreCheck(value, array))
                     {
-                        unsigned indexFrom = indexFromOp1->AsLclVar()->GetLclNum();
-                        unsigned indexTo   = arrayNodeTo->AsLclVar()->GetLclNum();
-                        if (indexFrom == indexTo && !lvaGetDesc(indexTo)->lvAddrExposed)
-                        {
-                            JITDUMP("\nstelem of ref from same array: skipping covariant store check\n");
-                            lclTyp = TYP_REF;
-                            goto ARR_ST_POST_VERIFY;
-                        }
+                        lclTyp = TYP_REF;
+                        goto ARR_ST_POST_VERIFY;
                     }
                 }
 
-                // Check for assignment of NULL. This does not need CORINFO_HELP_ARRADDR_ST
-                if (arrayNodeFrom->OperGet() == GT_CNS_INT)
-                {
-                    JITDUMP("\nstelem of null: skipping covariant store check\n");
-                    assert(arrayNodeFrom->gtType == TYP_REF && arrayNodeFrom->AsIntCon()->gtIconVal == 0);
-                    lclTyp = TYP_REF;
-                    goto ARR_ST_POST_VERIFY;
-                }
-
-                /* Call a helper function to do the assignment */
+                // Else call a helper function to do the assignment
                 op1 = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST, TYP_VOID, impPopCallArgs(3, nullptr));
-
                 goto SPILL_APPEND;
 
             case CEE_STELEM_I1:
@@ -20711,3 +20674,122 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall*          call,
 
     call->gtGuardedDevirtualizationCandidateInfo = pInfo;
 }
+
+//------------------------------------------------------------------------
+// impIsClassExact: check if a class handle can only describe values
+//    of exactly one class.
+//
+// Arguments:
+//    classHnd - handle for class in question
+//
+// Returns:
+//    true if class is final and not subject to special casting from
+//    variance or similar.
+//
+// Note:
+//    We are conservative on arrays here. It might be worth checking
+//    for types like int[].
+
+bool Compiler::impIsClassExact(CORINFO_CLASS_HANDLE classHnd)
+{
+    DWORD flags     = info.compCompHnd->getClassAttribs(classHnd);
+    DWORD flagsMask = CORINFO_FLG_FINAL | CORINFO_FLG_MARSHAL_BYREF | CORINFO_FLG_CONTEXTFUL | CORINFO_FLG_VARIANCE |
+                      CORINFO_FLG_ARRAY;
+
+    return ((flags & flagsMask) == CORINFO_FLG_FINAL);
+}
+
+//------------------------------------------------------------------------
+// impCanSkipCovariantStoreCheck: see if storing a ref type value to an array
+//    can skip the array store covariance check.
+//
+// Arguments:
+//    value -- tree producing the value to store
+//    array -- tree representing the array to store to
+//
+// Returns:
+//    true if the store does not require a covariance check.
+//
+bool Compiler::impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array)
+{
+    // We should only call this when optimizing.
+    assert(opts.OptimizationEnabled());
+
+    // Check for assignment to same array, ie. arrLcl[i] = arrLcl[j]
+    if (value->OperIs(GT_INDEX) && array->OperIs(GT_LCL_VAR))
+    {
+        GenTree* valueIndex = value->AsIndex()->Arr();
+        if (valueIndex->OperIs(GT_LCL_VAR))
+        {
+            unsigned valueLcl = valueIndex->AsLclVar()->GetLclNum();
+            unsigned arrayLcl = array->AsLclVar()->GetLclNum();
+            if ((valueLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->lvAddrExposed)
+            {
+                JITDUMP("\nstelem of ref from same array: skipping covariant store check\n");
+                return true;
+            }
+        }
+    }
+
+    // Check for assignment of NULL.
+    if (value->OperIs(GT_CNS_INT))
+    {
+        JITDUMP("\nstelem of null: skipping covariant store check\n");
+        assert((value->gtType == TYP_REF) && (value->AsIntCon()->gtIconVal == 0));
+        return true;
+    }
+
+    // Try and get a class handle for the array
+    if (value->gtType != TYP_REF)
+    {
+        return false;
+    }
+
+    bool                 arrayIsExact   = false;
+    bool                 arrayIsNonNull = false;
+    CORINFO_CLASS_HANDLE arrayHandle    = gtGetClassHandle(array, &arrayIsExact, &arrayIsNonNull);
+
+    if (arrayHandle == NO_CLASS_HANDLE)
+    {
+        return false;
+    }
+
+    // There are some methods in corelib where we're storing to an array but the IL
+    // doesn't reflect this (see SZArrayHelper). Avoid.
+    DWORD attribs = info.compCompHnd->getClassAttribs(arrayHandle);
+    if ((attribs & CORINFO_FLG_ARRAY) == 0)
+    {
+        return false;
+    }
+
+    CORINFO_CLASS_HANDLE arrayElementHandle = nullptr;
+    CorInfoType          arrayElemType      = info.compCompHnd->getChildType(arrayHandle, &arrayElementHandle);
+
+    // Verify array type handle is really an array of ref type
+    assert(arrayElemType == CORINFO_TYPE_CLASS);
+
+    // Check for exactly object[]
+    if (arrayIsExact && (arrayElementHandle == impGetObjectClass()))
+    {
+        JITDUMP("\nstelem to (exact) object[]: skipping covariant store check\n");
+        return true;
+    }
+
+    // Check for T[] with T exact.
+    if (!impIsClassExact(arrayElementHandle))
+    {
+        return false;
+    }
+
+    bool                 valueIsExact   = false;
+    bool                 valueIsNonNull = false;
+    CORINFO_CLASS_HANDLE valueHandle    = gtGetClassHandle(value, &valueIsExact, &valueIsNonNull);
+
+    if (valueHandle == arrayElementHandle)
+    {
+        JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n");
+        return true;
+    }
+
+    return false;
+}

src/coreclr/src/jit/morph.cpp

@@ -8332,7 +8332,8 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
 
     // Morph stelem.ref helper call to store a null value, into a store into an array without the helper.
     // This needs to be done after the arguments are morphed to ensure constant propagation has already taken place.
-    if ((call->gtCallType == CT_HELPER) && (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST)))
+    if (opts.OptimizationEnabled() && (call->gtCallType == CT_HELPER) &&
+        (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST)))
     {
         GenTree* value = gtArgEntryByArgNum(call, 2)->GetNode();
         if (value->IsIntegralConst(0))