JIT: switch escape analysis flagging to look for calls returning `IEnumerator<T>` #116978

AndyAyersMS · 2025-06-24T18:38:17Z

Conditional Escape Analysis (CEA) is currently driven by flagging GDV calls made to IEnumerable<T>.GetEnumerator. Generalize this slightly to instead look for any call that returns an IEnumerator<T>, since there are some classes that return enumerators via other calls. These calls likely won't be GDVs; I will address adapting CEA to handle that in subsequent changes.

…umerator<T>` Conditional Escape Analysis (CEA) is currently driven by flagging GDV calls made to `IEnumerable<T>.GetEnumerator`. Generalize this slightly to instead look for any call that returns an `IEnumerator<T>`, since there are some classes that return enumerators via other calls. These calls likely won't be GDVs; I will address adapting CEA to handle that in subsequent changes.

AndyAyersMS · 2025-06-24T18:39:01Z

SPMI not going to be helpful here, but I should be able to replay a bespoke collection on this PR vs base. Expecting no diffs (or at least no lost CEAs).

Also this PR still includes the "old way" of checking for flagged calls, and an assert if the old way ever finds one that the new way doesn't.

Copilot

Pull Request Overview

This PR broadens escape analysis to flag any calls returning IEnumerator<T> rather than only IEnumerable<T>.GetEnumerator.

Updated IntrinsicAttribute to apply to interfaces.
Marked IEnumerator<T> as intrinsic in CoreLib.
Enhanced JIT importer to detect calls whose return type is IEnumerator<T> via metadata inspection.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/IntrinsicAttribute.cs	Allow `[Intrinsic]` on interfaces
src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/CompilerServices/IntrinsicAttribute.cs	Same interface support in NativeAOT runtime
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerator.cs	Added `[Intrinsic]` to generic IEnumerator
src/coreclr/jit/importer.cpp	Generalized detection logic to “returning IEnumerator”

Comments suppressed due to low confidence (2)

src/coreclr/jit/importer.cpp:6975

The new isEnumeratorT path handles non-GDV calls returning IEnumerator<T>, but there are no tests covering this scenario. Consider adding a JIT import or codegen test that invokes a custom method returning IEnumerator<T> to verify escape analysis is applied correctly.

                        if (isEnumeratorT)

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/IntrinsicAttribute.cs:9

The summary comment above this attribute should be updated to mention that interfaces are now supported as intrinsic targets, improving clarity for future maintainers.

    [AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface, Inherited = false)]

Copilot · 2025-06-24T18:39:30Z

src/coreclr/jit/importer.cpp

+                            if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&
+                                (strcmp(className, "IEnumerator`1") == 0))
+                            {


[nitpick] Comparing namespace and class names with strcmp on every call can be inefficient. Consider caching the CORINFO_CLASS_HANDLE for IEnumerator and directly comparing handles to avoid repeated string comparisons.

Suggested change

if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&

(strcmp(className, "IEnumerator`1") == 0))

{

static CORINFO_CLASS_HANDLE cachedIEnumeratorHandle = NO_CLASS_HANDLE;

if (cachedIEnumeratorHandle == NO_CLASS_HANDLE)

{

cachedIEnumeratorHandle = info.compCompHnd->findClass("System.Collections.Generic", "IEnumerator`1");

}

if (retCls == cachedIEnumeratorHandle)

{

Sorry, copilot. There is no findClass like this.

AndyAyersMS · 2025-06-24T21:07:53Z

SPMI not going to be helpful here, but I should be able to replay a bespoke collection on this PR vs base. Expecting no diffs (or at least no lost CEAs).

No diffs in a bespoke SPMI... going to do some other spot checks and also have an issue in NAOT to chase down.

AndyAyersMS · 2025-06-24T21:15:48Z

SPMI not going to be helpful here, but I should be able to replay a bespoke collection on this PR vs base. Expecting no diffs (or at least no lost CEAs).

No diffs in a bespoke SPMI... going to do some other spot checks and also have an issue in NAOT to chase down.

Ah, NAOT is using its exact devirt rules, so the return type is often an instantiated IEnumerator. Will just remove the check since it's served its purpose.

AndyAyersMS · 2025-06-25T00:31:59Z

@dotnet/jit-contrib PTAL

No diffs though hard to see this from CI. Failures are unrelated.

am11 · 2025-06-25T10:56:57Z

src/coreclr/jit/importer.cpp

+                            const char* className = info.compCompHnd->getClassNameFromMetadata(retCls, &namespaceName);
+
+                            if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&
+                                (strcmp(className, "IEnumerator`1") == 0))


Perhaps move this check in lookupNamedIntrinsic and rename NI_System_Collections_Generic_IEnumerable_GetEnumerator since the constant is now unsued.

lookupNamedIntrinsic is for methods. This is a class check.

Removed the now unneded constant and lookup support.

amanasifkhalid · 2025-06-25T15:16:32Z

src/coreclr/jit/importer.cpp

+                            const char* className = info.compCompHnd->getClassNameFromMetadata(retCls, &namespaceName);
+
+                            if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&
+                                (strcmp(className, "IEnumerator`1") == 0))


