Implement convertPInvokeCalliToCall for CoreCLR

docs/design/coreclr/botr/clr-abi.md

@@ -148,8 +148,6 @@ ARM64-only: When a method returns a structure that is larger than 16 bytes the c
 
 *Stub dispatch* - when a virtual call uses a VSD stub, rather than back-patching the calling code (or disassembling it), the JIT must place the address of the stub used to load the call target, the "stub indirection cell", in (x86) `EAX` / (AMD64) `R11` / (ARM) `R12` / (ARM64) `R11`. In the JIT, this is encapsulated in the `VirtualStubParamInfo` class.
 
-*Calli Pinvoke* - The VM wants the address of the PInvoke in (AMD64) `R10` / (ARM) `R12` / (ARM64) `R14` (In the JIT: `REG_PINVOKE_TARGET_PARAM`), and the signature (the pinvoke cookie) in (AMD64) `R11` / (ARM) `R4` / (ARM64) `R15` (in the JIT: `REG_PINVOKE_COOKIE_PARAM`).
-
 *Normal PInvoke* - The VM shares IL stubs based on signatures, but wants the right method to show up in call stack and exceptions, so the MethodDesc for the exact PInvoke is passed in the (x86) `EAX` / (AMD64) `R10` / (ARM, ARM64) `R12` (in the JIT: `REG_SECRET_STUB_PARAM`). Then in the IL stub, when the JIT gets `CORJIT_FLG_PUBLISH_SECRET_PARAM`, it must move the register into a compiler temp. The value is returned for the intrinsic `NI_System_StubHelpers_GetStubContext`.
 
 ## Small primitive returns

docs/design/coreclr/botr/guide-for-porting.md

@@ -400,10 +400,8 @@ Here is an annotated list of the stubs implemented for Unix on Arm64.
         for ReadyToRun pre-compiled pinvoke calls, so that they do not cause GC
         starvation
 
-    15. `VarargPInvokeStub`/ `GenericPInvokeCalliHelper` Used to support calli
-        pinvokes. It is expected that C\# 8.0 will increase use of this feature.
-        Today use of this feature on Unix requires hand-written IL. On Windows
-        this feature is commonly used by C++/CLI
+    15. `VarargPInvokeStub` – Used to support vararg pinvokes, which are commonly
+        used by C++/CLI. Not necessary for non-Windows platforms at this time.
 
 #### cgencpu.h
 

src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs

@@ -2180,6 +2180,9 @@ internal static partial class StubHelpers
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_ThrowInteropParamException")]
         internal static partial void ThrowInteropParamException(int resID, int paramIdx);
 
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_ThrowInteropException")]
+        internal static partial void ThrowInteropException(int exceptionKind, int resID);
+
         internal static IntPtr AddToCleanupList(ref CleanupWorkListElement? pCleanupWorkList, SafeHandle handle)
         {
             SafeHandleCleanupWorkListElement element = new SafeHandleCleanupWorkListElement(handle);

src/coreclr/inc/corinfo.h

@@ -481,7 +481,6 @@ enum CorInfoHelpFunc
 
     /* Miscellaneous */
 
-    CORINFO_HELP_PINVOKE_CALLI,         // Indirect pinvoke call
     CORINFO_HELP_TAILCALL,              // Perform a tail call
 
     CORINFO_HELP_GETCURRENTMANAGEDTHREADID,
@@ -3363,12 +3362,6 @@ class ICorDynamicInfo : public ICorStaticInfo
             CORINFO_CONST_LOOKUP *  pLookup
             ) = 0;
 
-    // Generate a cookie based on the signature to pass to CORINFO_HELP_PINVOKE_CALLI
-    virtual void* GetCookieForPInvokeCalliSig(
-            CORINFO_SIG_INFO*   szMetaSig,
-            void**              ppIndirection = NULL
-            ) = 0;
-
     // Generate a cookie based on the signature to pass to INTOP_CALLI in the interpreter.
     virtual void* GetCookieForInterpreterCalliSig(
             CORINFO_SIG_INFO*   szMetaSig) = 0;
@@ -3512,6 +3505,7 @@ class ICorDynamicInfo : public ICorStaticInfo
         ) = 0;
 
     // Optionally, convert calli to regular method call. This is for PInvoke argument marshalling.
