Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6929,30 +6929,33 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}

// If we see a local being assigned the result of a GDV-inlineable
// IEnumerable<T>.GetEnumerator, keep track of both the local and the call.
// GetEnumerator call, keep track of both the local and the call.
//
if (op1->OperIs(GT_RET_EXPR))
{
JITDUMP(".... checking for GDV of IEnumerable<T>...\n");
JITDUMP(".... checking for GDV returning IEnumerator<T>...\n");

GenTreeCall* const call = op1->AsRetExpr()->gtInlineCandidate;
NamedIntrinsic ni = NI_Illegal;
bool isEnumeratorT = false;
GenTreeCall* const call = op1->AsRetExpr()->gtInlineCandidate;
bool isExact = false;
bool isNonNull = false;
CORINFO_CLASS_HANDLE retCls = gtGetClassHandle(call, &isExact, &isNonNull);

// TODO -- handle CT_INDIRECT virtuals here too
// but we don't have the right method handle
//
if (call->gtCallType == CT_USER_FUNC)
{
ni = lookupNamedIntrinsic(call->gtCallMethHnd);
}
else if (call->IsGuardedDevirtualizationCandidate())
if ((retCls != NO_CLASS_HANDLE) && info.compCompHnd->isIntrinsicType(retCls))
{
JITDUMP("No GDV IEnumerable<T> check for [%06u]\n", dspTreeID(call));
const char* namespaceName;
const char* className = info.compCompHnd->getClassNameFromMetadata(retCls, &namespaceName);

if ((strcmp(namespaceName, "System.Collections.Generic") == 0) &&
(strcmp(className, "IEnumerator`1") == 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupNamedIntrinsic is for methods. This is a class check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the now unneded constant and lookup support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
Comment on lines +6949 to +6951
Copy link

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)
{

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, copilot. There is no findClass like this.

isEnumeratorT = true;
}
}

if (ni == NI_System_Collections_Generic_IEnumerable_GetEnumerator)
if (isEnumeratorT)
{
JITDUMP("V%02u value is GDV of IEnumerable<T>.GetEnumerator\n", lclNum);
JITDUMP("V%02u value is IEnumerator<T> via GDV\n", lclNum);
lvaTable[lclNum].lvIsEnumerator = true;
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", dspTreeID(call), lclNum);
getImpEnumeratorGdvLocalMap()->Set(call, lclNum);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
namespace System.Runtime.CompilerServices
{
// This attribute is only for use in a Class Library
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field, Inherited = false)]
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface, Inherited = false)]
internal sealed class IntrinsicAttribute : Attribute
{
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

namespace System.Collections.Generic
{
// Base interface for all generic enumerators, providing a simple approach
// to iterating over a collection.
[Intrinsic]
public interface IEnumerator<out T> : IDisposable, IEnumerator
where T : allows ref struct
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace System.Runtime.CompilerServices
// Calls to methods or references to fields marked with this attribute may be replaced at
// some call sites with jit intrinsic expansions.
// Types marked with this attribute may be specially treated by the runtime/compiler.
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field, Inherited = false)]
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Field | AttributeTargets.Interface, Inherited = false)]
internal sealed class IntrinsicAttribute : Attribute
{
}
Expand Down
Loading