nit: I'd expect the className comparison to fail faster than the namespaceName comparison -- should we swap the order?

Sure. Might be small compared the cost of obtaining the strings in the first place, but every cycle counts.

Will do this in a subsequent PR.

Copilot AI review requested due to automatic review settings June 24, 2025 18:38

AndyAyersMS requested a review from MichalStrehovsky as a code owner June 24, 2025 18:38

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 24, 2025

dotnet-policy-service bot assigned AndyAyersMS Jun 24, 2025

Copilot AI reviewed Jun 24, 2025

View reviewed changes

AndyAyersMS added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jun 24, 2025

This was referenced Jun 24, 2025

Occasional failure in "browser-wasm windows Release LibraryTests: Build Product" #116671

Closed

browser-wasm Windows build error #116746

Closed

[iOS/tvOS] System.Runtime.Tests crash with signal 4 #116815

Closed

remove paranoia check (not accurate for NAOT)

1e27322

Merge branch 'main' into SwitchCEAToLookForIEnumeratorT

34e56e0

am11 reviewed Jun 25, 2025

View reviewed changes

remove now-unused named intrinsic support for IEnumerable

ca750f8

amanasifkhalid approved these changes Jun 25, 2025

View reviewed changes

AndyAyersMS enabled auto-merge (squash) June 25, 2025 20:51

AndyAyersMS merged commit ba53a92 into dotnet:main Jun 25, 2025
154 of 160 checks passed

LoopedBard3 mentioned this pull request Jul 1, 2025

[Perf] Linux/x64: 8 Regressions on 6/25/2025 10:56:24 PM +00:00 #117204

Closed

github-actions bot locked and limited conversation to collaborators Jul 26, 2025

-                            if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&
-                                (strcmp(className, "IEnumerator`1") == 0))
-                            {
+                            static CORINFO_CLASS_HANDLE cachedIEnumeratorHandle = NO_CLASS_HANDLE;
+                            if (cachedIEnumeratorHandle == NO_CLASS_HANDLE)
+                            {
+                                cachedIEnumeratorHandle = info.compCompHnd->findClass("System.Collections.Generic", "IEnumerator`1");
+                            }
+                            if (retCls == cachedIEnumeratorHandle)
+                            {

JIT: switch escape analysis flagging to look for calls returning IEnumerator<T> #116978

JIT: switch escape analysis flagging to look for calls returning IEnumerator<T> #116978

Uh oh!

Conversation

AndyAyersMS commented Jun 24, 2025

Uh oh!

AndyAyersMS commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS Jun 24, 2025

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS commented Jun 24, 2025

Uh oh!

AndyAyersMS commented Jun 24, 2025

Uh oh!

AndyAyersMS commented Jun 25, 2025

Uh oh!

am11 Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

amanasifkhalid Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

AndyAyersMS Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JIT: switch escape analysis flagging to look for calls returning `IEnumerator<T>` #116978

JIT: switch escape analysis flagging to look for calls returning `IEnumerator<T>` #116978

AndyAyersMS commented Jun 24, 2025 •

edited

Loading