+    // On success, pResolvedToken->hMethod is set to the method that should be called instead.
     virtual bool convertPInvokeCalliToCall(
             CORINFO_RESOLVED_TOKEN *    pResolvedToken,
             bool                        fMustConvert

src/coreclr/inc/icorjitinfoimpl_generated.h

@@ -597,10 +597,6 @@ void getAddressOfPInvokeTarget(
           CORINFO_METHOD_HANDLE method,
           CORINFO_CONST_LOOKUP* pLookup) override;
 
-void* GetCookieForPInvokeCalliSig(
-          CORINFO_SIG_INFO* szMetaSig,
-          void** ppIndirection) override;
-
 void* GetCookieForInterpreterCalliSig(
           CORINFO_SIG_INFO* szMetaSig) override;
 

src/coreclr/inc/jiteeversionguid.h

@@ -37,11 +37,11 @@
 
 #include <minipal/guid.h>
 
-constexpr GUID JITEEVersionIdentifier = { /* 134b051a-e1ec-4f52-a1bf-f919908aa33c */
-    0x134b051a,
-    0xe1ec,
-    0x4f52,
-    {0xa1, 0xbf, 0xf9, 0x19, 0x90, 0x8a, 0xa3, 0x3c}
+constexpr GUID JITEEVersionIdentifier = { /* 311b47c4-f2ef-4500-887d-a493343940d7 */
+    0x311b47c4,
+    0xf2ef,
+    0x4500,
+    {0x88, 0x7d, 0xa4, 0x93, 0x34, 0x39, 0x40, 0xd7}
   };
 
 #endif // JIT_EE_VERSIONING_GUID_H

src/coreclr/inc/jithelpers.h

@@ -216,8 +216,6 @@
     DYNAMICJITHELPER(CORINFO_HELP_PROF_FCN_TAILCALL, JIT_ProfilerEnterLeaveTailcallStub, METHOD__NIL)
 
     // Miscellaneous
-    JITHELPER(CORINFO_HELP_PINVOKE_CALLI,       GenericPInvokeCalliHelper, METHOD__NIL)
-
 #if defined(TARGET_X86) && !defined(UNIX_X86_ABI)
     JITHELPER(CORINFO_HELP_TAILCALL,            JIT_TailCall,             METHOD__NIL)
 #else

src/coreclr/jit/ICorJitInfo_names_generated.h

@@ -148,7 +148,6 @@ DEF_CLR_API(embedFieldHandle)
 DEF_CLR_API(embedGenericHandle)
 DEF_CLR_API(getLocationOfThisType)
 DEF_CLR_API(getAddressOfPInvokeTarget)
-DEF_CLR_API(GetCookieForPInvokeCalliSig)
 DEF_CLR_API(GetCookieForInterpreterCalliSig)
 DEF_CLR_API(getJustMyCodeHandle)
 DEF_CLR_API(GetProfilingHandle)

src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp

@@ -1413,16 +1413,6 @@ void WrapICorJitInfo::getAddressOfPInvokeTarget(
     API_LEAVE(getAddressOfPInvokeTarget);
 }
 
-void* WrapICorJitInfo::GetCookieForPInvokeCalliSig(
-          CORINFO_SIG_INFO* szMetaSig,
-          void** ppIndirection)
-{
-    API_ENTER(GetCookieForPInvokeCalliSig);
-    void* temp = wrapHnd->GetCookieForPInvokeCalliSig(szMetaSig, ppIndirection);
-    API_LEAVE(GetCookieForPInvokeCalliSig);
-    return temp;
-}
-
 void* WrapICorJitInfo::GetCookieForInterpreterCalliSig(
           CORINFO_SIG_INFO* szMetaSig)
 {

src/coreclr/jit/compiler.h

@@ -9574,8 +9574,6 @@ class Compiler
     // Get the offset of a MDArray's lower bound for a given dimension.
     static unsigned eeGetMDArrayLowerBoundOffset(unsigned rank, unsigned dimension);
 
-    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()
     {

src/coreclr/jit/ee_il_dll.cpp

@@ -504,34 +504,6 @@ unsigned Compiler::eeGetArgSizeAlignment(var_types type, bool isFloatHfa)
     }
 }
 
-//------------------------------------------------------------------------
-// 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)
-{
-    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;
-}
-
 //------------------------------------------------------------------------
 // eeGetArrayDataOffset: Gets the offset of a SDArray's first element
 //

src/coreclr/jit/gentree.cpp

@@ -1607,14 +1607,6 @@ bool CallArgs::GetCustomRegister(Compiler* comp, CorInfoCallConvExtension cc, We
             *reg = comp->virtualStubParamInfo->GetReg();
             return true;
 
-        case WellKnownArg::PInvokeCookie:
-            *reg = REG_PINVOKE_COOKIE_PARAM;
-            return true;
-
-        case WellKnownArg::PInvokeTarget:
-            *reg = REG_PINVOKE_TARGET_PARAM;
-            return true;
-
         case WellKnownArg::R2RIndirectionCell:
             *reg = REG_R2R_INDIRECT_PARAM;
             return true;
@@ -2396,12 +2388,6 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const
         // R11 = Virtual stub param
         return 1;
     }
-    else if ((gtCallType == CT_INDIRECT) && (gtCallCookie != nullptr))
-    {
-        // R10 = PInvoke target param
-        // R11 = PInvoke cookie param
-        return 2;
-    }
     return 0;
 }
 
@@ -2619,18 +2605,6 @@ bool GenTreeCall::Equals(GenTreeCall* c1, GenTreeCall* c2)
             }
         }
     }
