From 1c781c1732d2b6c64d99cdf114bbde91809a86d7 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 18 Oct 2025 22:52:12 +0900 Subject: [PATCH 01/18] Stop spilling ldvirtftn --- src/coreclr/inc/corinfo.h | 4 +- src/coreclr/jit/compiler.hpp | 2 +- src/coreclr/jit/importercalls.cpp | 41 ++++++++++--------- .../tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../tools/Common/JitInterface/CorInfoTypes.cs | 6 +-- .../tools/superpmi/superpmi-shared/agnostic.h | 2 +- .../superpmi-shared/methodcontext.cpp | 8 ++-- src/coreclr/vm/jitinterface.cpp | 6 +-- 8 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index f1a60023132a8a..275acf6b7bdaf6 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -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; @@ -1531,7 +1531,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; bool isInstantiatingStub; - bool wasArrayInterfaceDevirt; + bool needsMethodContext; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 12280cde617228..413ef2c12b5b81 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -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: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index beca2374c6713a..f852fdaca60763 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -400,11 +400,19 @@ 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); @@ -412,11 +420,6 @@ var_types Compiler::impImportCall(OPCODE opcode, ->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); - call->AsCall()->gtCallAddr = fptr; call->gtFlags |= GTF_EXCEPT | (fptr->gtFlags & GTF_GLOB_EFFECT); @@ -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); } @@ -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 @@ -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); } @@ -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 // and normal devirtualization. @@ -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. @@ -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) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index ab43b57640a4a5..eb38f8ac7aeb55 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -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); diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index f2688c585b0e52..cb91809a9813a0 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -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; @@ -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; } } } //---------------------------------------------------------------------------- diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index 01ec01427fa862..831ab5d7eb5dd9 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -676,7 +676,7 @@ struct Agnostic_ResolveVirtualMethodResult bool returnValue; DWORDLONG devirtualizedMethod; bool isInstantiatingStub; - bool wasArrayInterfaceDevirt; + bool needsMethodContext; DWORDLONG exactContext; DWORD detail; Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index ac87130dba1785..546d99fbbf4819 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -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) { @@ -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() : "???", @@ -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) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 77317625dbadd1..75c913bd4b0a34 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8549,7 +8549,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(); @@ -8793,13 +8793,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; From a2a1d86ca890773ba3da171955a2bf92880c2c91 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 19 Oct 2025 01:42:07 +0900 Subject: [PATCH 02/18] Try fixing assertion --- src/coreclr/jit/lower.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 6e7625745cb2e0..d0d4d98ab30a86 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -9440,7 +9440,6 @@ void Lowering::ContainCheckNode(GenTree* node) ContainCheckIndir(node->AsIndir()); break; case GT_PUTARG_REG: - case GT_PUTARG_STK: // The regNum must have been set by the lowering of the call. assert(node->GetRegNum() != REG_NA); break; From 3c07e08d1d2966b93bdecc63006d72140abe820f Mon Sep 17 00:00:00 2001 From: Steve Date: Mon, 17 Nov 2025 14:02:27 +0900 Subject: [PATCH 03/18] Try another fix --- src/coreclr/jit/lower.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index ab73e8fcdb4dd9..d94eec79d6b16c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3495,6 +3495,20 @@ GenTree* Lowering::LowerTailCallViaJitHelper(GenTreeCall* call, GenTree* callTar unsigned numArgs = call->gtArgs.CountArgs(); + // JIT_TailCall helper expects the target function pointer arg to have been materialized, + // because LowerTailCallViaJitHelper rewrites stack arguments assuming that the first arg + // is a simple local. Historically ldvirtftn was always spilled to a temp which satisfied + // this requirement. After removing that spill, we can end up with an arbitrary tree here, + // so force it into a temp when necessary. + // + CallArg* callTargetArg = call->gtArgs.GetArgByIndex(numArgs - 1); + GenTreePutArgStk* callTargetPutArg = callTargetArg->GetEarlyNode()->AsPutArgStk(); + if (!callTargetPutArg->gtGetOp1()->OperIs(GT_LCL_VAR)) + { + LIR::Use targetUse(BlockRange(), &callTargetPutArg->gtOp1, callTargetPutArg); + targetUse.ReplaceWithLclVar(comp); + } + // arg 0 == callTarget. CallArg* argEntry = call->gtArgs.GetArgByIndex(numArgs - 1); assert(argEntry != nullptr); @@ -9451,6 +9465,7 @@ void Lowering::ContainCheckNode(GenTree* node) ContainCheckIndir(node->AsIndir()); break; case GT_PUTARG_REG: + case GT_PUTARG_STK: // The regNum must have been set by the lowering of the call. assert(node->GetRegNum() != REG_NA); break; From 19983669710343294e59452215e8d4acb01008c9 Mon Sep 17 00:00:00 2001 From: Steve Date: Mon, 17 Nov 2025 15:44:34 +0900 Subject: [PATCH 04/18] Fix NativeAOT fat pointer handling --- src/coreclr/jit/importercalls.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8a4e42906336b9..d5fd19ad3745e0 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -6950,6 +6950,18 @@ class SpillRetExprHelper void Compiler::addFatPointerCandidate(GenTreeCall* call) { JITDUMP("Marking call [%06u] as fat pointer candidate\n", dspTreeID(call)); + + GenTree* fptr = call->gtCallAddr; + assert(fptr != nullptr); + if (!fptr->OperIsLocalRead()) + { + // Spill the call address to a local for fat pointer handling + const unsigned fptrLclNum = lvaGrabTemp(true DEBUGARG("fat pointer temp")); + impStoreToTemp(fptrLclNum, fptr, CHECK_SPILL_ALL); + fptr = gtNewLclvNode(fptrLclNum, genActualType(fptr->TypeGet())); + call->gtCallAddr = fptr; + } + setMethodHasFatPointer(); call->SetFatPointerCandidate(); SpillRetExprHelper helper(this); From d058cb034077d0cc38b6c59d324bab5dd4988e29 Mon Sep 17 00:00:00 2001 From: Steve Date: Mon, 17 Nov 2025 15:44:43 +0900 Subject: [PATCH 05/18] Minor refactor --- src/coreclr/jit/lower.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index dbe3e6bc5ef54c..526e02e643857b 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3491,23 +3491,23 @@ GenTree* Lowering::LowerTailCallViaJitHelper(GenTreeCall* call, GenTree* callTar unsigned numArgs = call->gtArgs.CountArgs(); + // arg 0 == callTarget. + CallArg* argEntry = call->gtArgs.GetArgByIndex(numArgs - 1); + assert(argEntry != nullptr); + // JIT_TailCall helper expects the target function pointer arg to have been materialized, // because LowerTailCallViaJitHelper rewrites stack arguments assuming that the first arg // is a simple local. Historically ldvirtftn was always spilled to a temp which satisfied // this requirement. After removing that spill, we can end up with an arbitrary tree here, // so force it into a temp when necessary. // - CallArg* callTargetArg = call->gtArgs.GetArgByIndex(numArgs - 1); - GenTreePutArgStk* callTargetPutArg = callTargetArg->GetEarlyNode()->AsPutArgStk(); - if (!callTargetPutArg->gtGetOp1()->OperIs(GT_LCL_VAR)) + GenTreePutArgStk* argEntryPutArg = argEntry->GetEarlyNode()->AsPutArgStk(); + if (!argEntryPutArg->gtGetOp1()->OperIs(GT_LCL_VAR)) { - LIR::Use targetUse(BlockRange(), &callTargetPutArg->gtOp1, callTargetPutArg); + LIR::Use targetUse(BlockRange(), &argEntryPutArg->gtOp1, argEntryPutArg); targetUse.ReplaceWithLclVar(comp); } - // arg 0 == callTarget. - CallArg* argEntry = call->gtArgs.GetArgByIndex(numArgs - 1); - assert(argEntry != nullptr); GenTree* arg0 = argEntry->GetEarlyNode()->AsPutArgStk()->gtGetOp1(); bool isClosed; From ee46acd42aa33991c10a4c7c57b9ad15e1afc508 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 17 Nov 2025 23:14:27 +0900 Subject: [PATCH 06/18] Revert lower change --- src/coreclr/jit/lower.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 526e02e643857b..f6c0e8634e36bf 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3494,20 +3494,6 @@ GenTree* Lowering::LowerTailCallViaJitHelper(GenTreeCall* call, GenTree* callTar // arg 0 == callTarget. CallArg* argEntry = call->gtArgs.GetArgByIndex(numArgs - 1); assert(argEntry != nullptr); - - // JIT_TailCall helper expects the target function pointer arg to have been materialized, - // because LowerTailCallViaJitHelper rewrites stack arguments assuming that the first arg - // is a simple local. Historically ldvirtftn was always spilled to a temp which satisfied - // this requirement. After removing that spill, we can end up with an arbitrary tree here, - // so force it into a temp when necessary. - // - GenTreePutArgStk* argEntryPutArg = argEntry->GetEarlyNode()->AsPutArgStk(); - if (!argEntryPutArg->gtGetOp1()->OperIs(GT_LCL_VAR)) - { - LIR::Use targetUse(BlockRange(), &argEntryPutArg->gtOp1, argEntryPutArg); - targetUse.ReplaceWithLclVar(comp); - } - GenTree* arg0 = argEntry->GetEarlyNode()->AsPutArgStk()->gtGetOp1(); bool isClosed; From 6c93a142063babc31126ad8c25cc68a2befc74f4 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 17 Nov 2025 23:15:36 +0900 Subject: [PATCH 07/18] Remove out-of-scope change --- src/coreclr/jit/importercalls.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index d5fd19ad3745e0..508728a6540599 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -406,19 +406,11 @@ 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); From 4b854f974de058ccffcb7148e3b2352935e14bf4 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 17 Nov 2025 16:55:36 +0100 Subject: [PATCH 08/18] Optimize call target ordering in lowering --- src/coreclr/jit/lower.cpp | 86 +++++++++++++++++++++++++++++++++++++++ src/coreclr/jit/lower.h | 1 + 2 files changed, 87 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index f6c0e8634e36bf..8eab4242b281f3 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2833,6 +2833,12 @@ GenTree* Lowering::LowerCall(GenTree* node) #endif call->ClearOtherRegs(); + + if (call->gtCallType == CT_INDIRECT) + { + OptimizeCallIndirectTargetEvaluation(call); + } + LowerArgsForCall(call); // note that everything generated from this point might run AFTER the outgoing args are placed @@ -6282,6 +6288,86 @@ GenTree* Lowering::LowerDelegateInvoke(GenTreeCall* call) return callTarget; } +//------------------------------------------------------------------------ +// OptimizeCallIndirectTargetEvaluation: +// Try to optimize the evaluation of the indirect call target to happen +// before arguments, if possible. +// +// Parameters: +// call - Call node +// +void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) +{ + assert((call->gtCallType == CT_INDIRECT) && (call->gtCallAddr != nullptr)); + + if (!call->gtCallAddr->IsHelperCall(comp, CORINFO_HELP_VIRTUAL_FUNC_PTR) && + !call->gtCallAddr->IsHelperCall(comp, CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR) && + !call->gtCallAddr->IsHelperCall(comp, CORINFO_HELP_GVMLOOKUP_FOR_SLOT) && + !call->gtCallAddr->IsHelperCall(comp, CORINFO_HELP_READYTORUN_GENERIC_HANDLE)) + { + return; + } + + JITDUMP("Target is a GVM; seeing if we can move arguments ahead of resolution\n"); + + bool callClosed; + LIR::ReadOnlyRange callRange = BlockRange().GetTreeRange(call, &callClosed); + if (!callClosed) + { + JITDUMP(" No; call range is not closed\n"); + return; + } + + bool tarClosed; + LIR::ReadOnlyRange tarRange = BlockRange().GetTreeRange(call->gtCallAddr, &tarClosed); + if (!tarClosed) + { + JITDUMP(" No; target range is not closed\n"); + return; + } + + if (tarRange.FirstNode() == callRange.FirstNode()) + { + JITDUMP(" Resolution is already before args (or there are no args)\n"); + // Nothing to do + return; + } + + if (!IsRangeInvariantInRange(callRange.FirstNode(), tarRange.FirstNode()->gtPrev, tarRange.LastNode(), nullptr)) + { + JITDUMP(" No; range of nodes is not invariant\n"); + return; + } + + // We did not check CORINFO_HELP_VIRTUAL_FUNC_PTR. It can throw NRE, so + // verify that we can move other effects past NRE. + for (GenTree* cur = callRange.FirstNode(); cur != tarRange.FirstNode(); cur = cur->gtNext) + { + GenTreeFlags flags = cur->OperEffects(comp); + if ((flags & GTF_PERSISTENT_SIDE_EFFECTS) != 0) + { + JITDUMP(" No; [%06u] has persistent side effects\n", Compiler::dspTreeID(cur)); + return; + } + + if ((flags & GTF_EXCEPT) != 0) + { + ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); + if (preciseExceptions != ExceptionSetFlags::NullReferenceException) + { + JITDUMP(" No; [%06u] throws an exception that is not NRE\n", Compiler::dspTreeID(cur)); + return; + } + } + } + + LIR::Range removedRange = BlockRange().Remove(std::move(tarRange)); + + BlockRange().InsertBefore(callRange.FirstNode(), std::move(removedRange)); + JITDUMP("Moved target evaluation before arguments:\n"); + DISPTREERANGE(BlockRange(), call); +} + GenTree* Lowering::LowerIndirectNonvirtCall(GenTreeCall* call) { #ifdef TARGET_X86 diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 5c4d356b737a0e..f495bcdc96b9b9 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -196,6 +196,7 @@ class Lowering final : public Phase GenTreeLclVar* SpillStructCallResult(GenTreeCall* call) const; #endif // WINDOWS_AMD64_ABI GenTree* LowerDelegateInvoke(GenTreeCall* call); + void OptimizeCallIndirectTargetEvaluation(GenTreeCall* call); GenTree* LowerIndirectNonvirtCall(GenTreeCall* call); GenTree* LowerDirectCall(GenTreeCall* call); GenTree* LowerNonvirtPinvokeCall(GenTreeCall* call); From a9ef09db24945a751313116853bca5d9afd52a7d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 17 Nov 2025 18:21:45 +0100 Subject: [PATCH 09/18] Generalize optimization --- src/coreclr/jit/lower.cpp | 148 +++++++++++++++++++++++++++----------- 1 file changed, 108 insertions(+), 40 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 8eab4242b281f3..1fce25ab1d1fb9 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6310,61 +6310,129 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) JITDUMP("Target is a GVM; seeing if we can move arguments ahead of resolution\n"); - bool callClosed; - LIR::ReadOnlyRange callRange = BlockRange().GetTreeRange(call, &callClosed); - if (!callClosed) - { - JITDUMP(" No; call range is not closed\n"); - return; - } + m_scratchSideEffects.Clear(); - bool tarClosed; - LIR::ReadOnlyRange tarRange = BlockRange().GetTreeRange(call->gtCallAddr, &tarClosed); - if (!tarClosed) - { - JITDUMP(" No; target range is not closed\n"); - return; - } + // Now move nodes ahead of the target range until we interfere or until we + // run out of nodes in the call's data flow. + unsigned numMarked = 1; + call->gtLIRFlags |= LIR::Flags::Mark; - if (tarRange.FirstNode() == callRange.FirstNode()) - { - JITDUMP(" Resolution is already before args (or there are no args)\n"); - // Nothing to do - return; - } + LIR::ReadOnlyRange movingRange; - if (!IsRangeInvariantInRange(callRange.FirstNode(), tarRange.FirstNode()->gtPrev, tarRange.LastNode(), nullptr)) + GenTree* prev; + for (GenTree* cur = call; numMarked > 0; cur = prev) { - JITDUMP(" No; range of nodes is not invariant\n"); - return; - } + prev = cur->gtPrev; - // We did not check CORINFO_HELP_VIRTUAL_FUNC_PTR. It can throw NRE, so - // verify that we can move other effects past NRE. - for (GenTree* cur = callRange.FirstNode(); cur != tarRange.FirstNode(); cur = cur->gtNext) - { - GenTreeFlags flags = cur->OperEffects(comp); - if ((flags & GTF_PERSISTENT_SIDE_EFFECTS) != 0) + if ((cur->gtLIRFlags & LIR::Flags::Mark) == 0) { - JITDUMP(" No; [%06u] has persistent side effects\n", Compiler::dspTreeID(cur)); - return; + // If we are still moving nodes then extend the range so that we + // also move this node outside the data flow of the call. + if (!movingRange.IsEmpty()) + { + assert(cur->gtNext == movingRange.FirstNode()); + movingRange = LIR::ReadOnlyRange(cur, movingRange.LastNode()); + m_scratchSideEffects.AddNode(comp, cur); + } + + continue; } - if ((flags & GTF_EXCEPT) != 0) + cur->gtLIRFlags &= ~LIR::Flags::Mark; + numMarked--; + + if (cur == call->gtCallAddr) { - ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); - if (preciseExceptions != ExceptionSetFlags::NullReferenceException) + // Start moving this range. Do not add its side effects as we will + // check the NRE manually for precision. + movingRange = LIR::ReadOnlyRange(cur, cur); + continue; + } + + cur->VisitOperands([&](GenTree* op) { + assert((op->gtLIRFlags & LIR::Flags::Mark) == 0); + op->gtLIRFlags |= LIR::Flags::Mark; + numMarked++; + return GenTree::VisitResult::Continue; + }); + + if (!movingRange.IsEmpty()) + { + // This node is in the dataflow. See if we can move it ahead of the + // range we are moving. + bool interferes = m_scratchSideEffects.InterferesWith(comp, cur, /* strict */ true); + if (!interferes) { - JITDUMP(" No; [%06u] throws an exception that is not NRE\n", Compiler::dspTreeID(cur)); - return; + // No problem so far. However the side effect set does not + // include the GVM call itself, which can throw NRE. Check the + // NRE now for precision. + GenTreeFlags flags = cur->OperEffects(comp); + if ((flags & GTF_PERSISTENT_SIDE_EFFECTS) != 0) + { + JITDUMP(" Stopping at [%06u]; it has persistent side effects\n", Compiler::dspTreeID(cur)); + interferes = true; + } + + if ((flags & GTF_EXCEPT) != 0) + { + ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); + if (preciseExceptions != ExceptionSetFlags::NullReferenceException) + { + JITDUMP(" Stopping at [%06u]; it throws an exception that is not NRE\n", + Compiler::dspTreeID(cur)); + interferes = true; + } + } + } + + if (interferes) + { + // Stop moving the range, but keep going through the rest + // of the nodes to unmark them + movingRange = LIR::Range(); + } + else + { + // Move 'cur' ahead of 'movingRange' + assert(cur->gtNext == movingRange.FirstNode()); + BlockRange().Remove(cur); + BlockRange().InsertAfter(movingRange.LastNode(), cur); } } } - LIR::Range removedRange = BlockRange().Remove(std::move(tarRange)); + // if (!IsRangeInvariantInRange(callRange.FirstNode(), tarRange.FirstNode()->gtPrev, tarRange.LastNode(), nullptr)) + //{ + // JITDUMP(" No; range of nodes is not invariant\n"); + // return; + // } + + // We did not check CORINFO_HELP_VIRTUAL_FUNC_PTR. It can throw NRE, so + // verify that we can move other effects past NRE. + // for (GenTree* cur = callRange.FirstNode(); cur != tarRange.FirstNode(); cur = cur->gtNext) + //{ + // GenTreeFlags flags = cur->OperEffects(comp); + // if ((flags & GTF_PERSISTENT_SIDE_EFFECTS) != 0) + // { + // JITDUMP(" No; [%06u] has persistent side effects\n", Compiler::dspTreeID(cur)); + // return; + // } + + // if ((flags & GTF_EXCEPT) != 0) + // { + // ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); + // if (preciseExceptions != ExceptionSetFlags::NullReferenceException) + // { + // JITDUMP(" No; [%06u] throws an exception that is not NRE\n", Compiler::dspTreeID(cur)); + // return; + // } + // } + //} + + // LIR::Range removedRange = BlockRange().Remove(std::move(tarRange)); - BlockRange().InsertBefore(callRange.FirstNode(), std::move(removedRange)); - JITDUMP("Moved target evaluation before arguments:\n"); + // BlockRange().InsertBefore(callRange.FirstNode(), std::move(removedRange)); + JITDUMP("Result of moved target evaluation:\n"); DISPTREERANGE(BlockRange(), call); } From 3853113f3d2e40639016ec90e3ddaf1b70acc2be Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 17 Nov 2025 18:23:40 +0100 Subject: [PATCH 10/18] Remove commented code --- src/coreclr/jit/lower.cpp | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 1fce25ab1d1fb9..3adbd40c826df2 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6401,37 +6401,6 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) } } - // if (!IsRangeInvariantInRange(callRange.FirstNode(), tarRange.FirstNode()->gtPrev, tarRange.LastNode(), nullptr)) - //{ - // JITDUMP(" No; range of nodes is not invariant\n"); - // return; - // } - - // We did not check CORINFO_HELP_VIRTUAL_FUNC_PTR. It can throw NRE, so - // verify that we can move other effects past NRE. - // for (GenTree* cur = callRange.FirstNode(); cur != tarRange.FirstNode(); cur = cur->gtNext) - //{ - // GenTreeFlags flags = cur->OperEffects(comp); - // if ((flags & GTF_PERSISTENT_SIDE_EFFECTS) != 0) - // { - // JITDUMP(" No; [%06u] has persistent side effects\n", Compiler::dspTreeID(cur)); - // return; - // } - - // if ((flags & GTF_EXCEPT) != 0) - // { - // ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); - // if (preciseExceptions != ExceptionSetFlags::NullReferenceException) - // { - // JITDUMP(" No; [%06u] throws an exception that is not NRE\n", Compiler::dspTreeID(cur)); - // return; - // } - // } - //} - - // LIR::Range removedRange = BlockRange().Remove(std::move(tarRange)); - - // BlockRange().InsertBefore(callRange.FirstNode(), std::move(removedRange)); JITDUMP("Result of moved target evaluation:\n"); DISPTREERANGE(BlockRange(), call); } From c228ca4981ced760a4a42cd86505e7cd8e75aeff Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Nov 2025 22:06:00 +0900 Subject: [PATCH 11/18] Spill side-effects for fat pointer handling before impPopCallArgs --- src/coreclr/jit/importercalls.cpp | 38 ++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 508728a6540599..116a368769d8d6 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -386,6 +386,27 @@ var_types Compiler::impImportCall(OPCODE opcode, { assert(!(mflags & CORINFO_FLG_STATIC)); // can't call a static method assert(!(clsFlags & CORINFO_FLG_VALUECLASS)); + assert(stackState.esStackDepth >= sig->numArgs + 1); + + const bool needsFatPointerHandling = + (sig->sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_NATIVEAOT_ABI); + if (needsFatPointerHandling) + { + // NativeAOT generic virtual method: need to handle potential fat function pointers + // Spill any side-effecting arguments before we do the LDVIRTFTN + for (unsigned argIndex = 0; argIndex < sig->numArgs; argIndex++) + { + const unsigned level = stackState.esStackDepth - 1 - argIndex; + GenTree* argTree = stackState.esStack[level].val; + if ((argTree->gtFlags & GTF_SIDE_EFFECT) == 0) + { + continue; + } + + impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(false) DEBUGARG("fat pointer arg spill")); + } + } + // OK, We've been told to call via LDVIRTFTN, so just // take the call now.... call = gtNewIndCallNode(nullptr, callRetTyp, di); @@ -421,9 +442,11 @@ var_types Compiler::impImportCall(OPCODE opcode, call->AsCall()->gtCallAddr = fptr; call->gtFlags |= GTF_EXCEPT | (fptr->gtFlags & GTF_GLOB_EFFECT); - if ((sig->sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_NATIVEAOT_ABI)) + if (needsFatPointerHandling) { - // NativeAOT generic virtual method: need to handle potential fat function pointers + const unsigned fptrLclNum = lvaGrabTemp(true DEBUGARG("fat pointer temp")); + impStoreToTemp(fptrLclNum, fptr, CHECK_SPILL_ALL); + call->AsCall()->gtCallAddr = gtNewLclvNode(fptrLclNum, genActualType(fptr->TypeGet())); addFatPointerCandidate(call->AsCall()); } #ifdef FEATURE_READYTORUN @@ -6943,17 +6966,6 @@ void Compiler::addFatPointerCandidate(GenTreeCall* call) { JITDUMP("Marking call [%06u] as fat pointer candidate\n", dspTreeID(call)); - GenTree* fptr = call->gtCallAddr; - assert(fptr != nullptr); - if (!fptr->OperIsLocalRead()) - { - // Spill the call address to a local for fat pointer handling - const unsigned fptrLclNum = lvaGrabTemp(true DEBUGARG("fat pointer temp")); - impStoreToTemp(fptrLclNum, fptr, CHECK_SPILL_ALL); - fptr = gtNewLclvNode(fptrLclNum, genActualType(fptr->TypeGet())); - call->gtCallAddr = fptr; - } - setMethodHasFatPointer(); call->SetFatPointerCandidate(); SpillRetExprHelper helper(this); From 21e890ba68e64c15f90f83325a34266b07fd3e2e Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Nov 2025 22:37:28 +0900 Subject: [PATCH 12/18] Fix build break --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 116a368769d8d6..4ea0adc190b162 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -386,7 +386,7 @@ var_types Compiler::impImportCall(OPCODE opcode, { assert(!(mflags & CORINFO_FLG_STATIC)); // can't call a static method assert(!(clsFlags & CORINFO_FLG_VALUECLASS)); - assert(stackState.esStackDepth >= sig->numArgs + 1); + assert(stackState.esStackDepth >= sig->numArgs + 1u); const bool needsFatPointerHandling = (sig->sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_NATIVEAOT_ABI); From fed014d612b5feff7f6ae0c5d122a83788a75aa6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Nov 2025 22:45:49 +0900 Subject: [PATCH 13/18] Use impSpillSideEffects --- src/coreclr/jit/importercalls.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 4ea0adc190b162..59250d697df502 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -386,7 +386,6 @@ var_types Compiler::impImportCall(OPCODE opcode, { assert(!(mflags & CORINFO_FLG_STATIC)); // can't call a static method assert(!(clsFlags & CORINFO_FLG_VALUECLASS)); - assert(stackState.esStackDepth >= sig->numArgs + 1u); const bool needsFatPointerHandling = (sig->sigInst.methInstCount != 0) && IsTargetAbi(CORINFO_NATIVEAOT_ABI); @@ -394,17 +393,7 @@ var_types Compiler::impImportCall(OPCODE opcode, { // NativeAOT generic virtual method: need to handle potential fat function pointers // Spill any side-effecting arguments before we do the LDVIRTFTN - for (unsigned argIndex = 0; argIndex < sig->numArgs; argIndex++) - { - const unsigned level = stackState.esStackDepth - 1 - argIndex; - GenTree* argTree = stackState.esStack[level].val; - if ((argTree->gtFlags & GTF_SIDE_EFFECT) == 0) - { - continue; - } - - impSpillStackEntry(level, BAD_VAR_NUM DEBUGARG(false) DEBUGARG("fat pointer arg spill")); - } + impSpillSideEffects(false, CHECK_SPILL_ALL DEBUGARG("fat pointer arg spill")); } // OK, We've been told to call via LDVIRTFTN, so just From e459630acbcc98574d8f0262ab3d47374c2e18b3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Nov 2025 15:23:09 +0100 Subject: [PATCH 14/18] Skip opt for debug codegen, do not call OperExceptions if we already interfere --- src/coreclr/jit/lower.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index f0b5e5b53b29d7..0665d250174488 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2834,7 +2834,7 @@ GenTree* Lowering::LowerCall(GenTree* node) call->ClearOtherRegs(); - if (call->gtCallType == CT_INDIRECT) + if ((call->gtCallType == CT_INDIRECT) && comp->opts.Tier0OptimizationEnabled()) { OptimizeCallIndirectTargetEvaluation(call); } @@ -6316,8 +6316,16 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) m_scratchSideEffects.Clear(); - // Now move nodes ahead of the target range until we interfere or until we - // run out of nodes in the call's data flow. + // We start at the call and move backwards from it. When we see a node that + // is part of the call's data flow we leave it in place. For nodes that are + // not part of the data flow, or that are part of the target's data flow, + // we move them before the call's data flow if legal. We stop when we run + // out of the call's data flow. + // + // The end result is that all nodes outside the call's data flow or inside + // the target's data flow are computed before call arguments, allowing LSRA + // to resolve the ABI constraints without interfering with the target + // computation. unsigned numMarked = 1; call->gtLIRFlags |= LIR::Flags::Mark; @@ -6376,8 +6384,7 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) JITDUMP(" Stopping at [%06u]; it has persistent side effects\n", Compiler::dspTreeID(cur)); interferes = true; } - - if ((flags & GTF_EXCEPT) != 0) + else if ((flags & GTF_EXCEPT) != 0) { ExceptionSetFlags preciseExceptions = cur->OperExceptions(comp); if (preciseExceptions != ExceptionSetFlags::NullReferenceException) From 00fd2886d01a113f8b78c69f0a9550029b8cd8c3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Nov 2025 15:42:50 +0100 Subject: [PATCH 15/18] Add test --- .../JitBlue/Runtime_121711/Runtime_121711.cs | 50 +++++++++++++++++++ .../JIT/Regression/Regression_ro_1.csproj | 1 + 2 files changed, 51 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_121711/Runtime_121711.cs diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_121711/Runtime_121711.cs b/src/tests/JIT/Regression/JitBlue/Runtime_121711/Runtime_121711.cs new file mode 100644 index 00000000000000..19a507a8997314 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_121711/Runtime_121711.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +public class Runtime_121711 +{ + [Fact] + public static int TestEntryPoint() + { + try + { + Test(null); + return 101; + } + catch (NullReferenceException) + { + return _exitCode; + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Test(Base b) + { + b.Foo(Bar()); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static int Bar() + { + _exitCode = 100; + return 42; + } + + private static int _exitCode = 102; +} + +public abstract class Base +{ + public abstract void Foo(int x); +} + +public class Derived : Base +{ + public override void Foo(int x) + { + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/Regression_ro_1.csproj b/src/tests/JIT/Regression/Regression_ro_1.csproj index cad8fb68dfa3e8..7181197dd10a42 100644 --- a/src/tests/JIT/Regression/Regression_ro_1.csproj +++ b/src/tests/JIT/Regression/Regression_ro_1.csproj @@ -74,6 +74,7 @@ + From ef94f07bf4f699f7daf2fa060259c50d865f8bfd Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 26 Nov 2025 18:01:30 +0100 Subject: [PATCH 16/18] Add some JITDUMP --- src/coreclr/jit/lower.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 0665d250174488..475182ef812ec4 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6372,7 +6372,14 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) { // This node is in the dataflow. See if we can move it ahead of the // range we are moving. - bool interferes = m_scratchSideEffects.InterferesWith(comp, cur, /* strict */ true); + bool interferes = false; + if (m_scratchSideEffects.InterferesWith(comp, cur, /* strict */ true)) + { + JITDUMP(" Stopping at [%06u]; it interferes with the current range we are moving\n", + Compiler::dspTreeID(cur)); + interferes = true; + } + if (!interferes) { // No problem so far. However the side effect set does not @@ -6400,7 +6407,7 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) { // Stop moving the range, but keep going through the rest // of the nodes to unmark them - movingRange = LIR::Range(); + movingRange = LIR::ReadOnlyRange(); } else { From 56833e90cdb0dec7e70df17a5e72ec6b4af2258b Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 27 Nov 2025 02:18:40 +0900 Subject: [PATCH 17/18] Revert a bunch of unrelated changes --- src/coreclr/inc/corinfo.h | 4 +-- src/coreclr/jit/importercalls.cpp | 26 +++++++++---------- .../tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../tools/Common/JitInterface/CorInfoTypes.cs | 6 ++--- .../tools/superpmi/superpmi-shared/agnostic.h | 2 +- .../superpmi-shared/methodcontext.cpp | 8 +++--- src/coreclr/vm/jitinterface.cpp | 6 ++--- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 226cf8da1fe120..7cd408af5905bb 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1578,7 +1578,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 - // - needsMethodContext is set TRUE if the devirtualized method requires a method context + // - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization // (in which case the method handle and context will be a generic method) // CORINFO_METHOD_HANDLE devirtualizedMethod; @@ -1587,7 +1587,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; bool isInstantiatingStub; - bool needsMethodContext; + bool wasArrayInterfaceDevirt; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index b12258492ea612..f9d0fddb50af9b 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7603,7 +7603,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, } addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, - clsAttrs, likelyHood, dvInfo.needsMethodContext, + clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt, dvInfo.isInstantiatingStub, baseMethod, originalContext); } @@ -7676,7 +7676,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, likelyContext = dvInfo.exactContext; likelyMethod = dvInfo.devirtualizedMethod; - arrayInterface = dvInfo.needsMethodContext; + arrayInterface = dvInfo.wasArrayInterfaceDevirt; instantiatingStub = dvInfo.isInstantiatingStub; } else @@ -8767,14 +8767,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS) { - assert(!dvInfo.needsMethodContext); + assert(!dvInfo.wasArrayInterfaceDevirt); 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.needsMethodContext); + assert(dvInfo.wasArrayInterfaceDevirt); assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); derivedClass = info.compCompHnd->getMethodClass(derivedMethod); } @@ -8797,7 +8797,7 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, { // We should only end up with generic methods that needs a method context (eg. array interface). // - assert(dvInfo.needsMethodContext); + assert(dvInfo.wasArrayInterfaceDevirt); // We don't expect NAOT to end up here, since it has Array // and normal devirtualization. @@ -8920,6 +8920,14 @@ 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. @@ -8931,14 +8939,6 @@ 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) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 0edb3ec0a4b88a..5ca092d1e5eae0 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -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->needsMethodContext = false; + info->wasArrayInterfaceDevirt = false; TypeDesc objType = HandleToObject(info->objClass); diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index e6d68d07dc0115..db2153d1b35d14 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1156,7 +1156,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 - // - needsMethodContext is set TRUE if the devirtualized method requires a method context + // - wasArrayInterfaceDevirt is set TRUE for array interface method devirtualization // (in which case the method handle and context will be a generic method) // public CORINFO_METHOD_STRUCT_* devirtualizedMethod; @@ -1166,8 +1166,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 _needsMethodContext; - public bool needsMethodContext { get { return _needsMethodContext != 0; } set { _needsMethodContext = value ? (byte)1 : (byte)0; } } + public byte _wasArrayInterfaceDevirt; + public bool wasArrayInterfaceDevirt { get { return _wasArrayInterfaceDevirt != 0; } set { _wasArrayInterfaceDevirt = value ? (byte)1 : (byte)0; } } } //---------------------------------------------------------------------------- diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index 4dd74b117a25d5..955f1253ee504c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -681,7 +681,7 @@ struct Agnostic_ResolveVirtualMethodResult bool returnValue; DWORDLONG devirtualizedMethod; bool isInstantiatingStub; - bool needsMethodContext; + bool wasArrayInterfaceDevirt; DWORDLONG exactContext; DWORD detail; Agnostic_CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index b7c59485f2af2e..9dbb8b8d6a40ff 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -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.needsMethodContext = info->needsMethodContext; + result.wasArrayInterfaceDevirt = info->wasArrayInterfaceDevirt; if (returnValue) { @@ -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, needsMethodContext-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}", + printf(", value returnValue-%s, devirtMethod-%016" PRIX64 ", instantiatingStub-%s, wasArrayInterfaceDevirt-%s, exactContext-%016" PRIX64 ", detail-%d, tokDvMeth{%s}, tokDvUnboxMeth{%s}", result.returnValue ? "true" : "false", result.devirtualizedMethod, result.isInstantiatingStub ? "true" : "false", - result.needsMethodContext ? "true" : "false", + result.wasArrayInterfaceDevirt ? "true" : "false", result.exactContext, result.detail, result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedMethod).c_str() : "???", @@ -3330,7 +3330,7 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod; info->isInstantiatingStub = result.isInstantiatingStub; - info->needsMethodContext = result.needsMethodContext; + info->wasArrayInterfaceDevirt = result.wasArrayInterfaceDevirt; info->exactContext = (CORINFO_CONTEXT_HANDLE) result.exactContext; info->detail = (CORINFO_DEVIRTUALIZATION_DETAIL) result.detail; if (result.returnValue) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 83e8321d2893ca..b892e1e6e86b1c 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8568,7 +8568,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->needsMethodContext = false; + info->wasArrayInterfaceDevirt = false; MethodDesc* pBaseMD = GetMethod(info->virtualMethod); MethodTable* pBaseMT = pBaseMD->GetMethodTable(); @@ -8812,13 +8812,13 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // info->isInstantiatingStub = pDevirtMD->IsInstantiatingStub(); info->exactContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD); - info->needsMethodContext = true; + info->wasArrayInterfaceDevirt = true; } else { info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->isInstantiatingStub = false; - info->needsMethodContext = false; + info->wasArrayInterfaceDevirt = false; } info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD; From 71ffa265f87dc1a10a857ec41934f6d543c00a41 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 27 Nov 2025 10:08:03 +0100 Subject: [PATCH 18/18] Feedback --- src/coreclr/jit/lower.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 475182ef812ec4..cfa301e9d9d18c 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -2834,10 +2834,12 @@ GenTree* Lowering::LowerCall(GenTree* node) call->ClearOtherRegs(); +#if HAS_FIXED_REGISTER_SET if ((call->gtCallType == CT_INDIRECT) && comp->opts.Tier0OptimizationEnabled()) { OptimizeCallIndirectTargetEvaluation(call); } +#endif LowerArgsForCall(call);