JIT: extend indirect virtual stub optimization to arrays

[AndyAyersMS]

Add support for array interface methods invoked via indirect virtual stubs. Trim out a layer of temps in the importer. Remember the base method handle and use that for detecting the GetEnumerator pattern and for array interface method devirtualization. Mark the SZGenericArrayEnumerator ctor as aggressively inlined; when inlined we can often prove the enumerator (conditionally) doesn't escape.

Part of #108913

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[AndyAyersMS]

@EgorBot -intel -arm64

using System.Collections.Generic;
using System.Runtime.CompilerServices;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

[GenericTypeArguments(typeof(string))]
[MemoryDiagnoser]
public class A<T>
{
    private IEnumerable<T> _ienumerable;

    [GlobalSetup(Target = nameof(IEnumerable))]
    public void SetupIEnumerable() => _ienumerable = new string[512] as IEnumerable<T>;

    [Benchmark]
    public T IEnumerable() => Get(_ienumerable);

    [MethodImpl(MethodImplOptions.NoInlining)]
    private T Get(IEnumerable<T> collection)
    {
        T result = default;
        foreach (var item in collection)
            result = item;
        return result;
    }

    private IEnumerable<string> _ienumerable_str;

    [GlobalSetup(Target = nameof(IEnumerable_str))]
    public void SetupIEnumerable_str() => _ienumerable_str = new string[512];

    [Benchmark]
    public string IEnumerable_str() => Get_str(_ienumerable_str);

    [MethodImpl(MethodImplOptions.NoInlining)]
    private string Get_str(IEnumerable<string> collection)
    {
        string result = default;
        foreach (var item in collection)
            result = item;
        return result;
    }
}

[AndyAyersMS]

Two failures that I spotted in scanning:

Unexpected NREs

    System.Collections.Tests.OrderedDictionary_Keys_IList_Generic_Tests.IEnumerable_Generic_GetEnumerator_NoExceptionsWhileGetting(count: 0) [FAIL]
      System.NullReferenceException : Object reference not set to an instance of an object.
      Stack Trace:
        /_/src/libraries/Common/src/System/Collections/Generic/EnumerableHelpers.cs(19,0): at System.Collections.Generic.EnumerableHelpers.GetEmptyEnumerator[T]()
        /_/src/libraries/System.Collections/src/System/Collections/Generic/OrderedDictionary.cs(1596,0): at System.Collections.Generic.OrderedDictionary`2.KeyCollection.System.Collections.Generic.IEnumerable<TKey>.GetEnumerator()
        /_/src/libraries/Common/tests/System/Collections/IEnumerable.Generic.Tests.cs(347,0): at System.Collections.Tests.IEnumerable_Generic_Tests`1.IEnumerable_Generic_GetEnumerator_NoExceptionsWhileGetting(Int32 count)
           at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(174,0): at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
    System.Collections.Tests.OrderedDictionary_Keys_IList_Generic_Tests.IEnumerable_Generic_Enumerator_Current_AfterEndOfEnumerable_UndefinedBehavior(count: 0) [FAIL]
      System.NullReferenceException : Object reference not set to an instance of an object.

A runtime assert

Assert failure(PID 6856 [0x00001ac8], Thread: 5192 [0x1448]): Consistency check failed: 
FAILED: (!implSlot.IsNull() || pMT->IsInterface()) && "Valid method implementation was not found."

[AndyAyersMS]

For the assert (here looking at the span Indexer test) we have this tree:

**** Late devirt opportunity
               [000157] --C-G------                         *  CALLV stub ref    System.Collections.Generic.IEnumerable`1[System.__Canon]:GetEnumerator():System.Collections.Generic.IEnumerator`1[System.__Canon]:this
               [000820] --CXG------ this                    \--*  CALL help ref    CORINFO_HELP_CHKCASTANY
               [000819] H------N--- arg0                       +--*  CNS_INT(h) long   0x7ffe0b544f48 class System.Collections.Generic.IEnumerable`1[Span.InlineDataAttribute]
               [000830] --C-G------ arg1                       \--*  CALL      ref    System.Attribute:GetCustomAttributes(System.Reflection.MemberInfo,System.Type,bool):System.Attribute[]
               [000145] ----------- arg0                          +--*  LCL_VAR   ref    V18 tmp5         
               [000827] --C-G------ arg1                          +--*  CALL help ref    CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
               [000828] H---------- arg0                          |  \--*  CNS_INT(h) long   0x7ffe0b5441d0 class Span.InlineDataAttribute
               [000829] ----------- arg2                          \--*  CNS_INT   int    1

We try late devirt, and need the type of this, so we invoke gtGetClassHandle([820]) .. this rejects the cast-to type as we try and avoid returning interface types, and uses the cast base type. So we devirt against Attribute[] instead of InlineDataAttribute[].

The devirt is to an instantiating stub, so we end up producing

               [000157] --C-G------                         *  CALL nullcheck ref    System.SZArrayHelper:GetEnumerator[System.__Canon]():System.Collections.Generic.IEnumerator`1[System.__Canon]:this
               [000820] --CXG------ this                    +--*  CALL help ref    CORINFO_HELP_CHKCASTANY
               [000819] H------N--- arg0                    |  +--*  CNS_INT(h) long   0x7ffe0b544f48 class System.Collections.Generic.IEnumerable`1[Span.InlineDataAttribute]
               [000830] --C-G------ arg1                    |  \--*  CALL      ref    System.Attribute:GetCustomAttributes(System.Reflection.MemberInfo,System.Type,bool):System.Attribute[]
               [000145] ----------- arg0                    |     +--*  LCL_VAR   ref    V18 tmp5         
               [000827] --C-G------ arg1                    |     +--*  CALL help ref    CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE
               [000828] H---------- arg0                    |     |  \--*  CNS_INT(h) long   0x7ffe0b5441d0 class Span.InlineDataAttribute
               [000829] ----------- arg2                    |     \--*  CNS_INT   int    1
               [000832] H---------- gctx                    \--*  CNS_INT(h) long   0x7ffe0b5caa68 method System.SZArrayHelper:GetEnumerator[System.Attribute]():System.Collections.Generic.IEnumerator`1[System.Attribute]:this