-    else if (!c1->IsVirtualStub())
-    {
-        if ((c1->gtCallCookie == nullptr) != (c2->gtCallCookie == nullptr))
-        {
-            return false;
-        }
-
-        if ((c1->gtCallCookie != nullptr) && !sameLookup(*c1->gtCallCookie, *c2->gtCallCookie))
-        {
-            return false;
-        }
-    }
 
     auto sameArgMetadata = [](CallArg* a1, CallArg* a2) {
         return (a1->GetSignatureType() == a2->GetSignatureType()) &&
@@ -10118,19 +10092,17 @@ GenTreeCall* Compiler::gtNewCallNode(gtCallTypes           callType,
     node->gtRetClsHnd     = nullptr;
     node->gtCallMoreFlags = GTF_CALL_M_EMPTY;
     INDEBUG(node->gtCallDebugFlags = GTF_CALL_MD_EMPTY);
-    node->gtInlineInfoCount = 0;
+    node->ClearInlineInfo();
 
     if (callType == CT_INDIRECT)
     {
-        node->gtCallCookie  = nullptr;
         node->gtCallMethHnd = NO_METHOD_HANDLE;
         node->gtControlExpr = (GenTree*)callHnd;
     }
     else
     {
         node->gtCallMethHnd = callHnd;
         node->gtControlExpr = nullptr;
-        node->ClearInlineInfo();
     }
     node->gtReturnType = type;
 
@@ -11741,15 +11713,15 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree)
     copy->gtStubCallStubAddr = tree->gtStubCallStubAddr;
 
     /* Copy the union */
+    copy->gtInlineCandidateInfo = tree->gtInlineCandidateInfo;
+
     if (tree->gtCallType == CT_INDIRECT)
     {
-        copy->gtCallCookie  = tree->gtCallCookie;
         copy->gtCallMethHnd = NO_METHOD_HANDLE;
     }
     else
     {
-        copy->gtCallMethHnd         = tree->gtCallMethHnd;
-        copy->gtInlineCandidateInfo = tree->gtInlineCandidateInfo;
+        copy->gtCallMethHnd = tree->gtCallMethHnd;
     }
 
     copy->gtInlineInfoCount          = tree->gtInlineInfoCount;

src/coreclr/jit/gentree.h

@@ -5875,9 +5875,6 @@ struct GenTreeCall final : public GenTree
 
     union
     {
-        // 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;
         // gtInlineCandidateInfoList is used when we have more than one GDV candidate

src/coreclr/jit/handlekinds.h

@@ -16,7 +16,6 @@ HANDLE_KIND(GTF_ICON_OBJ_HDL         , "object"                     , 0)
 HANDLE_KIND(GTF_ICON_CONST_PTR       , "const ptr"                  , HKF_INVARIANT) // pointer to immutable data, (e.g. IAT_PPVALUE)
 HANDLE_KIND(GTF_ICON_GLOBAL_PTR      , "global ptr"                 , 0)             // pointer to mutable data (e.g. from the VM state)
 HANDLE_KIND(GTF_ICON_VARG_HDL        , "vararg"                     , HKF_INVARIANT) // var arg cookie handle
-HANDLE_KIND(GTF_ICON_PINVKI_HDL      , "pinvoke"                    , 0)             // pinvoke calli handle
 HANDLE_KIND(GTF_ICON_TOKEN_HDL       , "token"                      , HKF_INVARIANT) // token handle (other than class, method or field)
 HANDLE_KIND(GTF_ICON_TLS_HDL         , "tls"                        , HKF_INVARIANT) // TLS ref with offset
 HANDLE_KIND(GTF_ICON_FTN_ADDR        , "ftn"                        , 0)             // function address

src/coreclr/jit/importercalls.cpp

@@ -230,13 +230,12 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
 
     if (opcode == CEE_CALLI)
     {
-        if (IsTargetAbi(CORINFO_NATIVEAOT_ABI))
+        if (info.compCompHnd->convertPInvokeCalliToCall(pResolvedToken, !impCanPInvokeInlineCallSite(compCurBB)))
         {
-            if (info.compCompHnd->convertPInvokeCalliToCall(pResolvedToken, !impCanPInvokeInlineCallSite(compCurBB)))
-            {
-                eeGetCallInfo(pResolvedToken, nullptr, CORINFO_CALLINFO_ALLOWINSTPARAM, callInfo);
-                return impImportCall(CEE_CALL, pResolvedToken, nullptr, nullptr, prefixFlags, callInfo, rawILOffset);
-            }
+            // The VM only fills in hMethod; derive the rest of the token from it.
+            pResolvedToken->hClass = info.compCompHnd->getMethodClass(pResolvedToken->hMethod);
+            eeGetCallInfo(pResolvedToken, nullptr, CORINFO_CALLINFO_ALLOWINSTPARAM, callInfo);
+            return impImportCall(CEE_CALL, pResolvedToken, nullptr, nullptr, prefixFlags, callInfo, rawILOffset);
         }
 
         /* Get the call site sig */
@@ -826,20 +825,6 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
 
         goto DONE;
     }
-    else if ((opcode == CEE_CALLI) && ((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_DEFAULT) &&
-             ((sig->callConv & CORINFO_CALLCONV_MASK) != CORINFO_CALLCONV_VARARG))
-    {
-        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)
-        {
-            canTailCall             = false;
-            szCanTailCallFailReason = "PInvoke calli";
-        }
-    }
 
     if (sig->isAsyncCall())
     {
@@ -7185,8 +7170,6 @@ void Compiler::impCheckForPInvokeCall(
         }
 
         unmanagedCallConv = info.compCompHnd->getUnmanagedCallConv(nullptr, sig, &suppressGCTransition);
-
-        assert(!call->gtCallCookie);
     }
 
     if (suppressGCTransition)
@@ -7209,13 +7192,11 @@ void Compiler::impCheckForPInvokeCall(
     }
     optNativeCallCount++;
 
-    if (methHnd == nullptr && (IsTargetAbi(CORINFO_NATIVEAOT_ABI) ||
-                               (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && !compIsForInlining())))
+    if (methHnd == nullptr)
     {
-        // PInvoke CALLI in NativeAOT ABI must be always inlined. Non-inlineable CALLI cases have been
-        // converted to regular method calls earlier using convertPInvokeCalliToCall.
-
-        // PInvoke CALLI in IL stubs must be inlined
+        // PInvoke CALLI must always be inlined. Call sites that cannot be inlined have been converted
+        // to a call to a marshalling stub earlier using convertPInvokeCalliToCall, or, in ReadyToRun,
+        // have aborted the compilation of this method.
     }
     else if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && IsReadyToRun())
     {
@@ -8402,17 +8383,6 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall*            call,
         return;
     }
 
-    // CT_INDIRECT calls may use the cookie, bail if so...
-    //
-    // If transforming these provides a benefit, we could save this off in the same way
-    // we save the stub address below.
-    if ((call->gtCallType == CT_INDIRECT) && (call->AsCall()->gtCallCookie != nullptr))
-    {
-        JITDUMP("NOT Marking call [%06u] as guarded devirtualization candidate -- CT_INDIRECT with cookie\n",
-                dspTreeID(call));
-        return;
-    }
-
 #ifdef DEBUG
 
     // See if disabled by range

src/coreclr/jit/lower.cpp

@@ -2044,7 +2044,10 @@ void Lowering::LowerSpecialCopyArgs(GenTreeCall* call)
         // which will be first in the list.
         // The this parameter is always passed in registers, so we can ignore it.
         unsigned argIndex = call->gtArgs.CountUserArgs() - 1;
-        assert(call->gtArgs.CountUserArgs() == m_compiler->info.compILargsCount);
+        // The arguments of the unmanaged call are the leading arguments of the IL stub, so the stub
+        // cannot have fewer of them. It can have more: an unmanaged CALLI stub takes the call target
+        // as an extra trailing argument that is not passed on to the unmanaged call.
+        assert(call->gtArgs.CountUserArgs() <= m_compiler->info.compILargsCount);
         bool checkForUnmanagedThisArg = call->GetUnmanagedCallConv() == CorInfoCallConvExtension::Thiscall;
         for (CallArg& arg : call->gtArgs.Args())
         {
@@ -2903,11 +2906,7 @@ GenTree* Lowering::LowerCall(GenTree* node)
                 {
                     controlExpr = LowerNonvirtPinvokeCall(call);
                 }
-                else if (call->gtCallType == CT_INDIRECT)
-                {
-                    controlExpr = LowerIndirectNonvirtCall(call);
-                }
-                else
+                else if (call->gtCallType != CT_INDIRECT)
                 {
                     controlExpr = LowerDirectCall(call);
                 }
@@ -6631,14 +6630,6 @@ void Lowering::OptimizeCallIndirectTargetEvaluation(GenTreeCall* call)
     DISPTREERANGE(BlockRange(), call);
 }
 
-GenTree* Lowering::LowerIndirectNonvirtCall(GenTreeCall* call)
-{
-    // 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;
-}
-
 //------------------------------------------------------------------------
 // CreateReturnTrapSeq: Create a tree to perform a "return trap", used in PInvoke
 // epilogs to invoke a GC under a condition. The return trap checks some global