[RyuJit] Remove the gtCallCookie operand from calls

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

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

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;
 }
 
 //------------------------------------------------------------------------

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<CALL_COOKIE>;
+                    m_advance = &GenTreeUseEdgeIterator::AdvanceCall<CALL_ADDRESS>;
                 }
                 else
                 {
@@ -10911,17 +10921,6 @@ void GenTreeUseEdgeIterator::AdvanceCall()
             }
             FALLTHROUGH;
 
-        case CALL_COOKIE:
-            assert(call->gtCallType == CT_INDIRECT);
-
-            m_advance = &GenTreeUseEdgeIterator::AdvanceCall<CALL_ADDRESS>;
-            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);

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;

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

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());

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;
 }
 

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