From ad3e6ea8fd66dfe902940e10497308fc1c51317b Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Wed, 18 Feb 2026 22:34:39 +0300 Subject: [PATCH] Remove the 'gtCallCookie' operand from calls It is only used to carry a simple lookup from importer to morph. There is no need to carry this as IR. --- src/coreclr/jit/compiler.h | 14 +----- src/coreclr/jit/compiler.hpp | 9 +--- src/coreclr/jit/ee_il_dll.cpp | 33 +++++++++--- src/coreclr/jit/gentree.cpp | 83 +++++++++++++++---------------- src/coreclr/jit/gentree.h | 9 ++-- src/coreclr/jit/importer.cpp | 31 ------------ src/coreclr/jit/importercalls.cpp | 29 +++-------- src/coreclr/jit/lower.cpp | 9 ---- src/coreclr/jit/morph.cpp | 8 ++- 9 files changed, 84 insertions(+), 141 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e8052fa1722ce5..6d63894fa8f8ea 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3021,6 +3021,7 @@ class Compiler GenTreeFlags gtTokenToIconFlags(unsigned token); + GenTree* gtNewIconEmbHndNode(CORINFO_CONST_LOOKUP* pLookup, GenTreeFlags flags, void* compileTimeHandle); GenTree* gtNewIconEmbHndNode(void* value, void* pValue, GenTreeFlags flags, void* compileTimeHandle); GenTree* gtNewIconEmbScpHndNode(CORINFO_MODULE_HANDLE scpHnd); @@ -4881,8 +4882,6 @@ class Compiler CORINFO_LOOKUP* pLookup, void* compileTimeHandle); - GenTree* impReadyToRunLookupToTree(CORINFO_CONST_LOOKUP* pLookup, GenTreeFlags flags, void* compileTimeHandle); - GenTreeCall* impReadyToRunHelperToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, CorInfoHelpFunc helper, var_types type, @@ -8814,7 +8813,7 @@ class Compiler // Get the offset of a MDArray's lower bound for a given dimension. static unsigned eeGetMDArrayLowerBoundOffset(unsigned rank, unsigned dimension); - GenTree* eeGetPInvokeCookie(CORINFO_SIG_INFO* szMetaSig); + CORINFO_CONST_LOOKUP eeConvertToLookup(void* value, void* pValue); // Returns the page size for the target machine as reported by the EE. target_size_t eeGetPageSize() @@ -12342,15 +12341,6 @@ class GenTreeVisitor if (call->gtCallType == CT_INDIRECT) { - if (!call->IsVirtualStub() && (call->gtCallCookie != nullptr)) - { - result = WalkTree(&call->gtCallCookie, call); - if (result == fgWalkResult::WALK_ABORT) - { - return result; - } - } - result = WalkTree(&call->gtCallAddr, call); if (result == fgWalkResult::WALK_ABORT) { diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index f994d51f170a1e..b9a365908e172f 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4496,14 +4496,7 @@ GenTree::VisitResult GenTree::VisitOperands(TVisitor visitor) if (call->gtCallType == CT_INDIRECT) { - if (!call->IsVirtualStub() && (call->gtCallCookie != nullptr)) - { - RETURN_IF_ABORT(visitor(call->gtCallCookie)); - } - if (call->gtCallAddr != nullptr) - { - RETURN_IF_ABORT(visitor(call->gtCallAddr)); - } + RETURN_IF_ABORT(visitor(call->gtCallAddr)); } if (call->gtControlExpr != nullptr) { diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index c5f591be04c7a7..e75f9b521d6fc1 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -507,15 +507,32 @@ unsigned Compiler::eeGetArgSizeAlignment(var_types type, bool isFloatHfa) } } -/*****************************************************************************/ - -GenTree* Compiler::eeGetPInvokeCookie(CORINFO_SIG_INFO* szMetaSig) +//------------------------------------------------------------------------ +// eeConvertToLookup: Convert a tuple of "{ value, pValue }" to "CORINFO_CONST_LOOKUP". +// +// Arguments: +// value - The direct value (IAT_VALUE) +// pValue - The indirect value (IAT_PVALUE) +// +// Return Value: +// The lookup. +// +CORINFO_CONST_LOOKUP Compiler::eeConvertToLookup(void* value, void* pValue) { - void *cookie, *pCookie; - cookie = info.compCompHnd->GetCookieForPInvokeCalliSig(szMetaSig, &pCookie); - assert((cookie == nullptr) != (pCookie == nullptr)); - - return gtNewIconEmbHndNode(cookie, pCookie, GTF_ICON_PINVKI_HDL, szMetaSig); + CORINFO_CONST_LOOKUP lookup; + if (value != nullptr) + { + assert(pValue == nullptr); + lookup.accessType = IAT_VALUE; + lookup.addr = value; + } + else + { + assert(pValue != nullptr); + lookup.accessType = IAT_PVALUE; + lookup.addr = pValue; + } + return lookup; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index cf4f69d8ca2376..036014e4baaaa0 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -6150,11 +6150,6 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) if (call->gtCallType == CT_INDIRECT) { - // pinvoke-calli cookie is a constant, or constant indirection - // or a non-tree if this is a managed call. - assert(call->IsVirtualStub() || (call->gtCallCookie == nullptr) || - call->gtCallCookie->OperIs(GT_CNS_INT, GT_IND)); - GenTree* indirect = call->gtCallAddr; lvl2 = gtSetEvalOrder(indirect); @@ -6977,18 +6972,10 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse) *pUse = &call->gtControlExpr; return true; } - if (call->gtCallType == CT_INDIRECT) + if ((call->gtCallType == CT_INDIRECT) && (operand == call->gtCallAddr)) { - if (!call->IsVirtualStub() && (operand == call->gtCallCookie)) - { - *pUse = &call->gtCallCookie; - return true; - } - if (operand == call->gtCallAddr) - { - *pUse = &call->gtCallAddr; - return true; - } + *pUse = &call->gtCallAddr; + return true; } for (CallArg& arg : call->gtArgs.Args()) { @@ -7964,6 +7951,36 @@ GenTree* Compiler::gtNewIndOfIconHandleNode(var_types indType, size_t addr, GenT return indNode; } +//------------------------------------------------------------------------ +// gtNewIconEmbHndNode: Create a tree that computes a constant lookup. +// +// Arguments: +// pLookup - The lookup +// handleKind - The handle kind of the computed value +// compileTimeHandle - The compile-time handle of the computed value +// +// Return Value: +// "CNS_INT" or "IND(CNS_INT)" that computes "pLookup". +// +GenTree* Compiler::gtNewIconEmbHndNode(CORINFO_CONST_LOOKUP* pLookup, GenTreeFlags handleKind, void* compileTimeHandle) +{ + CORINFO_GENERIC_HANDLE handle = nullptr; + void* pIndirection = nullptr; + assert(pLookup->accessType != IAT_PPVALUE && pLookup->accessType != IAT_RELPVALUE); + + if (pLookup->accessType == IAT_VALUE) + { + handle = pLookup->handle; + } + else if (pLookup->accessType == IAT_PVALUE) + { + pIndirection = pLookup->addr; + } + + GenTree* addr = gtNewIconEmbHndNode(handle, pIndirection, handleKind, compileTimeHandle); + return addr; +} + /***************************************************************************** * * Allocates a integer constant entry that represents a HANDLE to something. @@ -8002,13 +8019,13 @@ GenTree* Compiler::gtNewIconEmbHndNode(void* value, void* pValue, GenTreeFlags i iconNode->gtCompileTimeHandle = (size_t)compileTimeHandle; #ifdef DEBUG - if (iconFlags == GTF_ICON_FTN_ADDR) + if (iconNode->IsIconHandle(GTF_ICON_CLASS_HDL, GTF_ICON_METHOD_HDL, GTF_ICON_FTN_ADDR)) { iconNode->gtTargetHandle = (size_t)compileTimeHandle; } - if (iconFlags == GTF_ICON_OBJ_HDL) + if (iconNode->IsIconHandle(GTF_ICON_OBJ_HDL)) { - iconNode->gtTargetHandle = (size_t)value; + iconNode->gtTargetHandle = (value != nullptr) ? (size_t)value : (size_t)pValue; } #endif return handleNode; @@ -10167,15 +10184,8 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree) /* Copy the union */ if (tree->gtCallType == CT_INDIRECT) { - if (tree->IsVirtualStub()) - { - copy->gtCallCookie = tree->gtCallCookie; - } - else - { - copy->gtCallCookie = tree->gtCallCookie ? gtCloneExpr(tree->gtCallCookie) : nullptr; - } - copy->gtCallAddr = tree->gtCallAddr ? gtCloneExpr(tree->gtCallAddr) : nullptr; + copy->gtCallCookie = tree->gtCallCookie; + copy->gtCallAddr = gtCloneExpr(tree->gtCallAddr); } else { @@ -10895,7 +10905,7 @@ void GenTreeUseEdgeIterator::AdvanceCall() { if (call->gtCallType == CT_INDIRECT) { - m_advance = &GenTreeUseEdgeIterator::AdvanceCall; + m_advance = &GenTreeUseEdgeIterator::AdvanceCall; } else { @@ -10911,17 +10921,6 @@ void GenTreeUseEdgeIterator::AdvanceCall() } FALLTHROUGH; - case CALL_COOKIE: - assert(call->gtCallType == CT_INDIRECT); - - m_advance = &GenTreeUseEdgeIterator::AdvanceCall; - if (!call->IsVirtualStub() && (call->gtCallCookie != nullptr)) - { - m_edge = &call->gtCallCookie; - return; - } - FALLTHROUGH; - case CALL_ADDRESS: assert(call->gtCallType == CT_INDIRECT); @@ -13719,10 +13718,6 @@ void Compiler::gtDispLIRNode(GenTree* node, const char* prefixMsg /* = nullptr * { displayOperand(operand, "control expr", operandArc, indentStack, prefixIndent); } - else if (operand == call->gtCallCookie) - { - displayOperand(operand, "cookie", operandArc, indentStack, prefixIndent); - } else { CallArg* curArg = call->gtArgs.FindByNode(operand); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index a0da22d21572df..b1e84c28e55aee 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3033,9 +3033,8 @@ class GenTreeUseEdgeIterator final CALL_ARGS = 0, CALL_LATE_ARGS = 1, CALL_CONTROL_EXPR = 2, - CALL_COOKIE = 3, - CALL_ADDRESS = 4, - CALL_TERMINAL = 5, + CALL_ADDRESS = 3, + CALL_TERMINAL = 4, }; typedef void (GenTreeUseEdgeIterator::*AdvanceFn)(); @@ -5818,8 +5817,8 @@ struct GenTreeCall final : public GenTree union { - // only used for CALLI unmanaged calls (CT_INDIRECT) - GenTree* gtCallCookie; + // The serialized CALLI unmanaged call (CT_INDIRECT) cookie; reified into argument IR in morph + CORINFO_CONST_LOOKUP* gtCallCookie; // gtInlineCandidateInfo is only used when inlining methods InlineCandidateInfo* gtInlineCandidateInfo; diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 70b2aeb3a46c14..c674ecf1d01faa 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -1412,37 +1412,6 @@ GenTree* Compiler::impLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken, } #ifdef FEATURE_READYTORUN -GenTree* Compiler::impReadyToRunLookupToTree(CORINFO_CONST_LOOKUP* pLookup, - GenTreeFlags handleFlags, - void* compileTimeHandle) -{ - CORINFO_GENERIC_HANDLE handle = nullptr; - void* pIndirection = nullptr; - assert(pLookup->accessType != IAT_PPVALUE && pLookup->accessType != IAT_RELPVALUE); - - if (pLookup->accessType == IAT_VALUE) - { - handle = pLookup->handle; - } - else if (pLookup->accessType == IAT_PVALUE) - { - pIndirection = pLookup->addr; - } - GenTree* addr = gtNewIconEmbHndNode(handle, pIndirection, handleFlags, compileTimeHandle); -#ifdef DEBUG - assert((handleFlags == GTF_ICON_CLASS_HDL) || (handleFlags == GTF_ICON_METHOD_HDL)); - if (handle != nullptr) - { - addr->AsIntCon()->gtTargetHandle = (size_t)compileTimeHandle; - } - else - { - addr->gtGetOp1()->AsIntCon()->gtTargetHandle = (size_t)compileTimeHandle; - } -#endif // DEBUG - return addr; -} - //------------------------------------------------------------------------ // impIsCastHelperEligibleForClassProbe: Checks whether a tree is a cast helper eligible to // to be profiled and then optimized with PGO data diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 62b79e697788ff..bd19f8f3b9e43a 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -691,25 +691,10 @@ var_types Compiler::impImportCall(OPCODE opcode, else if ((opcode == CEE_CALLI) && ((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_DEFAULT) && ((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG)) { - GenTree* cookie = eeGetPInvokeCookie(sig); - - // This cookie is required to be either a simple GT_CNS_INT or - // an indirection of a GT_CNS_INT - // - GenTree* cookieConst = cookie; - if (cookie->OperIs(GT_IND)) - { - cookieConst = cookie->AsOp()->gtOp1; - } - assert(cookieConst->OperIs(GT_CNS_INT)); - - // Setting GTF_DONT_CSE on the GT_CNS_INT as well as on the GT_IND (if it exists) will ensure that - // we won't allow this tree to participate in any CSE logic - // - cookie->gtFlags |= GTF_DONT_CSE; - cookieConst->gtFlags |= GTF_DONT_CSE; - - call->AsCall()->gtCallCookie = cookie; + void* pCookie; + void* cookie = info.compCompHnd->GetCookieForPInvokeCalliSig(sig, &pCookie); + CORINFO_CONST_LOOKUP cookieLookup = eeConvertToLookup(cookie, pCookie); + call->AsCall()->gtCallCookie = new (getAllocator(CMK_ASTNode)) CORINFO_CONST_LOOKUP(cookieLookup); if (canTailCall) { @@ -800,8 +785,8 @@ var_types Compiler::impImportCall(OPCODE opcode, #ifdef FEATURE_READYTORUN if (IsAot()) { - instParam = impReadyToRunLookupToTree(&callInfo->instParamLookup, GTF_ICON_METHOD_HDL, - exactMethodHandle); + instParam = + gtNewIconEmbHndNode(&callInfo->instParamLookup, GTF_ICON_METHOD_HDL, exactMethodHandle); if (instParam == nullptr) { assert(compDonotInline()); @@ -851,7 +836,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (IsAot()) { instParam = - impReadyToRunLookupToTree(&callInfo->instParamLookup, GTF_ICON_CLASS_HDL, exactClassHandle); + gtNewIconEmbHndNode(&callInfo->instParamLookup, GTF_ICON_CLASS_HDL, exactClassHandle); if (instParam == nullptr) { assert(compDonotInline()); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 656b58ee84fbf7..a45bdd41c921a7 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -6588,18 +6588,9 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call) GenTree* Lowering::LowerIndirectNonvirtCall(GenTreeCall* call) { -#ifdef TARGET_X86 - if (call->gtCallCookie != nullptr) - { - NYI_X86("Morphing indirect non-virtual call with non-standard args"); - } -#endif - // Indirect cookie calls gets transformed by fgMorphArgs as indirect call with non-standard args. // Hence we should never see this type of call in lower. - noway_assert(call->gtCallCookie == nullptr); - return nullptr; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 5d8075ec7236d3..9c5404b9c9eac6 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1773,10 +1773,14 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call { assert(!call->IsUnmanaged()); - GenTree* arg = call->gtCallCookie; - noway_assert(arg != nullptr); + GenTree* arg = comp->gtNewIconEmbHndNode(call->gtCallCookie, GTF_ICON_PINVKI_HDL, nullptr); call->gtCallCookie = nullptr; + // TODO: this is preserving existing behavior, but do we actually need these NO_CSEs? + GenTree* argConst = arg->OperIs(GT_IND) ? arg->AsIndir()->Addr() : arg; + argConst->gtFlags |= GTF_DONT_CSE; + arg->gtFlags |= GTF_DONT_CSE; + // All architectures pass the cookie in a register. InsertAfterThisOrFirst(comp, NewCallArg::Primitive(arg).WellKnown(WellKnownArg::PInvokeCookie)); // put destination into R10/EAX