Skip to content

Stop spilling ldvirtftn #120866

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+37 −35
Open

Stop spilling ldvirtftn #120866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c781c1
Stop spilling ldvirtftn
hez2010 Oct 18, 2025
a2a1d86
Try fixing assertion
hez2010 Oct 18, 2025
bdcaf94
Merge branch 'main' into ldvirtftn-no-spill
hez2010 Oct 18, 2025
98f9883
Merge branch 'main' into ldvirtftn-no-spill
hez2010 Oct 19, 2025
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
4 changes: 2 additions & 2 deletions src/coreclr/inc/corinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,7 +1522,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO
// - If pResolvedTokenDevirtualizedMethod is not set to NULL and targeting an R2R image
// use it as the parameter to getCallInfo
// - isInstantiatingStub is set to TRUE if the devirtualized method is a generic method instantiating stub
// - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization
// - needsMethodContext is set TRUE if the devirtualized method requires a method context
// (in which case the method handle and context will be a generic method)
//
CORINFO_METHOD_HANDLE devirtualizedMethod;
Expand All @@ -1531,7 +1531,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO
CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod;
CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod;
bool isInstantiatingStub;
bool wasArrayInterfaceDevirt;
bool needsMethodContext;
Copy link
Contributor Author

@hez2010 hez2010 Oct 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is just a rename without any meaningful change.

};

//----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,7 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode(
/*****************************************************************************/

//------------------------------------------------------------------------------
// gtNewHelperCallNode : Helper to create a call helper node.
// gtNewVirtualFunctionLookupHelperCallNode : Helper to create a virtual function lookup helper node.
//
//
// Arguments:
Expand Down
41 changes: 22 additions & 19 deletions src/coreclr/jit/importercalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,23 +400,26 @@ var_types Compiler::impImportCall(OPCODE opcode,
thisPtr = impTransformThis(thisPtr, pConstrainedResolvedToken, callInfo->thisTransform);
assert(thisPtr != nullptr);

GenTree* origThisPtr = thisPtr;
// Clone the (possibly transformed) "this" pointer
GenTree* thisPtrCopy;
thisPtr =
impCloneExpr(thisPtr, &thisPtrCopy, CHECK_SPILL_ALL, nullptr DEBUGARG("LDVIRTFTN this pointer"));

// We cloned the "this" pointer, mark it as a single def and set the class for it
if (thisPtr->TypeIs(TYP_REF) && (origThisPtr != thisPtr))
{
lvaGetDesc(thisPtr->AsLclVar())->lvSingleDef = 1;
lvaSetClass(thisPtr->AsLclVar()->GetLclNum(), origThisPtr);
}

GenTree* fptr = impImportLdvirtftn(thisPtr, pResolvedToken, callInfo);
assert(fptr != nullptr);

call->AsCall()
->gtArgs.PushFront(this, NewCallArg::Primitive(thisPtrCopy).WellKnown(WellKnownArg::ThisPointer));

// Now make an indirect call through the function pointer

unsigned lclNum = lvaGrabTemp(true DEBUGARG("VirtualCall through function pointer"));
impStoreToTemp(lclNum, fptr, CHECK_SPILL_ALL);
fptr = gtNewLclvNode(lclNum, TYP_I_IMPL);
Comment on lines -416 to -418
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only actual change.


call->AsCall()->gtCallAddr = fptr;
call->gtFlags |= GTF_EXCEPT | (fptr->gtFlags & GTF_GLOB_EFFECT);

Expand Down Expand Up @@ -7603,7 +7606,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
}

addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs,
clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt,
clsAttrs, likelyHood, dvInfo.needsMethodContext,
dvInfo.isInstantiatingStub, baseMethod, originalContext);
}

Expand Down Expand Up @@ -7676,7 +7679,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,

