Skip to content

JIT: extend indirect virtual stub optimization to arrays #116771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AndyAyersMS
Copy link
Member

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

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 dotnet#108913
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 18, 2025
Copy link
Contributor

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

@AndyAyersMS
Copy link
Member Author

@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
Copy link
Member Author

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant