From ba26707643df974a5bb77879562912b26ad8451a Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 14:24:02 +0900 Subject: [PATCH 01/12] Add IsDevirtualizationCandidate --- src/coreclr/jit/fginline.cpp | 4 ++-- src/coreclr/jit/gentree.cpp | 38 +++++++++++++++++++++++++++++++ src/coreclr/jit/gentree.h | 4 ++++ src/coreclr/jit/importercalls.cpp | 8 +++---- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index d66609dc788b4c..dc5146b6f1f6ad 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -573,7 +573,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorOperGet() == GT_CALL) { GenTreeCall* call = tree->AsCall(); - bool tryLateDevirt = call->IsVirtual() && (call->gtCallType == CT_USER_FUNC); + bool tryLateDevirt = call->IsDevirtualizationCandidate(m_compiler); #ifdef DEBUG tryLateDevirt = tryLateDevirt && (JitConfig.JitEnableLateDevirtualization() == 1); @@ -610,7 +610,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorimpDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context, isLateDevirtualization, explicitTailCall); - if (!call->IsVirtual()) + if (!call->IsDevirtualizationCandidate(m_compiler)) { assert(context != nullptr); CORINFO_CALL_INFO callInfo = {}; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 46c57705853a1f..f83426fb8c668f 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2417,6 +2417,44 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const return 0; } +//------------------------------------------------------------------------- +// IsDevirtualizationCandidate: Determine if this GT_CALL node is a devirtualization candidate. +// A call will be unmarked from devirtualization candidate if it +// is devirtualized. +// +// Arguments: +// compiler - the compiler instance so that we can call eeFindHelper +// +// Return Value: +// Returns true if this GT_CALL node is a devirtualization candidate. +// +bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const +{ + return (IsVirtual() && gtCallType == CT_USER_FUNC) || + (gtCallType == CT_INDIRECT && gtCallAddr->IsCall() && + gtCallAddr->AsCall()->IsVirtualFunctionPointerLookup(compiler)); +} + +//------------------------------------------------------------------------- +// IsVirtualFunctionPointerLookup: Determine if this GT_CALL node is a virtual function pointer lookup. +// +// Arguments: +// compiler - the compiler instance so that we can call eeFindHelper +// +// Return Value: +// Returns true if this GT_CALL node is a virtual function pointer lookup. +// +bool GenTreeCall::IsVirtualFunctionPointerLookup(Compiler* compiler) const +{ + return gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) || + gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT) +#ifdef FEATURE_READYTORUN + || gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR) || + gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_READYTORUN_GENERIC_HANDLE) +#endif // FEATURE_READYTORUN + ; +} + //------------------------------------------------------------------------- // IsHelperCall: Determine if this GT_CALL node is a specific helper call. // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index a1b67e3d63b253..1260ca001b18d3 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -5331,6 +5331,10 @@ struct GenTreeCall final : public GenTree { return (gtFlags & GTF_CALL_VIRT_KIND_MASK) == GTF_CALL_VIRT_VTABLE; } + + bool IsDevirtualizationCandidate(Compiler* compiler) const; + bool IsVirtualFunctionPointerLookup(Compiler* compiler) const; + bool IsInlineCandidate() const { return (gtFlags & GTF_CALL_INLINE_CANDIDATE) != 0; diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 3789d10cb59829..ad533784156d75 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -949,7 +949,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // See if we can devirt if we aren't probing. if (!probing && opts.OptimizationEnabled()) { - if (call->AsCall()->IsVirtual()) + if (call->AsCall()->IsDevirtualizationCandidate(this)) { // only true object pointers can be virtual assert(call->AsCall()->gtArgs.HasThisPointer() && @@ -965,7 +965,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // inlinees. rawILOffset); - const bool wasDevirtualized = !call->AsCall()->IsVirtual(); + const bool wasDevirtualized = !call->AsCall()->IsDevirtualizationCandidate(this); if (wasDevirtualized) { @@ -8020,9 +8020,9 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, assert(methodFlags != nullptr); assert(pContextHandle != nullptr); - // This should be a virtual vtable or virtual stub call. + // This should be a devirtualization candidate. // - assert(call->IsVirtual()); + assert(call->IsDevirtualizationCandidate(this)); assert(opts.OptimizationEnabled()); #if defined(DEBUG) From 228248dc92e69a71be69b042d4e1c1fd3124708f Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 14:35:43 +0900 Subject: [PATCH 02/12] Use GetMethodHandle instead of gtCallMethHnd --- src/coreclr/jit/fginline.cpp | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index dc5146b6f1f6ad..011297ee1ea3c8 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -530,6 +530,35 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorIsDevirtualizationCandidate(m_compiler)); + if (call->IsVirtual()) + { + return call->gtCallMethHnd; + } + else + { + assert(call->gtCallType == CT_INDIRECT); + assert(call->gtCallAddr->IsCall() && + call->gtCallAddr->AsCall()->IsVirtualFunctionPointerLookup(m_compiler)); + assert(call->gtCallAddr->AsCall()->gtArgs.CountArgs() == 3); + GenTree* methodInstNode = call->gtCallAddr->AsCall()->gtArgs.GetArgByIndex(2)->GetNode(); + switch (methodInstNode->OperGet()) + { + case GT_RUNTIMELOOKUP: + return methodInstNode->AsRuntimeLookup()->GetMethodHandle(); + case GT_CNS_INT: + return CORINFO_METHOD_HANDLE(methodInstNode->AsIntCon()->IconValue()); + default: + assert(!"Unexpected type in MethodInstHandle arg."); + return nullptr; + } + return nullptr; + } + } + //------------------------------------------------------------------------ // LateDevirtualization: re-examine calls after inlining to see if we // can do more devirtualization @@ -590,7 +619,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtCallMethHnd; + CORINFO_METHOD_HANDLE method = GetMethodHandle(call); unsigned methodFlags = 0; const bool isLateDevirtualization = true; const bool explicitTailCall = call->IsTailPrefixedCall(); From 46ec414a376d95219aa8eaa087b3bf3f679524c5 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:07:38 +0900 Subject: [PATCH 03/12] Introduce a well-known arg --- src/coreclr/jit/compiler.h | 3 ++ src/coreclr/jit/compiler.hpp | 57 ++++++++++++++++++++++++++++++++++++ src/coreclr/jit/fginline.cpp | 15 +++++----- src/coreclr/jit/gentree.cpp | 25 ++-------------- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/importer.cpp | 3 +- 6 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f8855bfc41ab2d..c3e8a7ed620756 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3219,6 +3219,9 @@ class Compiler GenTreeCall* gtNewHelperCallNode( unsigned helper, var_types type, GenTree* arg1 = nullptr, GenTree* arg2 = nullptr, GenTree* arg3 = nullptr); + + GenTreeCall* gtNewVirtualFunctionLookupHelperCallNode( + unsigned helper, var_types type, GenTree* thisTree, GenTree* clsHndTree = nullptr, GenTree* methHndTree = nullptr); GenTreeCall* gtNewRuntimeLookupHelperCallNode(CORINFO_RUNTIME_LOOKUP* pRuntimeLookup, GenTree* ctxTree, diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 1738810e149b4b..0ff64eca3c6eb6 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1579,6 +1579,63 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode( return result; } +/*****************************************************************************/ + +//------------------------------------------------------------------------------ +// gtNewHelperCallNode : Helper to create a call helper node. +// +// +// Arguments: +// helper - Call helper +// type - Type of the node +// thisTree - 'this' argument +// clsHndTree - Class handle argument +// methHndTree - Runtime method handle argument +// +// Return Value: +// New CT_HELPER node +// +inline GenTreeCall* Compiler::gtNewVirtualFunctionLookupHelperCallNode( + unsigned helper, var_types type, GenTree* thisTree, GenTree* clsHndTree, GenTree* methHndTree) +{ + + GenTreeCall* const result = gtNewCallNode(CT_HELPER, eeFindHelper(helper), type); + + if (!s_helperCallProperties.NoThrow((CorInfoHelpFunc)helper)) + { + result->gtFlags |= GTF_EXCEPT; + + if (s_helperCallProperties.AlwaysThrow((CorInfoHelpFunc)helper)) + { + setCallDoesNotReturn(result); + } + } +#if DEBUG + // Helper calls are never candidates. + + result->gtInlineObservation = InlineObservation::CALLSITE_IS_CALL_TO_HELPER; +#endif + + if (methHndTree != nullptr) + { + result->gtArgs.PushFront(this, NewCallArg::Primitive(methHndTree).WellKnown(WellKnownArg::RuntimeMethodHandle)); + result->gtFlags |= methHndTree->gtFlags & GTF_ALL_EFFECT; + } + + if (clsHndTree != nullptr) + { + result->gtArgs.PushFront(this, NewCallArg::Primitive(clsHndTree)); + result->gtFlags |= clsHndTree->gtFlags & GTF_ALL_EFFECT; + } + + assert(thisTree != nullptr); + + result->gtArgs.PushFront(this, NewCallArg::Primitive(thisTree).WellKnown(WellKnownArg::ThisPointer)); + result->gtFlags |= thisTree->gtFlags & GTF_ALL_EFFECT; + + return result; +} + //------------------------------------------------------------------------ // gtNewAllocObjNode: A little helper to create an object allocation node. // diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 011297ee1ea3c8..e846a7989720e6 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -541,18 +541,17 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtCallType == CT_INDIRECT); - assert(call->gtCallAddr->IsCall() && - call->gtCallAddr->AsCall()->IsVirtualFunctionPointerLookup(m_compiler)); - assert(call->gtCallAddr->AsCall()->gtArgs.CountArgs() == 3); - GenTree* methodInstNode = call->gtCallAddr->AsCall()->gtArgs.GetArgByIndex(2)->GetNode(); - switch (methodInstNode->OperGet()) + GenTree* runtimeMethHndNode = + call->gtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetNode(); + assert(runtimeMethHndNode != nullptr); + switch (runtimeMethHndNode->OperGet()) { case GT_RUNTIMELOOKUP: - return methodInstNode->AsRuntimeLookup()->GetMethodHandle(); + return runtimeMethHndNode->AsRuntimeLookup()->GetMethodHandle(); case GT_CNS_INT: - return CORINFO_METHOD_HANDLE(methodInstNode->AsIntCon()->IconValue()); + return CORINFO_METHOD_HANDLE(runtimeMethHndNode->AsIntCon()->IconValue()); default: - assert(!"Unexpected type in MethodInstHandle arg."); + assert(!"Unexpected type in RuntimeMethodHandle arg."); return nullptr; } return nullptr; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index f83426fb8c668f..a7afa4a7dbf01b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2431,28 +2431,7 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const { return (IsVirtual() && gtCallType == CT_USER_FUNC) || - (gtCallType == CT_INDIRECT && gtCallAddr->IsCall() && - gtCallAddr->AsCall()->IsVirtualFunctionPointerLookup(compiler)); -} - -//------------------------------------------------------------------------- -// IsVirtualFunctionPointerLookup: Determine if this GT_CALL node is a virtual function pointer lookup. -// -// Arguments: -// compiler - the compiler instance so that we can call eeFindHelper -// -// Return Value: -// Returns true if this GT_CALL node is a virtual function pointer lookup. -// -bool GenTreeCall::IsVirtualFunctionPointerLookup(Compiler* compiler) const -{ - return gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) || - gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT) -#ifdef FEATURE_READYTORUN - || gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_READYTORUN_VIRTUAL_FUNC_PTR) || - gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_READYTORUN_GENERIC_HANDLE) -#endif // FEATURE_READYTORUN - ; + (gtCallType == CT_INDIRECT && gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR)); } //------------------------------------------------------------------------- @@ -13236,6 +13215,8 @@ const char* Compiler::gtGetWellKnownArgNameForArgMsg(WellKnownArg arg) return "tail call"; case WellKnownArg::StackArrayLocal: return "&lcl arr"; + case WellKnownArg::RuntimeMethodHandle: + return "meth hnd"; default: return nullptr; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 1260ca001b18d3..fd2beb893a469a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4568,6 +4568,7 @@ enum class WellKnownArg : unsigned SwiftSelf, X86TailCallSpecialArg, StackArrayLocal, + RuntimeMethodHandle, }; #ifdef DEBUG @@ -5333,7 +5334,6 @@ struct GenTreeCall final : public GenTree } bool IsDevirtualizationCandidate(Compiler* compiler) const; - bool IsVirtualFunctionPointerLookup(Compiler* compiler) const; bool IsInlineCandidate() const { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index bf408c330d777d..d3e5de02155d24 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2821,7 +2821,8 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, // Call helper function. This gets the target address of the final destination callsite. // - call = gtNewHelperCallNode(CORINFO_HELP_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr, exactTypeDesc, exactMethodDesc); + call = gtNewVirtualFunctionLookupHelperCallNode(CORINFO_HELP_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr, + exactTypeDesc, exactMethodDesc); } assert(call != nullptr); From 1d0fb51223aaf125f4267cfb4238b0be038a3011 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:18:25 +0900 Subject: [PATCH 04/12] Adjust arg order --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 32 +++++++++++++++----------------- src/coreclr/jit/importer.cpp | 5 +++-- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c3e8a7ed620756..7a9d673be67554 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3221,7 +3221,7 @@ class Compiler unsigned helper, var_types type, GenTree* arg1 = nullptr, GenTree* arg2 = nullptr, GenTree* arg3 = nullptr); GenTreeCall* gtNewVirtualFunctionLookupHelperCallNode( - unsigned helper, var_types type, GenTree* thisTree, GenTree* clsHndTree = nullptr, GenTree* methHndTree = nullptr); + unsigned helper, var_types type, GenTree* thisPtr, GenTree* methHnd, GenTree* clsHnd = nullptr); GenTreeCall* gtNewRuntimeLookupHelperCallNode(CORINFO_RUNTIME_LOOKUP* pRuntimeLookup, GenTree* ctxTree, diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 0ff64eca3c6eb6..069f33e6729947 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1586,17 +1586,17 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode( // // // Arguments: -// helper - Call helper -// type - Type of the node -// thisTree - 'this' argument -// clsHndTree - Class handle argument -// methHndTree - Runtime method handle argument +// helper - Call helper +// type - Type of the node +// thisPtr - 'this' argument +// methHnd - Runtime method handle argument +// clsHnd - Class handle argument // // Return Value: // New CT_HELPER node // inline GenTreeCall* Compiler::gtNewVirtualFunctionLookupHelperCallNode( - unsigned helper, var_types type, GenTree* thisTree, GenTree* clsHndTree, GenTree* methHndTree) + unsigned helper, var_types type, GenTree* thisPtr, GenTree* methHnd, GenTree* clsHnd) { GenTreeCall* const result = gtNewCallNode(CT_HELPER, eeFindHelper(helper), type); @@ -1616,22 +1616,20 @@ inline GenTreeCall* Compiler::gtNewVirtualFunctionLookupHelperCallNode( result->gtInlineObservation = InlineObservation::CALLSITE_IS_CALL_TO_HELPER; #endif - if (methHndTree != nullptr) - { - result->gtArgs.PushFront(this, NewCallArg::Primitive(methHndTree).WellKnown(WellKnownArg::RuntimeMethodHandle)); - result->gtFlags |= methHndTree->gtFlags & GTF_ALL_EFFECT; - } + assert(methHnd != nullptr); + result->gtArgs.PushFront(this, NewCallArg::Primitive(methHnd).WellKnown(WellKnownArg::RuntimeMethodHandle)); + result->gtFlags |= methHnd->gtFlags & GTF_ALL_EFFECT; - if (clsHndTree != nullptr) + if (clsHnd != nullptr) { - result->gtArgs.PushFront(this, NewCallArg::Primitive(clsHndTree)); - result->gtFlags |= clsHndTree->gtFlags & GTF_ALL_EFFECT; + result->gtArgs.PushFront(this, NewCallArg::Primitive(clsHnd)); + result->gtFlags |= clsHnd->gtFlags & GTF_ALL_EFFECT; } - assert(thisTree != nullptr); + assert(thisPtr != nullptr); - result->gtArgs.PushFront(this, NewCallArg::Primitive(thisTree).WellKnown(WellKnownArg::ThisPointer)); - result->gtFlags |= thisTree->gtFlags & GTF_ALL_EFFECT; + result->gtArgs.PushFront(this, NewCallArg::Primitive(thisPtr).WellKnown(WellKnownArg::ThisPointer)); + result->gtFlags |= thisPtr->gtFlags & GTF_ALL_EFFECT; return result; } diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index d3e5de02155d24..d0874ca2c85a9f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2780,7 +2780,8 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, { GenTree* runtimeMethodHandle = impLookupToTree(pResolvedToken, &pCallInfo->codePointerLookup, GTF_ICON_METHOD_HDL, pCallInfo->hMethod); - call = gtNewHelperCallNode(CORINFO_HELP_GVMLOOKUP_FOR_SLOT, TYP_I_IMPL, thisPtr, runtimeMethodHandle); + call = gtNewVirtualFunctionLookupHelperCallNode(CORINFO_HELP_GVMLOOKUP_FOR_SLOT, TYP_I_IMPL, thisPtr, + runtimeMethodHandle); } #ifdef FEATURE_READYTORUN @@ -2822,7 +2823,7 @@ GenTree* Compiler::impImportLdvirtftn(GenTree* thisPtr, // Call helper function. This gets the target address of the final destination callsite. // call = gtNewVirtualFunctionLookupHelperCallNode(CORINFO_HELP_VIRTUAL_FUNC_PTR, TYP_I_IMPL, thisPtr, - exactTypeDesc, exactMethodDesc); + exactMethodDesc, exactTypeDesc); } assert(call != nullptr); From 3e299c659bada77df61d5184a56ca72f7662eb5a Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:18:45 +0900 Subject: [PATCH 05/12] Nit --- src/coreclr/jit/compiler.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 069f33e6729947..0d5c1da57eecc7 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -1598,7 +1598,6 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode( inline GenTreeCall* Compiler::gtNewVirtualFunctionLookupHelperCallNode( unsigned helper, var_types type, GenTree* thisPtr, GenTree* methHnd, GenTree* clsHnd) { - GenTreeCall* const result = gtNewCallNode(CT_HELPER, eeFindHelper(helper), type); if (!s_helperCallProperties.NoThrow((CorInfoHelpFunc)helper)) From fb4099172bb92c19975f3f83c176e51cf4d62daf Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:21:36 +0900 Subject: [PATCH 06/12] Add NAOT helper as well --- src/coreclr/jit/gentree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index a7afa4a7dbf01b..65e8ded06ff5fc 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2431,7 +2431,8 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const { return (IsVirtual() && gtCallType == CT_USER_FUNC) || - (gtCallType == CT_INDIRECT && gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR)); + (gtCallType == CT_INDIRECT && (gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) || + gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT))); } //------------------------------------------------------------------------- From a1d14b9c1149aa21eab9f6dad725b00a5cb388f9 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:22:33 +0900 Subject: [PATCH 07/12] Add an assertion --- src/coreclr/jit/fginline.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index e846a7989720e6..8554181f8b8490 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -536,6 +536,7 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorIsDevirtualizationCandidate(m_compiler)); if (call->IsVirtual()) { + assert(call->gtCallType == CT_USER_FUNC); return call->gtCallMethHnd; } else From 20a8940c4c2a5202c150d5bc43de8260cf85ee8e Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 15:35:36 +0900 Subject: [PATCH 08/12] JIT format --- src/coreclr/jit/fginline.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 8554181f8b8490..efe62ec4ddf130 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -530,7 +530,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorIsDevirtualizationCandidate(m_compiler)); From 5baf5a4bae3f80d9686f9ef67c84b1729278befe Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 18 Feb 2025 16:28:06 +0900 Subject: [PATCH 09/12] Oops --- src/coreclr/jit/morph.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 43b2d527c14795..a8cf125d4b11f4 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -736,6 +736,8 @@ const char* getWellKnownArgName(WellKnownArg arg) return "X86TailCallSpecialArg"; case WellKnownArg::StackArrayLocal: return "StackArrayLocal"; + case WellKnownArg::RuntimeMethodHandle: + return "RuntimeMethodHandle"; } return "N/A"; From 3e8ea8dd9de4de904cacb5c5cb86393732c84f2f Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 3 Mar 2025 22:51:45 +0900 Subject: [PATCH 10/12] Polish the implementation --- src/coreclr/inc/corinfo.h | 6 +- src/coreclr/jit/gentree.cpp | 4 +- src/coreclr/jit/importercalls.cpp | 87 +++++++++++-------- src/coreclr/jit/inline.def | 2 - .../tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../tools/Common/JitInterface/CorInfoTypes.cs | 8 +- .../tools/superpmi/superpmi-shared/agnostic.h | 2 +- .../superpmi-shared/methodcontext.cpp | 18 ++-- src/coreclr/vm/jitinterface.cpp | 32 +++++-- 9 files changed, 93 insertions(+), 68 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 08970ec4102e9b..ffeca0ab1af6fb 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1512,8 +1512,8 @@ 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 - // (in which case the method handle and context will be a generic method) + // - hasGenericMethodHandleContext is set TRUE for cases where the method handle and context will be a generic method + // (for array interface method or generic virtual method devirtualization) // CORINFO_METHOD_HANDLE devirtualizedMethod; CORINFO_CONTEXT_HANDLE exactContext; @@ -1521,7 +1521,7 @@ struct CORINFO_DEVIRTUALIZATION_INFO CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedMethod; CORINFO_RESOLVED_TOKEN resolvedTokenDevirtualizedUnboxedMethod; bool isInstantiatingStub; - bool wasArrayInterfaceDevirt; + bool hasGenericMethodHandleContext; }; //---------------------------------------------------------------------------- diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3029eebca17301..95d68016f28e53 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2318,8 +2318,8 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const { return (IsVirtual() && gtCallType == CT_USER_FUNC) || - (gtCallType == CT_INDIRECT && (gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR) || - gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_GVMLOOKUP_FOR_SLOT))); + // TODO: Support CORINFO_HELP_GVMLOOKUP_FOR_SLOT for NativeAOT + (gtCallType == CT_INDIRECT && gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR)); } //------------------------------------------------------------------------- diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8297b9e1bb0d6d..aa69acb0d84c11 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -185,12 +185,6 @@ var_types Compiler::impImportCall(OPCODE opcode, compInlineResult->NoteFatal(InlineObservation::CALLEE_HAS_MANAGED_VARARGS); return TYP_UNDEF; } - - if ((mflags & CORINFO_FLG_VIRTUAL) && (sig->sigInst.methInstCount != 0) && (opcode == CEE_CALLVIRT)) - { - compInlineResult->NoteFatal(InlineObservation::CALLEE_IS_GENERIC_VIRTUAL); - return TYP_UNDEF; - } } clsHnd = pResolvedToken->hClass; @@ -384,12 +378,6 @@ var_types Compiler::impImportCall(OPCODE opcode, case CORINFO_VIRTUALCALL_LDVIRTFTN: { - if (compIsForInlining()) - { - compInlineResult->NoteFatal(InlineObservation::CALLSITE_HAS_CALL_VIA_LDVIRTFTN); - return TYP_UNDEF; - } - assert(!(mflags & CORINFO_FLG_STATIC)); // can't call a static method assert(!(clsFlags & CORINFO_FLG_VALUECLASS)); // OK, We've been told to call via LDVIRTFTN, so just @@ -402,11 +390,20 @@ 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); @@ -415,10 +412,6 @@ var_types Compiler::impImportCall(OPCODE opcode, // 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); @@ -439,7 +432,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // Sine we are jumping over some code, check that its OK to skip that code assert((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG && (sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_NATIVEVARARG); - goto DONE; + goto DEVIRT; } case CORINFO_CALL: @@ -933,6 +926,8 @@ var_types Compiler::impImportCall(OPCODE opcode, } } +DEVIRT: + bool probing; probing = impConsiderCallProbe(call->AsCall(), rawILOffset); @@ -1232,7 +1227,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // If the call is virtual, record the inliner's context for possible use during late devirt inlining. // Also record the generics context if there is any. // - if (call->AsCall()->IsVirtual() && (call->AsCall()->gtCallType != CT_INDIRECT)) + if (call->AsCall()->IsDevirtualizationCandidate(this)) { JITDUMP("\nSaving generic context %p and inline context %p for call [%06u]\n", dspPtr(exactContextHnd), dspPtr(compInlineContext), dspTreeID(call->AsCall())); @@ -1449,8 +1444,7 @@ var_types Compiler::impImportCall(OPCODE opcode, varDsc->lvType = call->AsCall()->gtReturnType; } - // TODO-Bug: CHECK_SPILL_NONE here looks wrong. - impStoreToTemp(calliSlot, call, CHECK_SPILL_NONE); + impStoreToTemp(calliSlot, call, CHECK_SPILL_ALL); // impStoreToTemp can change src arg list and return type for call that returns struct. var_types type = genActualType(lvaTable[calliSlot].TypeGet()); call = gtNewLclvNode(calliSlot, type); @@ -7136,7 +7130,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, } addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, - clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt, + clsAttrs, likelyHood, dvInfo.hasGenericMethodHandleContext, dvInfo.isInstantiatingStub, originalContext); } @@ -7208,7 +7202,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, likelyContext = dvInfo.exactContext; likelyMethod = dvInfo.devirtualizedMethod; - arrayInterface = dvInfo.wasArrayInterfaceDevirt; + arrayInterface = dvInfo.hasGenericMethodHandleContext; instantiatingStub = dvInfo.isInstantiatingStub; } else @@ -8210,14 +8204,14 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, if (((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_CLASS) { - assert(!dvInfo.wasArrayInterfaceDevirt); + assert(!dvInfo.hasGenericMethodHandleContext); 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.hasGenericMethodHandleContext); assert(((size_t)exactContext & CORINFO_CONTEXTFLAGS_MASK) == CORINFO_CONTEXTFLAGS_METHOD); derivedClass = info.compCompHnd->getMethodClass(derivedMethod); } @@ -8238,17 +8232,17 @@ 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 for array interface or GVM devirt. // - assert(dvInfo.wasArrayInterfaceDevirt); + assert(dvInfo.hasGenericMethodHandleContext); // We don't expect NAOT to end up here, since it has Array - // and normal devirtualization. + // and normal devirtualization. For GVM devirt we don't support NAOT yet. // assert(!IsTargetAbi(CORINFO_NATIVEAOT_ABI)); // We don't expect R2R to end up here, since it does not (yet) support - // array interface devirtualization. + // array interface or GVM devirtualization. // assert(!opts.IsReadyToRun()); @@ -8354,14 +8348,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. @@ -8369,11 +8355,36 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // Note different embedding would be needed for NAOT/R2R, // but we have ruled those out above. // - GenTree* const instParam = - gtNewIconEmbHndNode(instantiatingStub, nullptr, GTF_ICON_METHOD_HDL, instantiatingStub); + GenTree* instParam = nullptr; + // See if this is a generic virtual method call. + if (call->gtCallType == CT_INDIRECT && call->gtCallAddr->IsHelperCall(this, CORINFO_HELP_VIRTUAL_FUNC_PTR)) + { + // We need to pass the RUNTIMELOOKUP helper call as the inst param arg + // instead of the instantiating stub if there is one. + // + GenTree* const methHndNode = + call->gtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetEarlyNode(); + if (methHndNode->OperIs(GT_RUNTIMELOOKUP)) + { + instParam = methHndNode; + } + } + // For cases where we don't have a RUNTIMELOOKUP helper call, we pass the instantiating stub. + if (instParam == nullptr) + { + instParam = gtNewIconEmbHndNode(instantiatingStub, nullptr, GTF_ICON_METHOD_HDL, instantiatingStub); + } 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/jit/inline.def b/src/coreclr/jit/inline.def index 2b045ad5d20009..91ad0a5f5f5596 100644 --- a/src/coreclr/jit/inline.def +++ b/src/coreclr/jit/inline.def @@ -39,7 +39,6 @@ INLINE_OBSERVATION(HAS_NO_BODY, bool, "has no body", INLINE_OBSERVATION(HAS_NULL_FOR_LDELEM, bool, "has null pointer for ldelem", FATAL, CALLEE) INLINE_OBSERVATION(HAS_UNMANAGED_CALLCONV, bool, "has unmanaged calling convention", FATAL, CALLEE) INLINE_OBSERVATION(IS_ARRAY_METHOD, bool, "is array method", FATAL, CALLEE) -INLINE_OBSERVATION(IS_GENERIC_VIRTUAL, bool, "generic virtual", FATAL, CALLEE) INLINE_OBSERVATION(IS_JIT_NOINLINE, bool, "noinline per JitNoinline", FATAL, CALLEE) INLINE_OBSERVATION(IS_NOINLINE, bool, "noinline per IL/cached result", FATAL, CALLEE) INLINE_OBSERVATION(IS_SYNCHRONIZED, bool, "is synchronized", FATAL, CALLEE) @@ -134,7 +133,6 @@ INLINE_OBSERVATION(COMPILATION_ERROR, bool, "compilation error", INLINE_OBSERVATION(COMPILATION_FAILURE, bool, "failed to compile", FATAL, CALLSITE) INLINE_OBSERVATION(EXPLICIT_TAIL_PREFIX, bool, "explicit tail prefix", FATAL, CALLSITE) INLINE_OBSERVATION(GENERIC_DICTIONARY_LOOKUP, bool, "runtime dictionary lookup", FATAL, CALLSITE) -INLINE_OBSERVATION(HAS_CALL_VIA_LDVIRTFTN, bool, "call via ldvirtftn", FATAL, CALLSITE) INLINE_OBSERVATION(HAS_COMPLEX_HANDLE, bool, "complex handle access", FATAL, CALLSITE) INLINE_OBSERVATION(IMPLICIT_REC_TAIL_CALL, bool, "implicit recursive tail call", FATAL, CALLSITE) INLINE_OBSERVATION(IS_CALL_TO_HELPER, bool, "target is helper", FATAL, CALLSITE) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 70dde2b702ea3b..4daed1775bc60d 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1303,7 +1303,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->hasGenericMethodHandleContext = 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 08d7df78c946d8..291564e866e430 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1082,8 +1082,8 @@ 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 - // (in which case the method handle and context will be a generic method) + // - hasGenericMethodHandleContext is set TRUE for cases where the method handle and context will be a generic method + // (for array interface method or generic virtual method devirtualization) // public CORINFO_METHOD_STRUCT_* devirtualizedMethod; public CORINFO_CONTEXT_STRUCT* exactContext; @@ -1092,8 +1092,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 _hasGenericMethodHandleContext; + public bool hasGenericMethodHandleContext { get { return _hasGenericMethodHandleContext != 0; } set { _hasGenericMethodHandleContext = 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 18fd918a7a5cea..3db51f7dcef99d 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -653,7 +653,7 @@ struct Agnostic_ResolveVirtualMethodResult bool returnValue; DWORDLONG devirtualizedMethod; bool isInstantiatingStub; - bool wasArrayInterfaceDevirt; + bool hasGenericMethodHandleContext; 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 0621f2bcd1eb23..71a968993a58ab 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -3293,12 +3293,12 @@ void MethodContext::recResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info key.pResolvedTokenVirtualMethod = SpmiRecordsHelper::StoreAgnostic_CORINFO_RESOLVED_TOKEN(info->pResolvedTokenVirtualMethod, ResolveToken); Agnostic_ResolveVirtualMethodResult result; - result.returnValue = returnValue; - result.devirtualizedMethod = CastHandle(info->devirtualizedMethod); - result.isInstantiatingStub = info->isInstantiatingStub; - result.exactContext = CastHandle(info->exactContext); - result.detail = (DWORD) info->detail; - result.wasArrayInterfaceDevirt = info->wasArrayInterfaceDevirt; + result.returnValue = returnValue; + result.devirtualizedMethod = CastHandle(info->devirtualizedMethod); + result.isInstantiatingStub = info->isInstantiatingStub; + result.exactContext = CastHandle(info->exactContext); + result.detail = (DWORD)info->detail; + result.hasGenericMethodHandleContext = info->hasGenericMethodHandleContext; if (returnValue) { @@ -3323,11 +3323,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, hasGenericMethodHandleContext-%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.hasGenericMethodHandleContext ? "true" : "false", result.exactContext, result.detail, result.returnValue ? SpmiDumpHelper::DumpAgnostic_CORINFO_RESOLVED_TOKEN(result.resolvedTokenDevirtualizedMethod).c_str() : "???", @@ -3352,7 +3352,7 @@ bool MethodContext::repResolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO * info info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) result.devirtualizedMethod; info->isInstantiatingStub = result.isInstantiatingStub; - info->wasArrayInterfaceDevirt = result.wasArrayInterfaceDevirt; + info->hasGenericMethodHandleContext = result.hasGenericMethodHandleContext; 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 87c0b89e593d04..1e248ca0e9644a 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7709,7 +7709,7 @@ CEEInfo::getMethodInfo( getMethodInfoHelper(cxt, methInfo, context); result = true; } - else if (!ftn->IsWrapperStub() && ftn->HasILHeader()) + else if (!ftn->IsUnboxingStub() && ftn->HasILHeader()) { COR_ILMETHOD_DECODER header(ftn->GetILHeader(), ftn->GetMDImport(), NULL); cxt.Header = &header; @@ -8578,7 +8578,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->hasGenericMethodHandleContext = false; MethodDesc* pBaseMD = GetMethod(info->virtualMethod); MethodTable* pBaseMT = pBaseMD->GetMethodTable(); @@ -8586,9 +8586,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // Method better be from a fully loaded class _ASSERTE(pBaseMT->IsFullyLoaded()); - //@GENERICS: shouldn't be doing this for instantiated methods as they live elsewhere - _ASSERTE(!pBaseMD->HasMethodInstantiation()); - // Method better be virtual _ASSERTE(pBaseMD->IsVirtual()); @@ -8762,6 +8759,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) MethodTable* pApproxMT = pDevirtMD->GetMethodTable(); MethodTable* pExactMT = pApproxMT; bool isArray = false; + bool isGenericVirtual = false; if (pApproxMT->IsInterface()) { @@ -8779,22 +8777,40 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); } + // This is generic virtual method devirtualization. + if (!isArray && pBaseMD->HasMethodInstantiation()) + { + // If we're in a shared context we'll devirt to a shared + // generic method and won't be able to inline, so just bail. + // + if (pExactMT->IsSharedByGenericInstantiations()) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + + pDevirtMD = pDevirtMD->FindOrCreateAssociatedMethodDesc( + pDevirtMD, pExactMT, pExactMT->IsValueType() && !pDevirtMD->IsStatic(), pBaseMD->GetMethodInstantiation(), false); + + isGenericVirtual = true; + } + // Success! Pass back the results. // - if (isArray) + if (isArray || isGenericVirtual) { // Note if array devirtualization produced an instantiation stub // so jit can try and inline it. // info->isInstantiatingStub = pDevirtMD->IsInstantiatingStub(); info->exactContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD); - info->wasArrayInterfaceDevirt = true; + info->hasGenericMethodHandleContext = true; } else { info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->isInstantiatingStub = false; - info->wasArrayInterfaceDevirt = false; + info->hasGenericMethodHandleContext = false; } info->devirtualizedMethod = (CORINFO_METHOD_HANDLE) pDevirtMD; From 2b267c975cad52227d5dc011bbed219870ab821f Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 4 Mar 2025 00:15:58 +0900 Subject: [PATCH 11/12] Remove outdated assertions --- src/coreclr/jit/importercalls.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index aa69acb0d84c11..2685e814071c66 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -8082,7 +8082,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // if ((baseMethodAttribs & CORINFO_FLG_VIRTUAL) == 0) { - assert(call->IsVirtualStub()); assert(opts.IsReadyToRun()); JITDUMP("\nimpDevirtualizeCall: [R2R] base method not virtual, sorry\n"); return; @@ -8174,7 +8173,6 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, // It may or may not know enough to devirtualize... if (isInterface) { - assert(call->IsVirtualStub()); JITDUMP("--- base class is interface\n"); } From 3ef67bed7567919cfb3db7d61cb9e2bce85e64b2 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 4 Mar 2025 01:15:57 +0900 Subject: [PATCH 12/12] Block R2R --- src/coreclr/jit/gentree.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 95d68016f28e53..4da933bdb94691 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2318,8 +2318,9 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const bool GenTreeCall::IsDevirtualizationCandidate(Compiler* compiler) const { return (IsVirtual() && gtCallType == CT_USER_FUNC) || - // TODO: Support CORINFO_HELP_GVMLOOKUP_FOR_SLOT for NativeAOT - (gtCallType == CT_INDIRECT && gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR)); + // TODO: Support R2R and NativeAOT (CORINFO_HELP_GVMLOOKUP_FOR_SLOT) + (!compiler->opts.IsReadyToRun() && gtCallType == CT_INDIRECT && + gtCallAddr->IsHelperCall(compiler, CORINFO_HELP_VIRTUAL_FUNC_PTR)); } //-------------------------------------------------------------------------