From 025510fc974430c131138b7ccb9c1281dcbdc8b3 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 2 Dec 2021 10:16:39 +0200 Subject: [PATCH 1/8] Convert some old style intrinsics to NamedIntrinsic --- docs/design/coreclr/botr/clr-abi.md | 2 +- .../src/System/StubHelpers.cs | 2 + src/coreclr/inc/corinfo.h | 5 - src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/gentree.cpp | 208 ++++++------ src/coreclr/jit/gtlist.h | 2 +- src/coreclr/jit/importer.cpp | 306 ++++++++++-------- src/coreclr/jit/namedintrinsiclist.h | 5 + src/coreclr/jit/valuenum.cpp | 2 +- .../JitInterface/CorInfoImpl.Intrinsics.cs | 6 - .../tools/Common/JitInterface/CorInfoTypes.cs | 5 - src/coreclr/vm/corelib.cpp | 6 - src/coreclr/vm/ecalllist.h | 34 +- src/coreclr/vm/interpreter.cpp | 7 +- 15 files changed, 293 insertions(+), 308 deletions(-) diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index 1f65ca71bc9ce..55ad398e9bfe1 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -109,7 +109,7 @@ ARM64-only: When a method returns a structure that is larger than 16 bytes the c *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 `CORINFO_INTRINSIC_StubHelpers_GetStubContext`, and the address of that location is returned for `CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr`. +*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`, and the address of that location is returned for `NI_System_StubHelpers_GetStubContextAddr`. # PInvokes diff --git a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs index 41a85755b01d0..279d53816124a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs @@ -1349,6 +1349,7 @@ internal static void CheckStringLength(uint length) internal static extern IntPtr GetStubContext(); #if TARGET_64BIT + [Intrinsic] [MethodImpl(MethodImplOptions.InternalCall)] internal static extern IntPtr GetStubContextAddr(); #endif // TARGET_64BIT @@ -1363,6 +1364,7 @@ internal static void CheckStringLength(uint length) internal static extern void MulticastDebuggerTraceHelper(object o, int count); #endif + [Intrinsic] [MethodImplAttribute(MethodImplOptions.InternalCall)] internal static extern IntPtr NextCallReturnAddress(); } // class StubHelpers diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 4cc0c44b83c1d..67928310221fb 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -885,11 +885,6 @@ enum CorInfoIntrinsics CORINFO_INTRINSIC_Array_Get, // Get the value of an element in an array CORINFO_INTRINSIC_Array_Address, // Get the address of an element in an array CORINFO_INTRINSIC_Array_Set, // Set the value of an element in an array - CORINFO_INTRINSIC_RTH_GetValueInternal, - CORINFO_INTRINSIC_Object_GetType, - CORINFO_INTRINSIC_StubHelpers_GetStubContext, - CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr, - CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress, CORINFO_INTRINSIC_ByReference_Ctor, CORINFO_INTRINSIC_ByReference_Value, diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index b51f56b2df5c7..21679993f73a6 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 0c6f2d8d-f1b7-4c28-bbe8-36c8f6b35fbf */ - 0xc6f2d8d, - 0xf1b7, - 0x4c28, - { 0xbb, 0xe8, 0x36, 0xc8, 0xf6, 0xb3, 0x5f, 0xbf} +constexpr GUID JITEEVersionIdentifier = { /* 3d9496d4-03f7-4eb0-bf7a-a88794e74537 */ + 0x3d9496d4, + 0x03f7, + 0x4eb0, + {0xbf, 0x7a, 0xa8, 0x87, 0x94, 0xe7, 0x45, 0x37} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 1a8d4040d5ecf..97a2cf9188d2c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4411,6 +4411,7 @@ class Compiler CORINFO_RESOLVED_TOKEN* pContstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, CorInfoIntrinsics* pIntrinsicID, + NamedIntrinsic* pIntrinsicName, bool* isSpecialIntrinsic = nullptr); GenTree* impMathIntrinsic(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 00c58d70bae1d..59e32910d7261 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -3947,21 +3947,16 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree) } break; } + + case NI_System_Object_GetType: + // Giving intrinsics a large fixed execution cost is because we'd like to CSE + // them, even if they are implemented by calls. This is different from modeling + // user calls since we never CSE user calls. + costEx = 36; + costSz = 4; + break; } } - else - { - // old style intrinsic (only Object_GetType is expected) - assert(intrinsic->gtIntrinsicName == NI_Illegal); - assert(intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Object_GetType); - - // Giving intrinsics a large fixed execution cost is because we'd like to CSE - // them, even if they are implemented by calls. This is different from modeling - // user calls since we never CSE user calls. - costEx = 36; - costSz = 4; - break; - } level++; break; @@ -11018,101 +11013,96 @@ void Compiler::gtDispTree(GenTree* tree, { GenTreeIntrinsic* intrinsic = tree->AsIntrinsic(); - if (intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Object_GetType) + assert(intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Illegal); + switch (intrinsic->gtIntrinsicName) { - assert(intrinsic->gtIntrinsicName == NI_Illegal); - printf(" objGetType"); - } - else - { - assert(intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Illegal); - switch (intrinsic->gtIntrinsicName) - { - case NI_System_Math_Abs: - printf(" abs"); - break; - case NI_System_Math_Acos: - printf(" acos"); - break; - case NI_System_Math_Acosh: - printf(" acosh"); - break; - case NI_System_Math_Asin: - printf(" asin"); - break; - case NI_System_Math_Asinh: - printf(" asinh"); - break; - case NI_System_Math_Atan: - printf(" atan"); - break; - case NI_System_Math_Atanh: - printf(" atanh"); - break; - case NI_System_Math_Atan2: - printf(" atan2"); - break; - case NI_System_Math_Cbrt: - printf(" cbrt"); - break; - case NI_System_Math_Ceiling: - printf(" ceiling"); - break; - case NI_System_Math_Cos: - printf(" cos"); - break; - case NI_System_Math_Cosh: - printf(" cosh"); - break; - case NI_System_Math_Exp: - printf(" exp"); - break; - case NI_System_Math_Floor: - printf(" floor"); - break; - case NI_System_Math_FMod: - printf(" fmod"); - break; - case NI_System_Math_FusedMultiplyAdd: - printf(" fma"); - break; - case NI_System_Math_ILogB: - printf(" ilogb"); - break; - case NI_System_Math_Log: - printf(" log"); - break; - case NI_System_Math_Log2: - printf(" log2"); - break; - case NI_System_Math_Log10: - printf(" log10"); - break; - case NI_System_Math_Pow: - printf(" pow"); - break; - case NI_System_Math_Round: - printf(" round"); - break; - case NI_System_Math_Sin: - printf(" sin"); - break; - case NI_System_Math_Sinh: - printf(" sinh"); - break; - case NI_System_Math_Sqrt: - printf(" sqrt"); - break; - case NI_System_Math_Tan: - printf(" tan"); - break; - case NI_System_Math_Tanh: - printf(" tanh"); - break; + case NI_System_Math_Abs: + printf(" abs"); + break; + case NI_System_Math_Acos: + printf(" acos"); + break; + case NI_System_Math_Acosh: + printf(" acosh"); + break; + case NI_System_Math_Asin: + printf(" asin"); + break; + case NI_System_Math_Asinh: + printf(" asinh"); + break; + case NI_System_Math_Atan: + printf(" atan"); + break; + case NI_System_Math_Atanh: + printf(" atanh"); + break; + case NI_System_Math_Atan2: + printf(" atan2"); + break; + case NI_System_Math_Cbrt: + printf(" cbrt"); + break; + case NI_System_Math_Ceiling: + printf(" ceiling"); + break; + case NI_System_Math_Cos: + printf(" cos"); + break; + case NI_System_Math_Cosh: + printf(" cosh"); + break; + case NI_System_Math_Exp: + printf(" exp"); + break; + case NI_System_Math_Floor: + printf(" floor"); + break; + case NI_System_Math_FMod: + printf(" fmod"); + break; + case NI_System_Math_FusedMultiplyAdd: + printf(" fma"); + break; + case NI_System_Math_ILogB: + printf(" ilogb"); + break; + case NI_System_Math_Log: + printf(" log"); + break; + case NI_System_Math_Log2: + printf(" log2"); + break; + case NI_System_Math_Log10: + printf(" log10"); + break; + case NI_System_Math_Pow: + printf(" pow"); + break; + case NI_System_Math_Round: + printf(" round"); + break; + case NI_System_Math_Sin: + printf(" sin"); + break; + case NI_System_Math_Sinh: + printf(" sinh"); + break; + case NI_System_Math_Sqrt: + printf(" sqrt"); + break; + case NI_System_Math_Tan: + printf(" tan"); + break; + case NI_System_Math_Tanh: + printf(" tanh"); + break; + case NI_System_Object_GetType: + printf(" objGetType"); + break; - default: - unreached(); - } + default: + unreached(); } } @@ -15373,13 +15363,13 @@ Compiler::TypeProducerKind Compiler::gtGetTypeProducerKind(GenTree* tree) } else if (tree->AsCall()->gtCallMoreFlags & GTF_CALL_M_SPECIAL_INTRINSIC) { - if (info.compCompHnd->getIntrinsicID(tree->AsCall()->gtCallMethHnd) == CORINFO_INTRINSIC_Object_GetType) + if (lookupNamedIntrinsic(tree->AsCall()->gtCallMethHnd) == NI_System_Object_GetType) { return TPK_GetType; } } } - else if ((tree->gtOper == GT_INTRINSIC) && (tree->AsIntrinsic()->gtIntrinsicId == CORINFO_INTRINSIC_Object_GetType)) + else if ((tree->gtOper == GT_INTRINSIC) && (tree->AsIntrinsic()->gtIntrinsicName == NI_System_Object_GetType)) { return TPK_GetType; } @@ -16964,7 +16954,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b { GenTreeIntrinsic* intrinsic = obj->AsIntrinsic(); - if (intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Object_GetType) + if (intrinsic->gtIntrinsicName == NI_System_Object_GetType) { CORINFO_CLASS_HANDLE runtimeType = info.compCompHnd->getBuiltinClass(CLASSID_RUNTIME_TYPE); assert(runtimeType != NO_CLASS_HANDLE); diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index ad45f810ad85d..6f6462d6743bd 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -292,7 +292,7 @@ GTNODE(JMPTABLE , GenTree ,0, (GTK_LEAF|GTK_NOCONTAIN)) // Ge GTNODE(SWITCH_TABLE , GenTreeOp ,0, (GTK_BINOP|GTK_NOVALUE)) // Jump Table based switch construct #ifdef TARGET_ARM64 GTNODE(ADDEX, GenTreeOp ,0, GTK_BINOP) // Add with sign/zero extension -GTNODE(BFIZ , GenTreeOp ,0, GTK_BINOP) // Bitfield Insert in Zero +GTNODE(BFIZ , GenTreeOp ,0, GTK_BINOP) // Bitfield Insert in Zero #endif //----------------------------------------------------------------------------- diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 70a5cf39b544a..3254b0a6afd92 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3648,17 +3648,8 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) const char* Compiler::impGetIntrinsicName(CorInfoIntrinsics intrinsicID) { static const char* const intrinsicNameMap[CORINFO_INTRINSIC_Count] = { - "CORINFO_INTRINSIC_Array_Get", - "CORINFO_INTRINSIC_Array_Address", - "CORINFO_INTRINSIC_Array_Set", - "CORINFO_INTRINSIC_RTH_GetValueInternal", - "CORINFO_INTRINSIC_Object_GetType", - "CORINFO_INTRINSIC_StubHelpers_GetStubContext", - "CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr", - "CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress", - "CORINFO_INTRINSIC_ByReference_Ctor", - "CORINFO_INTRINSIC_ByReference_Value", - "CORINFO_INTRINSIC_GetRawHandle", + "CORINFO_INTRINSIC_Array_Get", "CORINFO_INTRINSIC_Array_Address", "CORINFO_INTRINSIC_Array_Set", + "CORINFO_INTRINSIC_ByReference_Ctor", "CORINFO_INTRINSIC_ByReference_Value", "CORINFO_INTRINSIC_GetRawHandle", }; if ((0 <= intrinsicID) && (intrinsicID < CORINFO_INTRINSIC_Count)) @@ -3844,6 +3835,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, CorInfoIntrinsics* pIntrinsicID, + NamedIntrinsic* pIntrinsicName, bool* isSpecialIntrinsic) { assert((methodFlags & (CORINFO_FLG_INTRINSIC | CORINFO_FLG_JIT_INTRINSIC)) != 0); @@ -3919,24 +3911,25 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, } } - *pIntrinsicID = intrinsicID; + *pIntrinsicID = intrinsicID; + *pIntrinsicName = ni; - if (intrinsicID == CORINFO_INTRINSIC_StubHelpers_GetStubContext) + if (ni == NI_System_StubHelpers_GetStubContext) { // must be done regardless of DbgCode and MinOpts return gtNewLclvNode(lvaStubArgumentVar, TYP_I_IMPL); } #ifdef TARGET_64BIT - if (intrinsicID == CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr) + if (ni == NI_System_StubHelpers_GetStubContextAddr) { // must be done regardless of DbgCode and MinOpts return gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(lvaStubArgumentVar, TYP_I_IMPL)); } #else - assert(intrinsicID != CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr); + assert(ni != NI_System_StubHelpers_GetStubContextAddr); #endif - if (intrinsicID == CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress) + if (ni == NI_System_StubHelpers_NextCallReturnAddress) { // For now we just avoid inlining anything into these methods since // this intrinsic is only rarely used. We could do this better if we @@ -3982,127 +3975,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, retNode = impArrayAccessIntrinsic(clsHnd, sig, memberRef, readonlyCall, intrinsicID); break; - case CORINFO_INTRINSIC_RTH_GetValueInternal: - op1 = impStackTop(0).val; - if (op1->gtOper == GT_CALL && (op1->AsCall()->gtCallType == CT_HELPER) && - gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall())) - { - // Old tree - // Helper-RuntimeTypeHandle -> TreeToGetNativeTypeHandle - // - // New tree - // TreeToGetNativeTypeHandle - - // Remove call to helper and return the native TypeHandle pointer that was the parameter - // to that helper. - - op1 = impPopStack().val; - - // Get native TypeHandle argument to old helper - GenTreeCall::Use* arg = op1->AsCall()->gtCallArgs; - assert(arg->GetNext() == nullptr); - op1 = arg->GetNode(); - retNode = op1; - } - // Call the regular function. - break; - - case CORINFO_INTRINSIC_Object_GetType: - { - JITDUMP("\n impIntrinsic: call to Object.GetType\n"); - op1 = impStackTop(0).val; - - // If we're calling GetType on a boxed value, just get the type directly. - if (op1->IsBoxedValue()) - { - JITDUMP("Attempting to optimize box(...).getType() to direct type construction\n"); - - // Try and clean up the box. Obtain the handle we - // were going to pass to the newobj. - GenTree* boxTypeHandle = gtTryRemoveBoxUpstreamEffects(op1, BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE); - - if (boxTypeHandle != nullptr) - { - // Note we don't need to play the TYP_STRUCT games here like - // do for LDTOKEN since the return value of this operator is Type, - // not RuntimeTypeHandle. - impPopStack(); - GenTreeCall::Use* helperArgs = gtNewCallArgs(boxTypeHandle); - GenTree* runtimeType = - gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE, TYP_REF, helperArgs); - retNode = runtimeType; - } - } - - // If we have a constrained callvirt with a "box this" transform - // we know we have a value class and hence an exact type. - // - // If so, instead of boxing and then extracting the type, just - // construct the type directly. - if ((retNode == nullptr) && (pConstrainedResolvedToken != nullptr) && - (constraintCallThisTransform == CORINFO_BOX_THIS)) - { - // Ensure this is one of the is simple box cases (in particular, rule out nullables). - const CorInfoHelpFunc boxHelper = info.compCompHnd->getBoxHelper(pConstrainedResolvedToken->hClass); - const bool isSafeToOptimize = (boxHelper == CORINFO_HELP_BOX); - - if (isSafeToOptimize) - { - JITDUMP("Optimizing constrained box-this obj.getType() to direct type construction\n"); - impPopStack(); - GenTree* typeHandleOp = - impTokenToHandle(pConstrainedResolvedToken, nullptr, true /* mustRestoreHandle */); - if (typeHandleOp == nullptr) - { - assert(compDonotInline()); - return nullptr; - } - GenTreeCall::Use* helperArgs = gtNewCallArgs(typeHandleOp); - GenTree* runtimeType = - gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE, TYP_REF, helperArgs); - retNode = runtimeType; - } - } - -#ifdef DEBUG - if (retNode != nullptr) - { - JITDUMP("Optimized result for call to GetType is\n"); - if (verbose) - { - gtDispTree(retNode); - } - } -#endif - - // Else expand as an intrinsic, unless the call is constrained, - // in which case we defer expansion to allow impImportCall do the - // special constraint processing. - if ((retNode == nullptr) && (pConstrainedResolvedToken == nullptr)) - { - JITDUMP("Expanding as special intrinsic\n"); - impPopStack(); - op1 = new (this, GT_INTRINSIC) GenTreeIntrinsic(genActualType(callType), op1, intrinsicID, ni, method); - - // Set the CALL flag to indicate that the operator is implemented by a call. - // Set also the EXCEPTION flag because the native implementation of - // CORINFO_INTRINSIC_Object_GetType intrinsic can throw NullReferenceException. - op1->gtFlags |= (GTF_CALL | GTF_EXCEPT); - retNode = op1; - // Might be further optimizable, so arrange to leave a mark behind - isSpecial = true; - } - - if (retNode == nullptr) - { - JITDUMP("Leaving as normal call\n"); - // Might be further optimizable, so arrange to leave a mark behind - isSpecial = true; - } - - break; - } - // Implement ByReference Ctor. This wraps the assignment of the ref into a byref-like field // in a value type. The canonical example of this is Span. In effect this is just a // substitution. The parameter byref will be assigned into the newly allocated object. @@ -4318,6 +4190,33 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + case NI_System_RuntimeTypeHandle_GetValueInternal: + { + GenTree* op1 = impStackTop(0).val; + if (op1->gtOper == GT_CALL && (op1->AsCall()->gtCallType == CT_HELPER) && + gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall())) + { + // Old tree + // Helper-RuntimeTypeHandle -> TreeToGetNativeTypeHandle + // + // New tree + // TreeToGetNativeTypeHandle + + // Remove call to helper and return the native TypeHandle pointer that was the parameter + // to that helper. + + op1 = impPopStack().val; + + // Get native TypeHandle argument to old helper + GenTreeCall::Use* arg = op1->AsCall()->gtCallArgs; + assert(arg->GetNext() == nullptr); + op1 = arg->GetNode(); + retNode = op1; + } + // Call the regular function. + break; + } + case NI_System_Type_GetTypeFromHandle: { GenTree* op1 = impStackTop(0).val; @@ -4678,6 +4577,103 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + case NI_System_Object_GetType: + { + JITDUMP("\n impIntrinsic: call to Object.GetType\n"); + GenTree* op1 = impStackTop(0).val; + + // If we're calling GetType on a boxed value, just get the type directly. + if (op1->IsBoxedValue()) + { + JITDUMP("Attempting to optimize box(...).getType() to direct type construction\n"); + + // Try and clean up the box. Obtain the handle we + // were going to pass to the newobj. + GenTree* boxTypeHandle = gtTryRemoveBoxUpstreamEffects(op1, BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE); + + if (boxTypeHandle != nullptr) + { + // Note we don't need to play the TYP_STRUCT games here like + // do for LDTOKEN since the return value of this operator is Type, + // not RuntimeTypeHandle. + impPopStack(); + GenTreeCall::Use* helperArgs = gtNewCallArgs(boxTypeHandle); + GenTree* runtimeType = + gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE, TYP_REF, helperArgs); + retNode = runtimeType; + } + } + + // If we have a constrained callvirt with a "box this" transform + // we know we have a value class and hence an exact type. + // + // If so, instead of boxing and then extracting the type, just + // construct the type directly. + if ((retNode == nullptr) && (pConstrainedResolvedToken != nullptr) && + (constraintCallThisTransform == CORINFO_BOX_THIS)) + { + // Ensure this is one of the is simple box cases (in particular, rule out nullables). + const CorInfoHelpFunc boxHelper = info.compCompHnd->getBoxHelper(pConstrainedResolvedToken->hClass); + const bool isSafeToOptimize = (boxHelper == CORINFO_HELP_BOX); + + if (isSafeToOptimize) + { + JITDUMP("Optimizing constrained box-this obj.getType() to direct type construction\n"); + impPopStack(); + GenTree* typeHandleOp = + impTokenToHandle(pConstrainedResolvedToken, nullptr, true /* mustRestoreHandle */); + if (typeHandleOp == nullptr) + { + assert(compDonotInline()); + return nullptr; + } + GenTreeCall::Use* helperArgs = gtNewCallArgs(typeHandleOp); + GenTree* runtimeType = + gtNewHelperCallNode(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE, TYP_REF, helperArgs); + retNode = runtimeType; + } + } + +#ifdef DEBUG + if (retNode != nullptr) + { + JITDUMP("Optimized result for call to GetType is\n"); + if (verbose) + { + gtDispTree(retNode); + } + } +#endif + + // Else expand as an intrinsic, unless the call is constrained, + // in which case we defer expansion to allow impImportCall do the + // special constraint processing. + if ((retNode == nullptr) && (pConstrainedResolvedToken == nullptr)) + { + JITDUMP("Expanding as special intrinsic\n"); + impPopStack(); + op1 = new (this, GT_INTRINSIC) + GenTreeIntrinsic(genActualType(callType), op1, intrinsicID, ni, method); + + // Set the CALL flag to indicate that the operator is implemented by a call. + // Set also the EXCEPTION flag because the native implementation of + // NI_System_Object_GetType intrinsic can throw NullReferenceException. + op1->gtFlags |= (GTF_CALL | GTF_EXCEPT); + retNode = op1; + // Might be further optimizable, so arrange to leave a mark behind + isSpecial = true; + } + + if (retNode == nullptr) + { + JITDUMP("Leaving as normal call\n"); + // Might be further optimizable, so arrange to leave a mark behind + isSpecial = true; + } + + break; + } + case NI_System_Array_GetLength: case NI_System_Array_GetLowerBound: case NI_System_Array_GetUpperBound: @@ -5206,6 +5202,17 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_Object_MemberwiseClone; } + else if (strcmp(methodName, "GetType") == 0) + { + result = NI_System_Object_GetType; + } + } + else if (strcmp(className, "RuntimeTypeHandle") == 0) + { + if (strcmp(methodName, "GetValueInternal") == 0) + { + result = NI_System_RuntimeTypeHandle_GetValueInternal; + } } else if (strcmp(className, "Type") == 0) { @@ -5420,6 +5427,26 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) } } } + else if (strcmp(namespaceName, "System.StubHelpers") == 0) + { + if (strcmp(className, "StubHelpers") == 0) + { + if (strcmp(methodName, "GetStubContext") == 0) + { + result = NI_System_StubHelpers_GetStubContext; + } +#ifdef TARGET_64BIT + else if (strcmp(methodName, "GetStubContextAddr") == 0) + { + result = NI_System_StubHelpers_GetStubContextAddr; + } +#endif + else if (strcmp(methodName, "NextCallReturnAddress") == 0) + { + result = NI_System_StubHelpers_NextCallReturnAddress; + } + } + } if (result == NI_Illegal) { @@ -8761,6 +8788,7 @@ var_types Compiler::impImportCall(OPCODE opcode, else // (opcode != CEE_CALLI) { CorInfoIntrinsics intrinsicID = CORINFO_INTRINSIC_Count; + NamedIntrinsic ni = NI_Illegal; // Passing CORINFO_CALLINFO_ALLOWINSTPARAM indicates that this JIT is prepared to // supply the instantiation parameters necessary to make direct calls to underlying @@ -8848,7 +8876,7 @@ var_types Compiler::impImportCall(OPCODE opcode, const bool isTailCall = canTailCall && (tailCallFlags != 0); call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token, isReadonlyCall, - isTailCall, pConstrainedResolvedToken, callInfo->thisTransform, &intrinsicID, + isTailCall, pConstrainedResolvedToken, callInfo->thisTransform, &intrinsicID, &ni, &isSpecialIntrinsic); if (compDonotInline()) @@ -9076,7 +9104,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // TODO-CQ: JIT64 does not introduce the null check for many more helper calls // and intrinsics. if (callInfo->nullInstanceCheck && - !((mflags & CORINFO_FLG_INTRINSIC) != 0 && (intrinsicID == CORINFO_INTRINSIC_Object_GetType))) + !((mflags & CORINFO_FLG_INTRINSIC) != 0 && (ni == NI_System_Object_GetType))) { call->gtFlags |= GTF_CALL_NULLCHECK; } diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index 62ebb80c08999..ccd0e128804bf 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -60,6 +60,11 @@ enum NamedIntrinsic : unsigned short NI_System_Array_GetLowerBound, NI_System_Array_GetUpperBound, NI_System_Object_MemberwiseClone, + NI_System_Object_GetType, + NI_System_RuntimeTypeHandle_GetValueInternal, + NI_System_StubHelpers_GetStubContext, + NI_System_StubHelpers_GetStubContextAddr, + NI_System_StubHelpers_NextCallReturnAddress, NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan, NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray, diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index e4d18a9a0c9f9..538fd5cb8ffae 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9452,7 +9452,7 @@ void Compiler::fgValueNumberIntrinsic(GenTree* tree) } else { - assert(intrinsic->gtIntrinsicId == CORINFO_INTRINSIC_Object_GetType); + assert(intrinsic->gtIntrinsicName == NI_System_Object_GetType); intrinsic->gtVNPair = vnStore->VNPWithExc(vnStore->VNPairForFunc(intrinsic->TypeGet(), VNF_ObjGetType, arg0VNP), arg0VNPx); } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.Intrinsics.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.Intrinsics.cs index 50f7783be8654..364b0c1384232 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.Intrinsics.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.Intrinsics.cs @@ -79,11 +79,6 @@ static IntrinsicHashtable InitializeIntrinsicHashtable() table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_Array_Get, "Get", null, null); table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_Array_Address, "Address", null, null); table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_Array_Set, "Set", null, null); - table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_RTH_GetValueInternal, "GetValueInternal", "System", "RuntimeTypeHandle"); - table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_Object_GetType, "GetType", "System", "Object"); - table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_StubHelpers_GetStubContext, "GetStubContext", "System.StubHelpers", "StubHelpers"); // interop-specific - // table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr, "GetStubContextAddr", "System.StubHelpers", "StubHelpers"); // interop-specific - table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress, "NextCallReturnAddress", "System.StubHelpers", "StubHelpers"); table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_ByReference_Ctor, ".ctor", "System", "ByReference`1"); table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_ByReference_Value, "get_Value", "System", "ByReference`1"); table.Add(CorInfoIntrinsics.CORINFO_INTRINSIC_GetRawHandle, "EETypePtrOf", "System", "EETypePtr"); @@ -134,7 +129,6 @@ private CorInfoIntrinsics getIntrinsicID(MethodDesc method, byte* pMustExpand) return CorInfoIntrinsics.CORINFO_INTRINSIC_Illegal; break; - case CorInfoIntrinsics.CORINFO_INTRINSIC_RTH_GetValueInternal: case CorInfoIntrinsics.CORINFO_INTRINSIC_ByReference_Ctor: case CorInfoIntrinsics.CORINFO_INTRINSIC_ByReference_Value: if (pMustExpand != null) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index bc6d53514df1d..4d59d99a771d3 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -442,11 +442,6 @@ public enum CorInfoIntrinsics CORINFO_INTRINSIC_Array_Get, // Get the value of an element in an array CORINFO_INTRINSIC_Array_Address, // Get the address of an element in an array CORINFO_INTRINSIC_Array_Set, // Set the value of an element in an array - CORINFO_INTRINSIC_RTH_GetValueInternal, - CORINFO_INTRINSIC_Object_GetType, - CORINFO_INTRINSIC_StubHelpers_GetStubContext, - CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr, - CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress, CORINFO_INTRINSIC_ByReference_Ctor, CORINFO_INTRINSIC_ByReference_Value, CORINFO_INTRINSIC_GetRawHandle, diff --git a/src/coreclr/vm/corelib.cpp b/src/coreclr/vm/corelib.cpp index 4189a6371b25c..dabd4589cf5fe 100644 --- a/src/coreclr/vm/corelib.cpp +++ b/src/coreclr/vm/corelib.cpp @@ -281,12 +281,6 @@ const USHORT c_nCoreLibFieldDescriptions = ARRAY_SIZE(c_rgCoreLibFieldDescriptio #define FCFuncElementSig(name,sig,impl) \ FCFuncFlag_HasSignature + FCFuncElement(name, impl) (LPVOID)sig, -#define FCIntrinsic(name,impl,intrinsicID) FCFuncFlags(intrinsicID, ECall::InvalidDynamicFCallId), \ - (LPVOID)GetEEFuncEntryPoint(impl), (LPVOID)name, - -#define FCIntrinsicSig(name,sig,impl,intrinsicID) \ - FCFuncFlag_HasSignature + FCIntrinsic(name,impl,intrinsicID) (LPVOID)sig, - #define FCDynamic(name,intrinsicID,dynamicID) FCFuncFlags(intrinsicID, dynamicID), \ NULL, (LPVOID)name, diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index afdbf678d1183..fd0119b354b1b 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -1,13 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. + // ECallList.H // // This file contains definitions of FCall entrypoints // - - - #ifndef FCFuncElement #define FCFuncElement(name, impl) #endif @@ -16,14 +14,6 @@ #define FCFuncElementSig(name,sig,impl) #endif -#ifndef FCIntrinsic -#define FCIntrinsic(name,impl,intrinsicID) -#endif - -#ifndef FCIntrinsicSig -#define FCIntrinsicSig(name,sig,impl,intrinsicID) -#endif - #ifndef FCDynamic #define FCDynamic(name,intrinsicID,dynamicID) #endif @@ -54,8 +44,6 @@ // // - - FCFuncStart(gDependentHandleFuncs) FCFuncElement("InternalInitialize", DependentHandle::InternalInitialize) FCFuncElement("InternalGetTarget", DependentHandle::InternalGetTarget) @@ -66,10 +54,6 @@ FCFuncStart(gDependentHandleFuncs) FCFuncElement("InternalFree", DependentHandle::InternalFree) FCFuncEnd() - - - - FCFuncStart(gEnumFuncs) FCFuncElement("InternalGetUnderlyingType", ReflectionEnum::InternalGetEnumUnderlyingType) FCFuncElement("InternalGetCorElementType", ReflectionEnum::InternalGetCorElementType) @@ -77,7 +61,7 @@ FCFuncStart(gEnumFuncs) FCFuncEnd() FCFuncStart(gObjectFuncs) - FCIntrinsic("GetType", ObjectNative::GetClass, CORINFO_INTRINSIC_Object_GetType) + FCFuncElement("GetType", ObjectNative::GetClass) FCFuncEnd() FCFuncStart(gStringFuncs) @@ -192,7 +176,7 @@ FCFuncStart(gCOMTypeHandleFuncs) FCFuncElement("AllocateComObject", RuntimeTypeHandle::AllocateComObject) #endif // FEATURE_COMINTEROP FCFuncElement("CompareCanonicalHandles", RuntimeTypeHandle::CompareCanonicalHandles) - FCIntrinsic("GetValueInternal", RuntimeTypeHandle::GetValueInternal, CORINFO_INTRINSIC_RTH_GetValueInternal) + FCFuncElement("GetValueInternal", RuntimeTypeHandle::GetValueInternal) FCFuncElement("IsEquivalentTo", RuntimeTypeHandle::IsEquivalentTo) FCFuncEnd() @@ -254,7 +238,6 @@ FCFuncStart(gRuntimeMethodHandle) FCFuncElement("GetLoaderAllocator", RuntimeMethodHandle::GetLoaderAllocator) FCFuncEnd() - FCFuncStart(gCOMFieldHandleNewFuncs) FCFuncElement("GetValue", RuntimeFieldHandle::GetValue) FCFuncElement("SetValue", RuntimeFieldHandle::SetValue) @@ -270,7 +253,6 @@ FCFuncStart(gCOMFieldHandleNewFuncs) FCFuncElement("AcquiresContextFromThis", RuntimeFieldHandle::AcquiresContextFromThis) FCFuncEnd() - FCFuncStart(gCOMModuleFuncs) FCFuncElement("GetTypes", COMModule::GetTypes) FCFuncEnd() @@ -713,9 +695,9 @@ FCFuncStart(gStubHelperFuncs) FCFuncElement("ValidateObject", StubHelpers::ValidateObject) FCFuncElement("ValidateByref", StubHelpers::ValidateByref) FCFuncElement("LogPinnedArgument", StubHelpers::LogPinnedArgument) - FCIntrinsic("GetStubContext", StubHelpers::GetStubContext, CORINFO_INTRINSIC_StubHelpers_GetStubContext) + FCFuncElement("GetStubContext", StubHelpers::GetStubContext) #ifdef TARGET_64BIT - FCIntrinsic("GetStubContextAddr", StubHelpers::GetStubContextAddr, CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr) + FCFuncElement("GetStubContextAddr", StubHelpers::GetStubContextAddr) #endif // TARGET_64BIT #ifdef FEATURE_ARRAYSTUB_AS_IL FCFuncElement("ArrayTypeCheck", StubHelpers::ArrayTypeCheck) @@ -723,7 +705,7 @@ FCFuncStart(gStubHelperFuncs) #ifdef FEATURE_MULTICASTSTUB_AS_IL FCFuncElement("MulticastDebuggerTraceHelper", StubHelpers::MulticastDebuggerTraceHelper) #endif //FEATURE_MULTICASTSTUB_AS_IL - FCIntrinsic("NextCallReturnAddress", StubHelpers::NextCallReturnAddress, CORINFO_INTRINSIC_StubHelpers_NextCallReturnAddress) + FCFuncElement("NextCallReturnAddress", StubHelpers::NextCallReturnAddress) FCFuncEnd() FCFuncStart(gGCHandleFuncs) @@ -780,7 +762,6 @@ FCFuncEnd() #endif // FEATURE_COMINTEROP - // // // Class definitions @@ -839,7 +820,6 @@ FCClassElement("ObjectMarshaler", "System.StubHelpers", gObjectMarshalerFuncs) #endif FCClassElement("OverlappedData", "System.Threading", gOverlappedFuncs) - FCClassElement("RegisteredWaitHandle", "System.Threading", gRegisteredWaitHandleFuncs) FCClassElement("RuntimeAssembly", "System.Reflection", gRuntimeAssemblyFuncs) @@ -869,8 +849,6 @@ FCClassElement("WeakReference`1", "System", gWeakReferenceOfTFuncs) #undef FCFuncElement #undef FCFuncElementSig -#undef FCIntrinsic -#undef FCIntrinsicSig #undef FCDynamic #undef FCDynamicSig #undef FCUnreferenced diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index cbec8481ce4c4..aba9dac31eed5 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9155,16 +9155,19 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T didIntrinsic = true; break; #if INTERP_ILSTUBS - case CORINFO_INTRINSIC_StubHelpers_GetStubContext: + // TODO: enable when NamedIntrinsic is available to interpreter + /* + case NI_System_StubHelpers_GetStubContext: OpStackSet(m_curStackHt, GetStubContext()); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT)); m_curStackHt++; didIntrinsic = true; break; - case CORINFO_INTRINSIC_StubHelpers_GetStubContextAddr: + case NI_System_StubHelpers_GetStubContextAddr: OpStackSet(m_curStackHt, GetStubContextAddr()); OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT)); m_curStackHt++; didIntrinsic = true; break; + */ #endif // INTERP_ILSTUBS default: #if INTERP_TRACING From 638836f51f151699773a6f46420d67eefcedfa6b Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Fri, 3 Dec 2021 06:19:53 +0200 Subject: [PATCH 2/8] Apply Egor's patch Co-authored-by: EgorBo --- .../ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs | 1 + src/coreclr/vm/jitinterface.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 542565a5128c9..5b78f559030fe 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1644,6 +1644,7 @@ private void ceeInfoGetCallInfo( // 3) JIT intrinsics - since they have pre-defined behavior devirt = targetMethod.OwningType.IsValueType || (targetMethod.OwningType.IsDelegate && targetMethod.Name == "Invoke") || + (targetMethod.OwningType.IsObject && targetMethod.Name == "GetType") || (targetMethod.IsIntrinsic && getIntrinsicID(targetMethod, null) != CorInfoIntrinsics.CORINFO_INTRINSIC_Illegal); callVirtCrossingVersionBubble = true; diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 94d7101094891..4fca033ef120f 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -5007,7 +5007,6 @@ void CEEInfo::getCallInfo( bool directCall = false; bool resolvedCallVirt = false; - bool callVirtCrossingVersionBubble = false; // Delegate targets are always treated as direct calls here. (It would be nice to clean it up...). if (flags & CORINFO_CALLINFO_LDFTN) From 615a5e76df21dba620095ca4ed0f9b4310ca486c Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 00:47:53 +0200 Subject: [PATCH 3/8] Delete GetStubContextAddr and related code --- docs/design/coreclr/botr/clr-abi.md | 2 +- .../src/System/StubHelpers.cs | 6 ------ src/coreclr/jit/importer.cpp | 15 -------------- src/coreclr/jit/namedintrinsiclist.h | 1 - src/coreclr/vm/corelib.h | 3 --- src/coreclr/vm/dllimport.cpp | 9 --------- src/coreclr/vm/ecalllist.h | 3 --- src/coreclr/vm/interpreter.cpp | 5 ----- src/coreclr/vm/interpreter.h | 1 - src/coreclr/vm/stubhelpers.cpp | 20 +------------------ src/coreclr/vm/stubhelpers.h | 9 +-------- 11 files changed, 3 insertions(+), 71 deletions(-) diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index 55ad398e9bfe1..f7b01352de922 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -109,7 +109,7 @@ ARM64-only: When a method returns a structure that is larger than 16 bytes the c *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`, and the address of that location is returned for `NI_System_StubHelpers_GetStubContextAddr`. +*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`. # PInvokes diff --git a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs index 279d53816124a..cf2061e249d4d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs @@ -1348,12 +1348,6 @@ internal static void CheckStringLength(uint length) [MethodImpl(MethodImplOptions.InternalCall)] internal static extern IntPtr GetStubContext(); -#if TARGET_64BIT - [Intrinsic] - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern IntPtr GetStubContextAddr(); -#endif // TARGET_64BIT - #if FEATURE_ARRAYSTUB_AS_IL [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void ArrayTypeCheck(object o, object[] arr); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 3254b0a6afd92..56ef996da359e 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -3919,15 +3919,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // must be done regardless of DbgCode and MinOpts return gtNewLclvNode(lvaStubArgumentVar, TYP_I_IMPL); } -#ifdef TARGET_64BIT - if (ni == NI_System_StubHelpers_GetStubContextAddr) - { - // must be done regardless of DbgCode and MinOpts - return gtNewOperNode(GT_ADDR, TYP_I_IMPL, gtNewLclvNode(lvaStubArgumentVar, TYP_I_IMPL)); - } -#else - assert(ni != NI_System_StubHelpers_GetStubContextAddr); -#endif if (ni == NI_System_StubHelpers_NextCallReturnAddress) { @@ -5435,12 +5426,6 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_StubHelpers_GetStubContext; } -#ifdef TARGET_64BIT - else if (strcmp(methodName, "GetStubContextAddr") == 0) - { - result = NI_System_StubHelpers_GetStubContextAddr; - } -#endif else if (strcmp(methodName, "NextCallReturnAddress") == 0) { result = NI_System_StubHelpers_NextCallReturnAddress; diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index ccd0e128804bf..5d07e88b576ae 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -63,7 +63,6 @@ enum NamedIntrinsic : unsigned short NI_System_Object_GetType, NI_System_RuntimeTypeHandle_GetValueInternal, NI_System_StubHelpers_GetStubContext, - NI_System_StubHelpers_GetStubContextAddr, NI_System_StubHelpers_NextCallReturnAddress, NI_System_Runtime_CompilerServices_RuntimeHelpers_CreateSpan, diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index b46562ddb13b7..a24c2f752f4a7 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1027,9 +1027,6 @@ DEFINE_METHOD(STUBHELPERS, VALIDATE_OBJECT, Validate DEFINE_METHOD(STUBHELPERS, VALIDATE_BYREF, ValidateByref, SM_IntPtr_IntPtr_Obj_RetVoid) DEFINE_METHOD(STUBHELPERS, GET_STUB_CONTEXT, GetStubContext, SM_RetIntPtr) DEFINE_METHOD(STUBHELPERS, LOG_PINNED_ARGUMENT, LogPinnedArgument, SM_IntPtr_IntPtr_RetVoid) -#ifdef TARGET_64BIT -DEFINE_METHOD(STUBHELPERS, GET_STUB_CONTEXT_ADDR, GetStubContextAddr, SM_RetIntPtr) -#endif // TARGET_64BIT DEFINE_METHOD(STUBHELPERS, NEXT_CALL_RETURN_ADDRESS, NextCallReturnAddress, SM_RetIntPtr) DEFINE_METHOD(STUBHELPERS, SAFE_HANDLE_ADD_REF, SafeHandleAddRef, SM_SafeHandle_RefBool_RetIntPtr) DEFINE_METHOD(STUBHELPERS, SAFE_HANDLE_RELEASE, SafeHandleRelease, SM_SafeHandle_RetVoid) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 7888c8e377f4b..c62c2c96fbdeb 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -2076,21 +2076,12 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth if (SF_IsForwardStub(dwStubFlags)) // managed-to-native { - if (SF_IsDelegateStub(dwStubFlags)) // delegate invocation { // get the delegate unmanaged target - we call a helper instead of just grabbing // the _methodPtrAux field because we may need to intercept the call for host, etc. pcsEmit->EmitLoadThis(); -#ifdef TARGET_64BIT - // on AMD64 GetDelegateTarget will return address of the generic stub for host when we are hosted - // and update the secret argument with real target - the secret arg will be embedded in the - // InlinedCallFrame by the JIT and fetched via TLS->Thread->Frame->Datum by the stub for host - pcsEmit->EmitCALL(METHOD__STUBHELPERS__GET_STUB_CONTEXT_ADDR, 0, 1); -#else // !TARGET_64BIT - // we don't need to do this on x86 because stub for host is generated dynamically per target pcsEmit->EmitLDNULL(); -#endif // !TARGET_64BIT pcsEmit->EmitCALL(METHOD__STUBHELPERS__GET_DELEGATE_TARGET, 2, 1); } else // direct invocation diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index fd0119b354b1b..8a593ab1f2566 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -696,9 +696,6 @@ FCFuncStart(gStubHelperFuncs) FCFuncElement("ValidateByref", StubHelpers::ValidateByref) FCFuncElement("LogPinnedArgument", StubHelpers::LogPinnedArgument) FCFuncElement("GetStubContext", StubHelpers::GetStubContext) -#ifdef TARGET_64BIT - FCFuncElement("GetStubContextAddr", StubHelpers::GetStubContextAddr) -#endif // TARGET_64BIT #ifdef FEATURE_ARRAYSTUB_AS_IL FCFuncElement("ArrayTypeCheck", StubHelpers::ArrayTypeCheck) #endif //FEATURE_ARRAYSTUB_AS_IL diff --git a/src/coreclr/vm/interpreter.cpp b/src/coreclr/vm/interpreter.cpp index aba9dac31eed5..0f3490ade5a05 100644 --- a/src/coreclr/vm/interpreter.cpp +++ b/src/coreclr/vm/interpreter.cpp @@ -9162,11 +9162,6 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT)); m_curStackHt++; didIntrinsic = true; break; - case NI_System_StubHelpers_GetStubContextAddr: - OpStackSet(m_curStackHt, GetStubContextAddr()); - OpStackTypeSet(m_curStackHt, InterpreterType(CORINFO_TYPE_NATIVEINT)); - m_curStackHt++; didIntrinsic = true; - break; */ #endif // INTERP_ILSTUBS default: diff --git a/src/coreclr/vm/interpreter.h b/src/coreclr/vm/interpreter.h index 33a83c200922d..a6346cebc4ccf 100644 --- a/src/coreclr/vm/interpreter.h +++ b/src/coreclr/vm/interpreter.h @@ -907,7 +907,6 @@ class Interpreter #if INTERP_ILSTUBS void* GetStubContext() { return m_stubContext; } - void* GetStubContextAddr() { return &m_stubContext; } #endif OBJECTREF* GetAddressOfSecurityObject() { return &m_securityObject; } diff --git a/src/coreclr/vm/stubhelpers.cpp b/src/coreclr/vm/stubhelpers.cpp index 8e15d3aa54a83..5c7a09ca897fa 100644 --- a/src/coreclr/vm/stubhelpers.cpp +++ b/src/coreclr/vm/stubhelpers.cpp @@ -4,9 +4,6 @@ // File: stubhelpers.cpp // -// - - #include "common.h" #include "mlinfo.h" @@ -504,7 +501,7 @@ FCIMPL1(void*, StubHelpers::GetNDirectTarget, NDirectMethodDesc* pNMD) } FCIMPLEND -FCIMPL2(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE, UINT_PTR *ppStubArg) +FCIMPL1(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE) { PCODE pEntryPoint = NULL; @@ -528,10 +525,6 @@ FCIMPL2(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE, UINT // The lowest bit is used to distinguish between MD and target on 64-bit. target = (target << 1) | 1; - // On 64-bit we pass the real target to the stub-for-host through this out argument, - // see IL code gen in NDirectStubLinker::DoNDirect for details. - *ppStubArg = target; - #elif defined(TARGET_ARM) // @ARMTODO: Nothing to do for ARM yet since we don't support the hosted path. #endif // HOST_64BIT, TARGET_ARM @@ -980,17 +973,6 @@ FCIMPL2(void, StubHelpers::LogPinnedArgument, MethodDesc *target, Object *pinned } FCIMPLEND -#ifdef TARGET_64BIT -FCIMPL0(void*, StubHelpers::GetStubContextAddr) -{ - FCALL_CONTRACT; - - FCUnique(0xa1); - UNREACHABLE_MSG("This is a JIT intrinsic!"); -} -FCIMPLEND -#endif // TARGET_64BIT - FCIMPL1(DWORD, StubHelpers::CalcVaListSize, VARARGS *varargs) { FCALL_CONTRACT; diff --git a/src/coreclr/vm/stubhelpers.h b/src/coreclr/vm/stubhelpers.h index 308af895ccadc..6beba1f163955 100644 --- a/src/coreclr/vm/stubhelpers.h +++ b/src/coreclr/vm/stubhelpers.h @@ -4,9 +4,6 @@ // File: stubhelpers.h // -// - - #ifndef __STUBHELPERS_h__ #define __STUBHELPERS_h__ @@ -60,8 +57,7 @@ class StubHelpers static FCDECL0(void, SetLastError ); static FCDECL0(void, ClearLastError ); static FCDECL1(void*, GetNDirectTarget, NDirectMethodDesc* pNMD); - static FCDECL2(void*, GetDelegateTarget, DelegateObject *pThisUNSAFE, UINT_PTR *ppStubArg); - + static FCDECL1(void*, GetDelegateTarget, DelegateObject *pThisUNSAFE); static FCDECL2(void, ThrowInteropParamException, UINT resID, UINT paramIdx); static FCDECL1(Object*, GetHRExceptionObject, HRESULT hr); @@ -80,9 +76,6 @@ class StubHelpers static FCDECL2(void, MarshalToManagedVaListInternal, va_list va, VARARGS* pArgIterator); static FCDECL0(void*, GetStubContext); static FCDECL2(void, LogPinnedArgument, MethodDesc *localDesc, Object *nativeArg); -#ifdef TARGET_64BIT - static FCDECL0(void*, GetStubContextAddr); -#endif // TARGET_64BIT static FCDECL1(DWORD, CalcVaListSize, VARARGS *varargs); static FCDECL3(void, ValidateObject, Object *pObjUNSAFE, MethodDesc *pMD, Object *pThisUNSAFE); static FCDECL3(void, ValidateByref, void *pByref, MethodDesc *pMD, Object *pThisUNSAFE); From 32ebd9cd339ac196e47a1a17d6fb6691d088e095 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 00:52:51 +0200 Subject: [PATCH 4/8] Cleanup post-merge: use minipal / delete whitespace --- src/coreclr/tools/StressLogAnalyzer/StressLogAnalyzer.cpp | 4 ++-- src/coreclr/tools/StressLogAnalyzer/StressLogDump.cpp | 7 +++---- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/coreclr/tools/StressLogAnalyzer/StressLogAnalyzer.cpp b/src/coreclr/tools/StressLogAnalyzer/StressLogAnalyzer.cpp index 5921e82b2b862..8b5523299762e 100644 --- a/src/coreclr/tools/StressLogAnalyzer/StressLogAnalyzer.cpp +++ b/src/coreclr/tools/StressLogAnalyzer/StressLogAnalyzer.cpp @@ -8,7 +8,7 @@ #include "assert.h" -#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0])) +#include #define MEMORY_MAPPED_STRESSLOG @@ -91,7 +91,7 @@ int main(int argc, char *argv[]) WCHAR filename[MAX_PATH]; if (MultiByteToWideChar(CP_ACP, 0, argv[1], -1, filename, MAX_PATH) == 0) return 1; - + HANDLE file = CreateFile(filename, GENERIC_READ, FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (file == INVALID_HANDLE_VALUE) { diff --git a/src/coreclr/tools/StressLogAnalyzer/StressLogDump.cpp b/src/coreclr/tools/StressLogAnalyzer/StressLogDump.cpp index 842b21eba7872..a0e8c877be52f 100644 --- a/src/coreclr/tools/StressLogAnalyzer/StressLogDump.cpp +++ b/src/coreclr/tools/StressLogAnalyzer/StressLogDump.cpp @@ -11,8 +11,7 @@ #include "util.h" #include #include - -#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0])) +#include #ifndef STRESS_LOG #define STRESS_LOG @@ -177,7 +176,7 @@ void formatOutput(struct IDebugDataSpaces* memCallBack, ___in FILE* file, __inou // Print the string up to that point c = *ptr; *ptr = 0; // Terminate the string temporarily - fprintf(file, format, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], args[9], args[10]); + fprintf(file, format, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], args[9], args[10]); *ptr = c; // Put it back // move the argument pointers past the part the was printed @@ -315,7 +314,7 @@ void formatOutput(struct IDebugDataSpaces* memCallBack, ___in FILE* file, __inou } // Print anything after the last special format instruction. - fprintf(file, format, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], args[9], args[10]); + fprintf(file, format, args[0], args[1], args[2], args[3], args[4], args[5], args[6], args[7], args[8], args[9], args[10]); fprintf(file, "\n"); } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index b417176a168f1..da50f5bbf458e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -970,8 +970,8 @@ private MethodWithToken ComputeMethodWithToken(MethodDesc method, ref CORINFO_RE private ModuleToken HandleToModuleToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken, MethodDesc methodDesc, out object context, ref TypeDesc constrainedType) { - if (methodDesc != null && (_compilation.NodeFactory.CompilationModuleGroup.VersionsWithMethodBody(methodDesc) - || (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_DevirtualizedMethod) + if (methodDesc != null && (_compilation.NodeFactory.CompilationModuleGroup.VersionsWithMethodBody(methodDesc) + || (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_DevirtualizedMethod) || methodDesc.IsPInvoke)) { if ((CorTokenType)(unchecked((uint)pResolvedToken.token) & 0xFF000000u) == CorTokenType.mdtMethodDef && @@ -1129,7 +1129,7 @@ private void setVars(CORINFO_METHOD_STRUCT_* ftn, uint cVars, NativeVarInfo* var { _debugVarInfos[i] = vars[i]; } - + // JIT gave the ownership of this to us, so need to free this. freeArray(vars); } @@ -1146,7 +1146,7 @@ private void setBoundaries(CORINFO_METHOD_STRUCT_* ftn, uint cMap, OffsetMapping { _debugLocInfos[i] = pMap[i]; } - + // JIT gave the ownership of this to us, so need to free this. freeArray(pMap); } From 9024f8cc9d1a1a510d92b91386027c1ddbac8008 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 04:49:46 +0200 Subject: [PATCH 5/8] More cleanups --- src/coreclr/vm/corelib.cpp | 23 +++++---------- src/coreclr/vm/dllimport.cpp | 1 - src/coreclr/vm/ecall.cpp | 50 --------------------------------- src/coreclr/vm/ecall.h | 1 - src/coreclr/vm/ecalllist.h | 24 ++++++++-------- src/coreclr/vm/jitinterface.cpp | 5 ---- src/coreclr/vm/stubhelpers.cpp | 10 ++----- 7 files changed, 21 insertions(+), 93 deletions(-) diff --git a/src/coreclr/vm/corelib.cpp b/src/coreclr/vm/corelib.cpp index dabd4589cf5fe..707cd24274148 100644 --- a/src/coreclr/vm/corelib.cpp +++ b/src/coreclr/vm/corelib.cpp @@ -114,7 +114,6 @@ enum _gsigc { #include "metasig.h" }; - // // The actual array with the hardcoded metasig: // @@ -176,8 +175,6 @@ enum _gsigc { #undef _IM #undef _Fld - - #ifdef _DEBUG // @@ -213,10 +210,6 @@ enum _gsigc { #endif - - - - /////////////////////////////////////////////////////////////////////////////// // // CoreLib binder @@ -271,30 +264,28 @@ const USHORT c_nCoreLibFieldDescriptions = ARRAY_SIZE(c_rgCoreLibFieldDescriptio // When compiling crossgen, we only need the target version of the ecall tables -#define FCFuncFlags(intrinsicID, dynamicID) \ - (BYTE*)( (((BYTE)intrinsicID) << 16) + (((BYTE)dynamicID) << 24) ) +#define FCFuncFlags(dynamicID) \ + (BYTE*)( (((BYTE)dynamicID) << 24) ) - -#define FCFuncElement(name, impl) FCFuncFlags(CORINFO_INTRINSIC_Illegal, ECall::InvalidDynamicFCallId), \ +#define FCFuncElement(name, impl) FCFuncFlags(ECall::InvalidDynamicFCallId), \ (LPVOID)GetEEFuncEntryPoint(impl), (LPVOID)name, #define FCFuncElementSig(name,sig,impl) \ FCFuncFlag_HasSignature + FCFuncElement(name, impl) (LPVOID)sig, -#define FCDynamic(name,intrinsicID,dynamicID) FCFuncFlags(intrinsicID, dynamicID), \ +#define FCDynamic(name,dynamicID) FCFuncFlags(dynamicID), \ NULL, (LPVOID)name, -#define FCDynamicSig(name,sig,intrinsicID,dynamicID) \ - FCFuncFlag_HasSignature + FCDynamic(name,intrinsicID,dynamicID) (LPVOID)sig, +#define FCDynamicSig(name,sig,dynamicID) \ + FCFuncFlag_HasSignature + FCDynamic(name,dynamicID) (LPVOID)sig, #define FCUnreferenced FCFuncFlag_Unreferenced + #define FCFuncStart(name) static const LPVOID name[] = { -#define FCFuncEnd() FCFuncFlag_EndOfArray + FCFuncFlags(CORINFO_INTRINSIC_Illegal, ECall::InvalidDynamicFCallId) }; +#define FCFuncEnd() FCFuncFlag_EndOfArray + FCFuncFlags(ECall::InvalidDynamicFCallId) }; #include "ecalllist.h" - // Extern definitions so that ecall.cpp can see these tables extern const ECClass c_rgECClasses[]; extern const int c_nECClasses; diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index c62c2c96fbdeb..5cb781568681c 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -2081,7 +2081,6 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth // get the delegate unmanaged target - we call a helper instead of just grabbing // the _methodPtrAux field because we may need to intercept the call for host, etc. pcsEmit->EmitLoadThis(); - pcsEmit->EmitLDNULL(); pcsEmit->EmitCALL(METHOD__STUBHELPERS__GET_DELEGATE_TARGET, 2, 1); } else // direct invocation diff --git a/src/coreclr/vm/ecall.cpp b/src/coreclr/vm/ecall.cpp index 439e9d9beced2..ad93743a9cbfa 100644 --- a/src/coreclr/vm/ecall.cpp +++ b/src/coreclr/vm/ecall.cpp @@ -641,56 +641,6 @@ MethodDesc* ECall::MapTargetBackToMethod(PCODE pTarg, PCODE * ppAdjustedEntryPoi #ifndef DACCESS_COMPILE -/* static */ -CorInfoIntrinsics ECall::GetIntrinsicID(MethodDesc* pMD) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_ANY; - PRECONDITION(pMD->IsFCall()); - } - CONTRACTL_END; - - MethodTable * pMT = pMD->GetMethodTable(); - -#ifdef FEATURE_COMINTEROP - // COM imported classes have special constructors - if (pMT->IsComObjectType()) - { - // This has to be tlbimp constructor - return(CORINFO_INTRINSIC_Illegal); - } -#endif // FEATURE_COMINTEROP - - // - // Delegate constructors are FCalls for which the entrypoint points to the target of the delegate - // We have to intercept these and set the call target to the helper COMDelegate::DelegateConstruct - // - if (pMT->IsDelegate()) - { - // COMDelegate::DelegateConstruct is the only fcall used by user delegates. - // All the other gDelegateFuncs are only used by System.Delegate - _ASSERTE(pMD->IsCtor()); - - return(CORINFO_INTRINSIC_Illegal); - } - - // All intrinsic live in CoreLib (FindECFuncForMethod does not work for non-CoreLib intrinsics) - if (!pMD->GetModule()->IsSystem()) - { - return(CORINFO_INTRINSIC_Illegal); - } - - ECFunc* info = FindECFuncForMethod(pMD); - - if (info == NULL) - return(CORINFO_INTRINSIC_Illegal); - - return info->IntrinsicID(); -} - #ifdef _DEBUG void FCallAssert(void*& cache, void* target) diff --git a/src/coreclr/vm/ecall.h b/src/coreclr/vm/ecall.h index f5c8ac646b440..1a538db3c67e5 100644 --- a/src/coreclr/vm/ecall.h +++ b/src/coreclr/vm/ecall.h @@ -83,7 +83,6 @@ class ECall static PCODE GetFCallImpl(MethodDesc* pMD, BOOL * pfSharedOrDynamicFCallImpl = NULL); static MethodDesc* MapTargetBackToMethod(PCODE pTarg, PCODE * ppAdjustedEntryPoint = NULL); static DWORD GetIDForMethod(MethodDesc *pMD); - static CorInfoIntrinsics GetIntrinsicID(MethodDesc *pMD); // Some fcalls (delegate ctors and tlbimpl ctors) shared one implementation. // We should never patch vtable for these since they have 1:N mapping between diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 8a593ab1f2566..be7a6826854f0 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -15,11 +15,11 @@ #endif #ifndef FCDynamic -#define FCDynamic(name,intrinsicID,dynamicID) +#define FCDynamic(name,dynamicID) #endif #ifndef FCDynamicSig -#define FCDynamicSig(name,sig,intrinsicID,dynamicID) +#define FCDynamicSig(name,sig,dynamicID) #endif #ifndef FCUnreferenced @@ -65,16 +65,16 @@ FCFuncStart(gObjectFuncs) FCFuncEnd() FCFuncStart(gStringFuncs) - FCDynamic("FastAllocateString", CORINFO_INTRINSIC_Illegal, ECall::FastAllocateString) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ArrChar_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorCharArrayManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ArrChar_Int_Int_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorCharArrayStartLengthManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrChar_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorCharPtrManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrChar_Int_Int_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorCharPtrStartLengthManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_Char_Int_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorCharCountManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ReadOnlySpanOfChar_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorReadOnlySpanOfCharManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorSBytePtrManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_Int_Int_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorSBytePtrStartLengthManaged) - FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_Int_Int_Encoding_RetVoid, CORINFO_INTRINSIC_Illegal, ECall::CtorSBytePtrStartLengthEncodingManaged) + FCDynamic("FastAllocateString", ECall::FastAllocateString) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ArrChar_RetVoid, ECall::CtorCharArrayManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ArrChar_Int_Int_RetVoid, ECall::CtorCharArrayStartLengthManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrChar_RetVoid, ECall::CtorCharPtrManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrChar_Int_Int_RetVoid, ECall::CtorCharPtrStartLengthManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_Char_Int_RetVoid, ECall::CtorCharCountManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ReadOnlySpanOfChar_RetVoid, ECall::CtorReadOnlySpanOfCharManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_RetVoid, ECall::CtorSBytePtrManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_Int_Int_RetVoid, ECall::CtorSBytePtrStartLengthManaged) + FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_Int_Int_Encoding_RetVoid, ECall::CtorSBytePtrStartLengthEncodingManaged) FCFuncElement("SetTrailByte", COMString::FCSetTrailByte) FCFuncElement("TryGetTrailByte", COMString::FCTryGetTrailByte) FCFuncElement("IsInterned", AppDomainNative::IsStringInterned) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index a20a1c051f8e4..1433856f3378a 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8365,11 +8365,6 @@ CorInfoIntrinsics CEEInfo::getIntrinsicID(CORINFO_METHOD_HANDLE methodHnd, result = arrMethod->GetIntrinsicID(); } else - if (method->IsFCall()) - { - result = ECall::GetIntrinsicID(method); - } - else { MethodTable * pMT = method->GetMethodTable(); if (pMT->GetModule()->IsSystem() && pMT->IsByRefLike()) diff --git a/src/coreclr/vm/stubhelpers.cpp b/src/coreclr/vm/stubhelpers.cpp index 5c7a09ca897fa..4fb4e9ff8c656 100644 --- a/src/coreclr/vm/stubhelpers.cpp +++ b/src/coreclr/vm/stubhelpers.cpp @@ -524,15 +524,9 @@ FCIMPL1(void*, StubHelpers::GetDelegateTarget, DelegateObject *pThisUNSAFE) // See code:GenericPInvokeCalliHelper // The lowest bit is used to distinguish between MD and target on 64-bit. target = (target << 1) | 1; +#endif // HOST_64BIT -#elif defined(TARGET_ARM) - // @ARMTODO: Nothing to do for ARM yet since we don't support the hosted path. -#endif // HOST_64BIT, TARGET_ARM - - if (pEntryPoint == NULL) - { - pEntryPoint = orefThis->GetMethodPtrAux(); - } + pEntryPoint = orefThis->GetMethodPtrAux(); #ifdef _DEBUG END_PRESERVE_LAST_ERROR; From 07ee3a83b3edda356a1b2b20b643d1be3f8042be Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 06:17:01 +0200 Subject: [PATCH 6/8] Update src/coreclr/vm/dllimport.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/dllimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 5cb781568681c..4960defe52c28 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -2081,7 +2081,7 @@ void NDirectStubLinker::DoNDirect(ILCodeStream *pcsEmit, DWORD dwStubFlags, Meth // get the delegate unmanaged target - we call a helper instead of just grabbing // the _methodPtrAux field because we may need to intercept the call for host, etc. pcsEmit->EmitLoadThis(); - pcsEmit->EmitCALL(METHOD__STUBHELPERS__GET_DELEGATE_TARGET, 2, 1); + pcsEmit->EmitCALL(METHOD__STUBHELPERS__GET_DELEGATE_TARGET, 1, 1); } else // direct invocation { From 50ac5626f88cc7b7c1edd21a3ddc7bb90768aa07 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 06:49:19 +0200 Subject: [PATCH 7/8] Fix method definition --- src/coreclr/vm/corelib.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index a24c2f752f4a7..6fb9d0e3e3f21 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -998,7 +998,7 @@ DEFINE_METHOD(BUFFER, MEMCPY, Memcpy, DEFINE_CLASS(STUBHELPERS, StubHelpers, StubHelpers) DEFINE_METHOD(STUBHELPERS, GET_NDIRECT_TARGET, GetNDirectTarget, SM_IntPtr_RetIntPtr) -DEFINE_METHOD(STUBHELPERS, GET_DELEGATE_TARGET, GetDelegateTarget, SM_Delegate_RefIntPtr_RetIntPtr) +DEFINE_METHOD(STUBHELPERS, GET_DELEGATE_TARGET, GetDelegateTarget, SM_Delegate_RetIntPtr) #ifdef FEATURE_COMINTEROP DEFINE_METHOD(STUBHELPERS, GET_COM_HR_EXCEPTION_OBJECT, GetCOMHRExceptionObject, SM_Int_IntPtr_Obj_RetException) DEFINE_METHOD(STUBHELPERS, GET_COM_IP_FROM_RCW, GetCOMIPFromRCW, SM_Obj_IntPtr_RefIntPtr_RefBool_RetIntPtr) From 8031b244f0c79651c3dc420c2a07b7e80c74d21a Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Thu, 9 Dec 2021 07:07:11 +0200 Subject: [PATCH 8/8] More cleanups --- src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs index cf2061e249d4d..41735099fb2e7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs @@ -1181,7 +1181,7 @@ internal static class StubHelpers internal static extern IntPtr GetNDirectTarget(IntPtr pMD); [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern IntPtr GetDelegateTarget(Delegate pThis, ref IntPtr pStubArg); + internal static extern IntPtr GetDelegateTarget(Delegate pThis); [MethodImpl(MethodImplOptions.InternalCall)] internal static extern void ClearLastError();