Jit intrinsics for Span<T>.get_Item and ReadOnlySpan<T>.get_Item. (#10910)

Implement these two methods as optional-expand jit intrinsics.
Uses `GT_ARR_BOUNDS_CHECK` for the bounds check so in some cases
downstream code is able to eliminate redundant checks. Fully general
support (on par with arrays in most cases) is still work in progress.

Update one bit of code in the optimizer that assumed it knew the
tree types that appeared in a `GT_ARR_BOUNDS_CHECK`.

Add benchmark tests for Span and ReadOnlySpan indexers.

Tests ability of jit to reason about indexer properties with respect
to loop bounds and related indexer uses. Some cases inspired by span
indexer usage in Kestrel.

Closes #10785.

Also addresses lack of indexer inlining noted in #10031. Span indexers
should now always be inlined, even when invoked from shared methods.

6 files changed, 114 insertions, 6 deletions

diff --git a/src/inc/corinfo.h b/src/inc/corinfo.h
index cbc4464e1d..97f395800e 100644
--- a/src/inc/corinfo.h
+++ b/src/inc/corinfo.h
@@ -213,7 +213,6 @@ TODO: Talk about initializing strutures before use
     #define SELECTANY extern __declspec(selectany)
 #endif
 
-// Update this one
 SELECTANY const GUID JITEEVersionIdentifier = { /* f00b3f49-ddd2-49be-ba43-6e49ffa66959 */
     0xf00b3f49,
     0xddd2,
@@ -959,6 +958,8 @@ enum CorInfoIntrinsics
     CORINFO_INTRINSIC_GetManagedThreadId,
     CORINFO_INTRINSIC_ByReference_Ctor,
     CORINFO_INTRINSIC_ByReference_Value,
+    CORINFO_INTRINSIC_Span_GetItem,
+    CORINFO_INTRINSIC_ReadOnlySpan_GetItem,
 
     CORINFO_INTRINSIC_Count,
     CORINFO_INTRINSIC_Illegal = -1,         // Not a true intrinsic,
diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 989130a247..a07c55c62d 100644
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -3620,6 +3620,87 @@ GenTreePtr Compiler::impIntrinsic(GenTreePtr            newobjThis,
             retNode                     = field;
             break;
         }
+        case CORINFO_INTRINSIC_Span_GetItem:
+        case CORINFO_INTRINSIC_ReadOnlySpan_GetItem:
+        {
+            // Have index, stack pointer-to Span<T> s on the stack. Expand to:
+            //
+            // For Span<T>
+            //   Comma
+            //     BoundsCheck(index, s->_length)
+            //     s->_pointer + index * sizeof(T)
+            //
+            // For ReadOnlySpan<T>
+            //   Comma
+            //     BoundsCheck(index, s->_length)
+            //     *(s->_pointer + index * sizeof(T))
+            //
+            // Signature should show one class type parameter, which
+            // we need to examine.
+            assert(sig->sigInst.classInstCount == 1);
+            CORINFO_CLASS_HANDLE spanElemHnd = sig->sigInst.classInst[0];
+            const unsigned       elemSize    = info.compCompHnd->getClassSize(spanElemHnd);
+            assert(elemSize > 0);
+
+            const bool isReadOnly = (intrinsicID == CORINFO_INTRINSIC_ReadOnlySpan_GetItem);
+
+            JITDUMP("\nimpIntrinsic: Expanding %sSpan<T>.get_Item, T=%s, sizeof(T)=%u\n", isReadOnly ? "ReadOnly" : "",
+                    info.compCompHnd->getClassName(spanElemHnd), elemSize);
+
+            GenTreePtr index          = impPopStack().val;
+            GenTreePtr ptrToSpan      = impPopStack().val;
+            GenTreePtr indexClone     = nullptr;
+            GenTreePtr ptrToSpanClone = nullptr;
+
+#if defined(DEBUG)
+            if (verbose)
+            {
+                printf("with ptr-to-span\n");
+                gtDispTree(ptrToSpan);
+                printf("and index\n");
+                gtDispTree(index);
+            }
+#endif // defined(DEBUG)
+
+            // We need to use both index and ptr-to-span twice, so clone or spill.
+            index = impCloneExpr(index, &indexClone, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL,
+                                 nullptr DEBUGARG("Span.get_Item index"));
+            ptrToSpan = impCloneExpr(ptrToSpan, &ptrToSpanClone, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL,
+                                     nullptr DEBUGARG("Span.get_Item ptrToSpan"));
+
+            // Bounds check
+            CORINFO_FIELD_HANDLE lengthHnd    = info.compCompHnd->getFieldInClass(clsHnd, 1);
+            const unsigned       lengthOffset = info.compCompHnd->getFieldOffset(lengthHnd);
+            GenTreePtr           length       = gtNewFieldRef(TYP_INT, lengthHnd, ptrToSpan, lengthOffset, false);
+            GenTreePtr           boundsCheck  = new (this, GT_ARR_BOUNDS_CHECK)
+                GenTreeBoundsChk(GT_ARR_BOUNDS_CHECK, TYP_VOID, index, length, SCK_RNGCHK_FAIL);
+
+            // Element access
+            GenTreePtr           indexIntPtr = impImplicitIorI4Cast(indexClone, TYP_I_IMPL);
+            GenTreePtr           sizeofNode  = gtNewIconNode(elemSize);
+            GenTreePtr           mulNode     = gtNewOperNode(GT_MUL, TYP_I_IMPL, indexIntPtr, sizeofNode);
+            CORINFO_FIELD_HANDLE ptrHnd      = info.compCompHnd->getFieldInClass(clsHnd, 0);
+            const unsigned       ptrOffset   = info.compCompHnd->getFieldOffset(ptrHnd);
+            GenTreePtr           data        = gtNewFieldRef(TYP_BYREF, ptrHnd, ptrToSpanClone, ptrOffset, false);
+            GenTreePtr           result      = gtNewOperNode(GT_ADD, TYP_BYREF, data, mulNode);
+
+            // Prepare result
+            var_types resultType = JITtype2varType(sig->retType);
+
+            if (isReadOnly)
+            {
+                result = gtNewOperNode(GT_IND, resultType, result);
+            }
+            else
+            {
+                assert(resultType == result->TypeGet());
+            }
+
+            retNode = gtNewOperNode(GT_COMMA, resultType, boundsCheck, result);
+
+            break;
+        }
+
         default:
             /* Unknown intrinsic */
             break;
diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp
index c18ebc55d0..710dac540c 100644
--- a/src/jit/optimizer.cpp
+++ b/src/jit/optimizer.cpp
@@ -7638,6 +7638,15 @@ bool Compiler::optExtractArrIndex(GenTreePtr tree, ArrIndex* result, unsigned lh
     {
         return false;
     }
+
+    // For span we may see gtArrLen is a local var or local field.
+    // We won't try and extract those.
+    const genTreeOps arrayOp = arrBndsChk->gtArrLen->gtOper;
+
+    if ((arrayOp == GT_LCL_VAR) || (arrayOp == GT_LCL_FLD))
+    {
+        return false;
+    }
     if (arrBndsChk->gtArrLen->gtGetOp1()->gtOper != GT_LCL_VAR)
     {
         return false;
diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp
index a82cf00448..5ef7700896 100644
--- a/src/vm/jitinterface.cpp
+++ b/src/vm/jitinterface.cpp
@@ -8623,7 +8623,7 @@ CorInfoIntrinsics CEEInfo::getIntrinsicID(CORINFO_METHOD_HANDLE methodHnd,
     else
     {
         MethodTable * pMT = method->GetMethodTable();
-        if (pMT->IsByRefLike() && pMT->GetModule()->IsSystem())
+        if (pMT->GetModule()->IsSystem() && pMT->IsByRefLike())
         {
             if (pMT->HasSameTypeDefAs(g_pByReferenceClass))
             {
@@ -8637,10 +8637,25 @@ CorInfoIntrinsics CEEInfo::getIntrinsicID(CORINFO_METHOD_HANDLE methodHnd,
                     _ASSERTE(strcmp(method->GetName(), "get_Value") == 0);
                     result = CORINFO_INTRINSIC_ByReference_Value;
                 }
-                *pMustExpand = true;
+                if (pMustExpand != nullptr)
+                {
+                    *pMustExpand = true;
+                }
+            }
+            else if (pMT->HasSameTypeDefAs(MscorlibBinder::GetClass(CLASS__SPAN)))
+            {
+                if (method->HasSameMethodDefAs(MscorlibBinder::GetMethod(METHOD__SPAN__GET_ITEM)))
+                {
+                    result = CORINFO_INTRINSIC_Span_GetItem;
+                }
+            }
+            else if (pMT->HasSameTypeDefAs(MscorlibBinder::GetClass(CLASS__READONLY_SPAN)))
+            {
+                if (method->HasSameMethodDefAs(MscorlibBinder::GetMethod(METHOD__READONLY_SPAN__GET_ITEM)))
+                {
+                    result = CORINFO_INTRINSIC_ReadOnlySpan_GetItem;
+                }
             }
-
-            // TODO-SPAN: Span<T> intrinsics for optimizations
         }
     }
 
diff --git a/src/vm/method.cpp b/src/vm/method.cpp
index a72b07b404..77a6a0d37f 100644
--- a/src/vm/method.cpp
+++ b/src/vm/method.cpp
@@ -2404,7 +2404,7 @@ BOOL MethodDesc::IsFCallOrIntrinsic()
     if (IsFCall() || IsArray())
         return TRUE;
 
-    // Intrinsic methods on ByReference<T> or Span<T>
+    // Intrinsic methods on ByReference<T>, Span<T>, or ReadOnlySpan<T>
     MethodTable * pMT = GetMethodTable();
     if (pMT->IsByRefLike() && pMT->GetModule()->IsSystem())
         return TRUE;
diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index 4b82de0a2c..87927f687d 100644
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -653,7 +653,9 @@ DEFINE_CLASS(NULLABLE,              System,                 Nullable`1)
 
 DEFINE_CLASS(BYREFERENCE,           System,                 ByReference`1)
 DEFINE_CLASS(SPAN,                  System,                 Span`1)
+DEFINE_METHOD(SPAN,                 GET_ITEM,               get_Item, NoSig)
 DEFINE_CLASS(READONLY_SPAN,         System,                 ReadOnlySpan`1)
+DEFINE_METHOD(READONLY_SPAN,        GET_ITEM,               get_Item, NoSig)
 
 // Keep this in sync with System.Globalization.NumberFormatInfo
 DEFINE_CLASS_U(Globalization,       NumberFormatInfo,   NumberFormatInfo)