and this context is the wrong type. So we do the wrong instantiation...(?) and blow up later trying to invoke MoveNext on this enumerator.

Maybe in array interface cases if the object (array) type is inexact we need to just bail out on devirtualization... let me see how messy that ends up being.

[AndyAyersMS]

@EgorBot -intel -arm64

using System.Collections.Generic;
using System.Runtime.CompilerServices;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

[GenericTypeArguments(typeof(string))]
[MemoryDiagnoser]
public class A<T>
{
    private IEnumerable<T> _ienumerable;

    [GlobalSetup(Target = nameof(IEnumerable))]
    public void SetupIEnumerable() => _ienumerable = new string[512] as IEnumerable<T>;

    [Benchmark]
    public T IEnumerable() => Get(_ienumerable);

    [MethodImpl(MethodImplOptions.NoInlining)]
    private T Get(IEnumerable<T> collection)
    {
        T result = default;
        foreach (var item in collection)
            result = item;
        return result;
    }

    private IEnumerable<string> _ienumerable_str;

    [GlobalSetup(Target = nameof(IEnumerable_str))]
    public void SetupIEnumerable_str() => _ienumerable_str = new string[512];

    [Benchmark]
    public string IEnumerable_str() => Get_str(_ienumerable_str);

    [MethodImpl(MethodImplOptions.NoInlining)]
    private string Get_str(IEnumerable<string> collection)
    {
        string result = default;
        foreach (var item in collection)
            result = item;
        return result;
    }
}

[AndyAyersMS]

@EgorBot --filter "System.Collections.IterateForEach*.Array"

[Copilot]

Pull Request Overview

This PR extends indirect virtual stub optimization to handle array interface methods, reduces temporary spills in the importer, and tracks the original base method handle for GetEnumerator pattern detection. It also marks the SZGenericArrayEnumerator constructor as aggressively inlined.

• Added [MethodImpl(AggressiveInlining)] to the array enumerator constructor.
• Enhanced CEEInfo::resolveVirtualMethodHelper to support array interface devirtualization with exact element types.
• Propagated an originalMethodHandle through JIT inlining and import phases; trimmed a layer of temps in impImportCall.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs	Marked SZGenericArrayEnumerator ctor as aggressively inlined
src/coreclr/vm/jitinterface.cpp	Added array-specific devirtualization path and refined interface/type checks
src/coreclr/jit/inline.h	Introduced originalMethodHandle field in InlineCandidateInfo
src/coreclr/jit/indirectcalltransformer.cpp	Switched to inlineInfo->originalMethodHandle for array calls
src/coreclr/jit/importercalls.cpp	Skipped redundant temp spill for local vars; updated addGuardedDevirtualizationCandidate signature; added exact‐type bailout
src/coreclr/jit/importer.cpp	Used originalMethodHandle for intrinsic lookup in GDV candidates
src/coreclr/jit/fginline.cpp	Added JITDUMP tracing around method-handle resolution
src/coreclr/jit/compiler.h	Updated addGuardedDevirtualizationCandidate declaration to include originalMethodHandle

Comments suppressed due to low confidence (3)

src/coreclr/vm/jitinterface.cpp:8624

• [nitpick] This comment is misleading, since the following HasInstantiation check filters out generic interfaces rather than determining array support. Please update the comment to accurately describe the intent of the check.

            // See if this interface is one of the ones arrays implement.

src/coreclr/jit/indirectcalltransformer.cpp:976

• [nitpick] Consider adding an assertion or null‐check to ensure inlineInfo->originalMethodHandle is valid before using it, to prevent unexpected null dereferences.

                    methodHnd = inlineInfo->originalMethodHandle;

src/coreclr/jit/importer.cpp:6952

• GetGDVCandidateInfo(0) may return null or originalMethodHandle might be uninitialized. Please add a check that info is non-null and that info->originalMethodHandle is set before passing it to lookupNamedIntrinsic.

                            InlineCandidateInfo* const info = call->GetGDVCandidateInfo(0);

[AndyAyersMS]

@davidwrighton @dotnet/jit-contrib PTAL

This mainly benefits PGO.... the case from #108913 (comment) where we're in a shared method and the underlying collection is nominally a shared array.

EgorBot/runtime-utils#401 (comment) shows the improvement in that case.

[AndyAyersMS]

SPMI diffs are not going to give much insight here, though the elimination of one temp (in many cases) in the importer saves some size for minopts/tier0.

I will do another bespoke asp.net SPMI run to get a better idea how many methods this impacts, but I expect it will be fairly small, seeing as there were only 400 or so missed contexts.

[AndyAyersMS]

@EgorBot --filter "System.Collections.IterateForEach*.IEnumerable"

[AndyAyersMS]

@EgorBot --filter "System.Collections.IterateForEach*.IEnumerable"

[AndyAyersMS]

@EgorBot --filter "System.Collections.IterateForEach*.IEnumerable"

[AndyAyersMS]

@EgorBot --filter "System.Collections.IterateForEach*.IEnumerable"

[AndyAyersMS]

@davidwrighton @dotnet/jit-contrib think this is finally in good shape.

[amanasifkhalid]

Few nits, otherwise LGTM