likelyContext = dvInfo.exactContext;
likelyMethod = dvInfo.devirtualizedMethod;
arrayInterface = dvInfo.wasArrayInterfaceDevirt;
arrayInterface = dvInfo.needsMethodContext;
instantiatingStub = dvInfo.isInstantiatingStub;
}
else
Expand Down Expand Up @@ -8767,14 +8770,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS)
{
assert(!dvInfo.wasArrayInterfaceDevirt);
assert(!dvInfo.needsMethodContext);
derivedClass = (CORINFO_CLASS_HANDLE)((size_t)exactContext & ~CORINFO_CONTEXTFLAGS_MASK);
}
else
{
// Array interface devirt can return a nonvirtual generic method of the non-generic SZArrayHelper class.
//
assert(dvInfo.wasArrayInterfaceDevirt);
assert(dvInfo.needsMethodContext);
assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD);
derivedClass = info.compCompHnd->getMethodClass(derivedMethod);
}
Expand All @@ -8795,9 +8798,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

if (dvInfo.isInstantiatingStub)
{
// We should only end up with generic methods for array interface devirt.
// We should only end up with generic methods that needs a method context (eg. array interface).
//
assert(dvInfo.wasArrayInterfaceDevirt);
assert(dvInfo.needsMethodContext);

// We don't expect NAOT to end up here, since it has Array<T>
// and normal devirtualization.
Expand Down Expand Up @@ -8920,14 +8923,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

JITDUMP(" %s; can devirtualize\n", note);

// Make the updates.
call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
call->gtFlags &= ~GTF_CALL_VIRT_STUB;
call->gtCallMethHnd = derivedMethod;
call->gtCallType = CT_USER_FUNC;
call->gtControlExpr = nullptr;
INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_DEVIRTUALIZED);

if (dvInfo.isInstantiatingStub)
{
// Pass the instantiating stub method desc as the inst param arg.
Expand All @@ -8939,6 +8934,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,
call->gtArgs.InsertInstParam(this, instParam);
}

// Make the updates.
call->gtFlags &= ~GTF_CALL_VIRT_VTABLE;
call->gtFlags &= ~GTF_CALL_VIRT_STUB;
call->gtCallMethHnd = derivedMethod;
call->gtCallType = CT_USER_FUNC;
call->gtControlExpr = nullptr;
INDEBUG(call->gtCallDebugFlags |= GTF_CALL_MD_DEVIRTUALIZED);

// Virtual calls include an implicit null check, which we may
// now need to make explicit.
if (!objIsNonNull)
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/lower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9451,7 +9451,6 @@ void Lowering::ContainCheckNode(GenTree* node)
ContainCheckIndir(node->AsIndir());
break;
case GT_PUTARG_REG:
case GT_PUTARG_STK:
Copy link
Member

Choose a reason for hiding this comment

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

what is the motivation behind this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's trying to suppress an assertion before addressing the lowering issue that is specific to windows-x86:

15:32:19.222 Running test: JIT/Directed/tailcall/more_tailcalls/more_tailcalls.cmd

Assert failure(PID 204 [0x000000cc], Thread: 2564 [0x0a04]): Assertion failed 'node->GetRegNum() != REG_NA' in 'GenInstance`2[System.__Canon,int]:VirtForward[System.__Canon,System.__Canon](System.__Canon,int,System.__Canon,System.__Canon):System.String:this' during 'Lowering nodeinfo' (IL size 23; hash 0xb3dc36bf; FullOpts)

    File: D:\a\_work\1\s\src\coreclr\jit\lower.cpp:9445
    Image: C:\h\w\ABFD09AF\p\corerun.exe

15:36:59.890 Running test: JIT/Regression/JitBlue/Runtime_87393/Runtime_87393/Runtime_87393.cmd

Assert failure(PID 9576 [0x00002568], Thread: 9588 [0x2574]): Assertion failed 'node->GetRegNum() != REG_NA' in 'Runtime_87393.Bar:M2[int](int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int):int' during 'Lowering nodeinfo' (IL size 41; hash 0x12b37abb; FullOpts)

    File: D:\a\_work\1\s\src\coreclr\jit\lower.cpp:9445
    Image: C:\h\w\ABFD09AF\p\corerun.exe

// The regNum must have been set by the lowering of the call.
assert(node->GetRegNum() != REG_NA);
break;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info)
info->exactContext = null;
info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_UNKNOWN;
info->isInstantiatingStub = false;
info->wasArrayInterfaceDevirt = false;
info->needsMethodContext = false;

TypeDesc objType = HandleToObject(info->objClass);

Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO
// - exactContext is set to wrapped CORINFO_CLASS_HANDLE of devirt'ed method table.
// - detail describes the computation done by the jit host
// - isInstantiatingStub is set to TRUE if the devirtualized method is a method instantiation stub
// - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization
// - needsMethodContext is set TRUE if the devirtualized method requires a method context
// (in which case the method handle and context will be a generic method)
//
public CORINFO_METHOD_STRUCT_* devirtualizedMethod;
Expand All @@ -1103,8 +1103,8 @@ public unsafe struct CORINFO_DEVIRTUALIZATION_INFO
public CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod;
public byte _isInstantiatingStub;
public bool isInstantiatingStub { get { return _isInstantiatingStub != 0; } set { _isInstantiatingStub = value ? (byte)1 : (byte)0; } }
public byte _wasArrayInterfaceDevirt;
public bool wasArrayInterfaceDevirt { get { return _wasArrayInterfaceDevirt != 0; } set { _wasArrayInterfaceDevirt = value ? (byte)1 : (byte)0; } }
public byte _needsMethodContext;
public bool needsMethodContext { get { return _needsMethodContext != 0; } set { _needsMethodContext = value ? (byte)1 : (byte)0; } }
}

//----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/superpmi/superpmi-shared/agnostic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ struct Agnostic_ResolveVirtualMethodResult
bool returnValue;
DWORDLONG devirtualizedMethod;
bool isInstantiatingStub;
bool wasArrayInterfaceDevirt;
bool needsMethodContext;
DWORDLONG exactContext;
DWORD detail;
Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod;
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3276,7 +3276,7 @@ void MethodContext::recResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info
result.isInstantiatingStub = info->isInstantiatingStub;
result.exactContext = CastHandle(info->exactContext);
result.detail = (DWORD) info->detail;
result.wasArrayInterfaceDevirt = info->wasArrayInterfaceDevirt;
result.needsMethodContext = info->needsMethodContext;

if (returnValue)
{
Expand All @@ -3301,11 +3301,11 @@ void MethodContext::dmpResolveVirtualMethod(const Agnostic_ResolveVirtualMethodK
key.context,
key.pResolvedTokenVirtualMethodNonNull,
key.pResolvedTokenVirtualMethodNonNull ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(key.pResolvedTokenVirtualMethod).c_str() : "???");
printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", instantiatingStub-%s, wasArrayInterfaceDevirt-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}",
printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", instantiatingStub-%s, needsMethodContext-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}",
result.returnValue ? "true" : "false",
result.devirtualizedMethod,
result.isInstantiatingStub ? "true" : "false",
result.wasArrayInterfaceDevirt ? "true" : "false",
result.needsMethodContext ? "true" : "false",
result.exactContext,
result.detail,
result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedMethod).c_str() : "???",
Expand All @@ -3330,7 +3330,7 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info

info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod;
info->isInstantiatingStub = result.isInstantiatingStub;
info->wasArrayInterfaceDevirt = result.wasArrayInterfaceDevirt;
info->needsMethodContext = result.needsMethodContext;
info->exactContext = (CORINFO_CONTEXT_HANDLE) result.exactContext;
info->detail = (CORINFO_DEVIRTUALIZATION_DETAIL) result.detail;
if (result.returnValue)
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/jitinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8567,7 +8567,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
memset(&info->resolvedTokenDevirtualizedMethod, 0, sizeof(info->resolvedTokenDevirtualizedMethod));
memset(&info->resolvedTokenDevirtualizedUnboxedMethod, 0, sizeof(info->resolvedTokenDevirtualizedUnboxedMethod));
info->isInstantiatingStub = false;
info->wasArrayInterfaceDevirt = false;
info->needsMethodContext = false;

MethodDesc* pBaseMD = GetMethod(info->virtualMethod);
MethodTable* pBaseMT = pBaseMD->GetMethodTable();
Expand Down Expand Up @@ -8811,13 +8811,13 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info)
//
info->isInstantiatingStub = pDevirtMD->IsInstantiatingStub();
info->exactContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD);
info->wasArrayInterfaceDevirt = true;
info->needsMethodContext = true;
}
else
{
info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT);
info->isInstantiatingStub = false;
info->wasArrayInterfaceDevirt = false;
info->needsMethodContext = false;
}

info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD;
Expand Down
Loading