From 52065e1bcda642685e28f4d9ea4ebae77547e554 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 May 2023 02:59:45 +0200 Subject: [PATCH 01/15] JIT: Fold typeof(T).TypeHandle.Value --- src/coreclr/vm/jitinterface.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 62a4400c7d86..f183fa716dca 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12001,11 +12001,25 @@ bool CEEInfo::getObjectContent(CORINFO_OBJECT_HANDLE handle, uint8_t* buffer, in _ASSERTE(objRef != NULL); // TODO: support types containing GC pointers - if (!objRef->GetMethodTable()->ContainsPointers() && bufferSize + valueOffset <= (int)objRef->GetSize()) + if (bufferSize + valueOffset <= (int)objRef->GetSize()) { Object* obj = OBJECTREFToObject(objRef); - memcpy(buffer, (uint8_t*)obj + valueOffset, bufferSize); - result = true; + PTR_MethodTable type = obj->GetMethodTable(); + if (type->ContainsPointers()) + { + // RuntimeType has a gc field (object m_keepAlive), but if the object is in a frozen segment + // it means that field is always nullptr so we can read any part of the object: + if (type == g_pRuntimeTypeClass && GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(obj)) + { + memcpy(buffer, (uint8_t*)obj + valueOffset, bufferSize); + result = true; + } + } + else + { + memcpy(buffer, (uint8_t*)obj + valueOffset, bufferSize); + result = true; + } } EE_TO_JIT_TRANSITION(); From 04210ec833f6574259fbe8fba88b2d11abb672b3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 May 2023 21:52:42 +0200 Subject: [PATCH 02/15] Handle NativeAOT --- src/coreclr/jit/fgbasic.cpp | 1 + src/coreclr/jit/importercalls.cpp | 25 +++++++++++++++++++ src/coreclr/jit/namedintrinsiclist.h | 1 + .../System.Private.CoreLib/src/System/Type.cs | 9 ++++++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 5a8f5f9ff0bc..36cdad5c56d5 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -1156,6 +1156,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed case NI_PRIMITIVE_TrailingZeroCount: case NI_System_Type_get_IsEnum: case NI_System_Type_GetEnumUnderlyingType: + case NI_System_Type_get_TypeHandle: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsByRefLike: case NI_System_Type_GetTypeFromHandle: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index f952836c7048..9a9dbada722b 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2595,6 +2595,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_Type_op_Inequality: // These may lead to early dead code elimination + case NI_System_Type_get_TypeHandle: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsEnum: case NI_System_Type_get_IsByRefLike: @@ -3093,6 +3094,26 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + case NI_System_Type_get_TypeHandle: + { + assert(IsTargetAbi(CORINFO_NATIVEAOT_ABI)); + + // Try to avoid ClsHandle -> Type object -> ClsHandle roundtrip: + GenTree* op1 = impStackTop(0).val; + if (op1->IsCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall())) + { + unsigned structLcl = lvaGrabTemp(true DEBUGARG("RuntimeTypeHandle")); + lvaSetStruct(structLcl, sig->retTypeClass, false); + GenTreeLclFld* handleFld = gtNewLclFldNode(structLcl, TYP_I_IMPL, 0); + GenTree* realHandle = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode(); + GenTree* asgHandleFld = gtNewAssignNode(handleFld, realHandle); + impAppendTree(asgHandleFld, CHECK_SPILL_NONE, impCurStmtDI); + retNode = impCreateLocalNode(structLcl DEBUGARG(0)); + impPopStack(); + } + break; + } + case NI_System_Type_get_IsEnum: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsByRefLike: @@ -8211,6 +8232,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_Type_op_Inequality; } + else if (strcmp(methodName, "get_TypeHandle") == 0) + { + result = NI_System_Type_get_TypeHandle; + } } break; } diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index 2e3f1f0a013b..9dc739a76bec 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -66,6 +66,7 @@ enum NamedIntrinsic : unsigned short NI_System_Type_GetEnumUnderlyingType, NI_System_Type_get_IsValueType, NI_System_Type_get_IsByRefLike, + NI_System_Type_get_TypeHandle, NI_System_Type_IsAssignableFrom, NI_System_Type_IsAssignableTo, NI_System_Type_op_Equality, diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index 28fd7905e577..425e86cd367b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -424,7 +424,14 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe | DynamicallyAccessedMemberTypes.PublicNestedTypes)] public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; - public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); + public virtual RuntimeTypeHandle TypeHandle + { +#if NATIVEAOT + [Intrinsic] +#endif + get => throw new NotSupportedException(); + } + public static RuntimeTypeHandle GetTypeHandle(object o) { ArgumentNullException.ThrowIfNull(o); From 3c81b797716a5d223a1e993a2ef8bb65eb12cb3b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 May 2023 22:15:39 +0200 Subject: [PATCH 03/15] Clean up --- src/coreclr/jit/importercalls.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 9a9dbada722b..50577e1ee877 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3100,12 +3100,15 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // Try to avoid ClsHandle -> Type object -> ClsHandle roundtrip: GenTree* op1 = impStackTop(0).val; - if (op1->IsCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall())) + if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall())) { + // struct RuntimeTypeHandle { IntPtr _value; } + assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); + unsigned structLcl = lvaGrabTemp(true DEBUGARG("RuntimeTypeHandle")); lvaSetStruct(structLcl, sig->retTypeClass, false); - GenTreeLclFld* handleFld = gtNewLclFldNode(structLcl, TYP_I_IMPL, 0); GenTree* realHandle = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode(); + GenTreeLclFld* handleFld = gtNewLclFldNode(structLcl, realHandle->TypeGet(), 0); GenTree* asgHandleFld = gtNewAssignNode(handleFld, realHandle); impAppendTree(asgHandleFld, CHECK_SPILL_NONE, impCurStmtDI); retNode = impCreateLocalNode(structLcl DEBUGARG(0)); From e4d391b09c9aa6e3f5292caf8f2df6f7fc8c0b42 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 5 May 2023 23:58:28 +0200 Subject: [PATCH 04/15] fix test --- src/coreclr/jit/importercalls.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 50577e1ee877..9a2207a33a0e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3100,7 +3100,8 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // Try to avoid ClsHandle -> Type object -> ClsHandle roundtrip: GenTree* op1 = impStackTop(0).val; - if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall())) + if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && + (methodFlags & CORINFO_FLG_VIRTUAL)) { // struct RuntimeTypeHandle { IntPtr _value; } assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); From 375df02736a0dc5cca86b263641c8f206bcc3700 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 6 May 2023 01:53:25 +0200 Subject: [PATCH 05/15] check callvirt explicitly --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/importercalls.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 64ae8d35e6fd..c37cc9cf172f 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3938,6 +3938,7 @@ class Compiler int memberRef, bool readonlyCall, bool tailCall, + bool callvirt, CORINFO_RESOLVED_TOKEN* pContstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 9a2207a33a0e..9684d96589a3 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -233,9 +233,9 @@ 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, &ni, &isSpecialIntrinsic); + call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token, isReadonlyCall, + isTailCall, opcode == CEE_CALLVIRT, pConstrainedResolvedToken, callInfo->thisTransform, + &ni, &isSpecialIntrinsic); if (compDonotInline()) { @@ -2304,6 +2304,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, int memberRef, bool readonlyCall, bool tailCall, + bool callvirt, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, @@ -3100,8 +3101,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // Try to avoid ClsHandle -> Type object -> ClsHandle roundtrip: GenTree* op1 = impStackTop(0).val; - if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && - (methodFlags & CORINFO_FLG_VIRTUAL)) + if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt) { // struct RuntimeTypeHandle { IntPtr _value; } assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); From 7716934feb605ff994239005aef166412be3a09e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 7 May 2023 17:18:39 +0200 Subject: [PATCH 06/15] Address feedback --- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/fgbasic.cpp | 1 - src/coreclr/jit/importercalls.cpp | 59 ++++++++----------- src/coreclr/jit/namedintrinsiclist.h | 1 + .../src/System/RuntimeType.cs | 7 ++- .../System.Private.CoreLib/src/System/Type.cs | 8 +-- 6 files changed, 34 insertions(+), 43 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 938a4fc0cacc..6678a0a6d9f2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3948,7 +3948,6 @@ class Compiler int memberRef, bool readonlyCall, bool tailCall, - bool callvirt, CORINFO_RESOLVED_TOKEN* pContstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index c05ae0ba13f1..cb627fd3b434 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -1156,7 +1156,6 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed case NI_PRIMITIVE_TrailingZeroCount: case NI_System_Type_get_IsEnum: case NI_System_Type_GetEnumUnderlyingType: - case NI_System_Type_get_TypeHandle: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsByRefLike: case NI_System_Type_GetTypeFromHandle: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 44799c1e0971..71c88f54fcdb 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -234,7 +234,7 @@ var_types Compiler::impImportCall(OPCODE opcode, const bool isTailCall = canTailCall && (tailCallFlags != 0); call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token, isReadonlyCall, - isTailCall, opcode == CEE_CALLVIRT, pConstrainedResolvedToken, callInfo->thisTransform, + isTailCall, pConstrainedResolvedToken, callInfo->thisTransform, &ni, &isSpecialIntrinsic); if (compDonotInline()) @@ -2304,7 +2304,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, int memberRef, bool readonlyCall, bool tailCall, - bool callvirt, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, @@ -2591,12 +2590,12 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, // We need these to be able to fold "typeof(...) == typeof(...)" case NI_System_RuntimeTypeHandle_ToIntPtr: + case NI_System_RuntimeType_get_TypeHandle: case NI_System_Type_GetTypeFromHandle: case NI_System_Type_op_Equality: case NI_System_Type_op_Inequality: // These may lead to early dead code elimination - case NI_System_Type_get_TypeHandle: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsEnum: case NI_System_Type_get_IsByRefLike: @@ -2972,8 +2971,8 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_RuntimeTypeHandle_ToIntPtr: { GenTree* op1 = impStackTop(0).val; - if (op1->gtOper == GT_CALL && (op1->AsCall()->gtCallType == CT_HELPER) && - gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall())) + + if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHandleHelper(op1->AsCall())) { // Old tree // Helper-RuntimeTypeHandle -> TreeToGetNativeTypeHandle @@ -2991,7 +2990,24 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, op1 = op1->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); retNode = op1; } - // Call the regular function. + else if (op1->OperIs(GT_RET_EXPR)) + { + // Skip roundtrip "handle -> RuntimeType -> handle" for RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) + GenTreeCall* call = op1->AsRetExpr()->gtInlineCandidate; + if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_RuntimeType_get_TypeHandle) + { + // Check that the arg is CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE helper call + GenTree* arg = call->gtArgs.GetArgByIndex(0)->GetNode(); + if (arg->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(arg->AsCall())) + { + impPopStack(); + // Bash the RET_EXPR to no-op since it's unused now + op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP(); + // Skip roundtrip and return the type handle directly + retNode = arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); + } + } + } break; } @@ -3091,29 +3107,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } - case NI_System_Type_get_TypeHandle: - { - assert(IsTargetAbi(CORINFO_NATIVEAOT_ABI)); - - // Try to avoid ClsHandle -> Type object -> ClsHandle roundtrip: - GenTree* op1 = impStackTop(0).val; - if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt) - { - // struct RuntimeTypeHandle { IntPtr _value; } - assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); - - unsigned structLcl = lvaGrabTemp(true DEBUGARG("RuntimeTypeHandle")); - lvaSetStruct(structLcl, sig->retTypeClass, false); - GenTree* realHandle = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode(); - GenTreeLclFld* handleFld = gtNewLclFldNode(structLcl, realHandle->TypeGet(), 0); - GenTree* asgHandleFld = gtNewAssignNode(handleFld, realHandle); - impAppendTree(asgHandleFld, CHECK_SPILL_NONE, impCurStmtDI); - retNode = impCreateLocalNode(structLcl DEBUGARG(0)); - impPopStack(); - } - break; - } - case NI_System_Type_get_IsEnum: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsByRefLike: @@ -8134,6 +8127,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_Type_get_IsEnum; } + if (strcmp(methodName, "get_TypeHandle") == 0) + { + result = NI_System_RuntimeType_get_TypeHandle; + } } else if (strcmp(className, "RuntimeTypeHandle") == 0) { @@ -8231,10 +8228,6 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_Type_op_Inequality; } - else if (strcmp(methodName, "get_TypeHandle") == 0) - { - result = NI_System_Type_get_TypeHandle; - } } break; } diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index 1a79bb189a39..50fb3e3ea214 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -79,6 +79,7 @@ enum NamedIntrinsic : unsigned short NI_System_Object_MemberwiseClone, NI_System_Object_GetType, NI_System_RuntimeTypeHandle_ToIntPtr, + NI_System_RuntimeType_get_TypeHandle, NI_System_StubHelpers_GetStubContext, NI_System_StubHelpers_NextCallReturnAddress, diff --git a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs index 176a1f75b147..943b00ad215a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs +++ b/src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs @@ -25,7 +25,12 @@ internal sealed partial class RuntimeType : TypeInfo, ICloneable public override int MetadataToken => RuntimeTypeHandle.GetToken(this); public override Module Module => GetRuntimeModule(); public override Type? ReflectedType => DeclaringType; - public override RuntimeTypeHandle TypeHandle => new RuntimeTypeHandle(this); + public override RuntimeTypeHandle TypeHandle + { + [Intrinsic] // to avoid round-trip "handle -> RuntimeType -> handle" in JIT + get => new RuntimeTypeHandle(this); + } + public override Type UnderlyingSystemType => this; public object Clone() => this; diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index 425e86cd367b..fedda2c38c12 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -424,13 +424,7 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe | DynamicallyAccessedMemberTypes.PublicNestedTypes)] public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; - public virtual RuntimeTypeHandle TypeHandle - { -#if NATIVEAOT - [Intrinsic] -#endif - get => throw new NotSupportedException(); - } + public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); public static RuntimeTypeHandle GetTypeHandle(object o) { From 726a42f60f141365db88b38ae540a8119c1ded23 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 7 May 2023 17:19:18 +0200 Subject: [PATCH 07/15] formatting --- src/coreclr/jit/importercalls.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 71c88f54fcdb..db43a3f54fd7 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -233,9 +233,9 @@ 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, - &ni, &isSpecialIntrinsic); + call = + impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token, isReadonlyCall, + isTailCall, pConstrainedResolvedToken, callInfo->thisTransform, &ni, &isSpecialIntrinsic); if (compDonotInline()) { @@ -2992,7 +2992,8 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, } else if (op1->OperIs(GT_RET_EXPR)) { - // Skip roundtrip "handle -> RuntimeType -> handle" for RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) + // Skip roundtrip "handle -> RuntimeType -> handle" for + // RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) GenTreeCall* call = op1->AsRetExpr()->gtInlineCandidate; if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_RuntimeType_get_TypeHandle) { From 5b313af5099765244dffe5cbef34f5dbb808898f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 7 May 2023 17:20:51 +0200 Subject: [PATCH 08/15] formatting --- src/coreclr/jit/namedintrinsiclist.h | 1 - src/libraries/System.Private.CoreLib/src/System/Type.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index 50fb3e3ea214..b1cc6281253e 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -66,7 +66,6 @@ enum NamedIntrinsic : unsigned short NI_System_Type_GetEnumUnderlyingType, NI_System_Type_get_IsValueType, NI_System_Type_get_IsByRefLike, - NI_System_Type_get_TypeHandle, NI_System_Type_IsAssignableFrom, NI_System_Type_IsAssignableTo, NI_System_Type_op_Equality, diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index fedda2c38c12..28fd7905e577 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -425,7 +425,6 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); - public static RuntimeTypeHandle GetTypeHandle(object o) { ArgumentNullException.ThrowIfNull(o); From 5bdfc2c6d5e694c1d543300c9e5d27fb57134968 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 8 May 2023 15:23:52 +0200 Subject: [PATCH 09/15] Address feedback --- .../src/System/GC.CoreCLR.cs | 15 ++----- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/importercalls.cpp | 39 +++++++++++++++++-- src/coreclr/jit/namedintrinsiclist.h | 1 + .../System.Private.CoreLib/src/System/Type.cs | 9 ++++- 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs index c68bc6aa5d3e..539b10e2aa02 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs @@ -671,18 +671,11 @@ internal static void UnregisterMemoryLoadChangeNotification(Action notification) ThrowHelper.ThrowInvalidTypeWithPointersNotSupported(typeof(T)); } - // kept outside of the small arrays hot path to have inlining without big size growth - return AllocateNewUninitializedArray(length, pinned); - - // remove the local function when https://github.com/dotnet/runtime/issues/5973 is implemented - static T[] AllocateNewUninitializedArray(int length, bool pinned) - { - GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_ZEROING_OPTIONAL; - if (pinned) - flags |= GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP; + GC_ALLOC_FLAGS flags = GC_ALLOC_FLAGS.GC_ALLOC_ZEROING_OPTIONAL; + if (pinned) + flags |= GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP; - return Unsafe.As(AllocateNewArray(RuntimeTypeHandle.ToIntPtr(typeof(T[]).TypeHandle), length, flags)); - } + return Unsafe.As(AllocateNewArray(RuntimeTypeHandle.ToIntPtr(typeof(T[]).TypeHandle), length, flags)); } /// diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6678a0a6d9f2..938a4fc0cacc 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3948,6 +3948,7 @@ class Compiler int memberRef, bool readonlyCall, bool tailCall, + bool callvirt, CORINFO_RESOLVED_TOKEN* pContstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index db43a3f54fd7..1d4657156390 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -233,9 +233,9 @@ 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, &ni, &isSpecialIntrinsic); + call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken->token, isReadonlyCall, + isTailCall, opcode == CEE_CALLVIRT, pConstrainedResolvedToken, callInfo->thisTransform, + &ni, &isSpecialIntrinsic); if (compDonotInline()) { @@ -2304,6 +2304,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, int memberRef, bool readonlyCall, bool tailCall, + bool callvirt, CORINFO_RESOLVED_TOKEN* pConstrainedResolvedToken, CORINFO_THIS_TRANSFORM constraintCallThisTransform, NamedIntrinsic* pIntrinsicName, @@ -2589,7 +2590,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant: // We need these to be able to fold "typeof(...) == typeof(...)" - case NI_System_RuntimeTypeHandle_ToIntPtr: case NI_System_RuntimeType_get_TypeHandle: case NI_System_Type_GetTypeFromHandle: case NI_System_Type_op_Equality: @@ -2615,6 +2615,8 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_BitConverter_SingleToInt32Bits: case NI_System_Buffers_Binary_BinaryPrimitives_ReverseEndianness: case NI_System_Type_GetEnumUnderlyingType: + case NI_System_Type_get_TypeHandle: + case NI_System_RuntimeTypeHandle_ToIntPtr: // Most atomics are compiled to single instructions case NI_System_Threading_Interlocked_And: @@ -3108,6 +3110,31 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, break; } + case NI_System_Type_get_TypeHandle: + { + assert(IsTargetAbi(CORINFO_NATIVEAOT_ABI)); + + // We can only expand this on NativeAOT where RuntimeTypeHandle looks like this: + // + // struct RuntimeTypeHandle { IntPtr _value; } + // + GenTree* op1 = impStackTop(0).val; + if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt) + { + assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); + + unsigned structLcl = lvaGrabTemp(true DEBUGARG("RuntimeTypeHandle")); + lvaSetStruct(structLcl, sig->retTypeClass, false); + GenTree* realHandle = op1->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode(); + GenTreeLclFld* handleFld = gtNewLclFldNode(structLcl, realHandle->TypeGet(), 0); + GenTree* asgHandleFld = gtNewAssignNode(handleFld, realHandle); + impAppendTree(asgHandleFld, CHECK_SPILL_NONE, impCurStmtDI); + retNode = impCreateLocalNode(structLcl DEBUGARG(0)); + impPopStack(); + } + break; + } + case NI_System_Type_get_IsEnum: case NI_System_Type_get_IsValueType: case NI_System_Type_get_IsByRefLike: @@ -8229,6 +8256,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { result = NI_System_Type_op_Inequality; } + else if (strcmp(methodName, "get_TypeHandle") == 0) + { + result = NI_System_Type_get_TypeHandle; + } } break; } diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h index b1cc6281253e..50fb3e3ea214 100644 --- a/src/coreclr/jit/namedintrinsiclist.h +++ b/src/coreclr/jit/namedintrinsiclist.h @@ -66,6 +66,7 @@ enum NamedIntrinsic : unsigned short NI_System_Type_GetEnumUnderlyingType, NI_System_Type_get_IsValueType, NI_System_Type_get_IsByRefLike, + NI_System_Type_get_TypeHandle, NI_System_Type_IsAssignableFrom, NI_System_Type_IsAssignableTo, NI_System_Type_op_Equality, diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index 28fd7905e577..425e86cd367b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -424,7 +424,14 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe | DynamicallyAccessedMemberTypes.PublicNestedTypes)] public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; - public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); + public virtual RuntimeTypeHandle TypeHandle + { +#if NATIVEAOT + [Intrinsic] +#endif + get => throw new NotSupportedException(); + } + public static RuntimeTypeHandle GetTypeHandle(object o) { ArgumentNullException.ThrowIfNull(o); From 7821c85548f488a857fb52adf0b9bfaef9ffa2c6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 8 May 2023 17:40:15 +0200 Subject: [PATCH 10/15] address feedback --- src/coreclr/jit/importercalls.cpp | 5 ++--- src/libraries/System.Private.CoreLib/src/System/Type.cs | 9 +-------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 1d4657156390..4527bcf3a008 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3112,14 +3112,13 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_Type_get_TypeHandle: { - assert(IsTargetAbi(CORINFO_NATIVEAOT_ABI)); - // We can only expand this on NativeAOT where RuntimeTypeHandle looks like this: // // struct RuntimeTypeHandle { IntPtr _value; } // GenTree* op1 = impStackTop(0).val; - if (op1->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt) + if (IsTargetAbi(CORINFO_NATIVEAOT_ABI) && op1->IsHelperCall() && + gtIsTypeHandleToRuntimeTypeHelper(op1->AsCall()) && callvirt) { assert(info.compCompHnd->getClassNumInstanceFields(sig->retTypeClass) == 1); diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index 425e86cd367b..28fd7905e577 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -424,14 +424,7 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe | DynamicallyAccessedMemberTypes.PublicNestedTypes)] public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; - public virtual RuntimeTypeHandle TypeHandle - { -#if NATIVEAOT - [Intrinsic] -#endif - get => throw new NotSupportedException(); - } - + public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); public static RuntimeTypeHandle GetTypeHandle(object o) { ArgumentNullException.ThrowIfNull(o); From 7ab11e51441e382b712c794f91a6a76578932889 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 8 May 2023 17:41:06 +0200 Subject: [PATCH 11/15] oops, reverted too much --- src/libraries/System.Private.CoreLib/src/System/Type.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Type.cs b/src/libraries/System.Private.CoreLib/src/System/Type.cs index 28fd7905e577..87b03dcfcbdf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Type.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Type.cs @@ -424,7 +424,12 @@ public virtual MemberInfo GetMemberWithSameMetadataDefinitionAs(MemberInfo membe | DynamicallyAccessedMemberTypes.PublicNestedTypes)] public virtual MemberInfo[] GetDefaultMembers() => throw NotImplemented.ByDesign; - public virtual RuntimeTypeHandle TypeHandle => throw new NotSupportedException(); + public virtual RuntimeTypeHandle TypeHandle + { + [Intrinsic] + get => throw new NotSupportedException(); + } + public static RuntimeTypeHandle GetTypeHandle(object o) { ArgumentNullException.ThrowIfNull(o); From ec522f2eb0c4e50163293a12bc7c05247dc75366 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 8 May 2023 17:50:46 +0200 Subject: [PATCH 12/15] Addressed feedback --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 4527bcf3a008..f1cecd5264f9 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2590,7 +2590,6 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant: // We need these to be able to fold "typeof(...) == typeof(...)" - case NI_System_RuntimeType_get_TypeHandle: case NI_System_Type_GetTypeFromHandle: case NI_System_Type_op_Equality: case NI_System_Type_op_Inequality: @@ -2616,6 +2615,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, case NI_System_Buffers_Binary_BinaryPrimitives_ReverseEndianness: case NI_System_Type_GetEnumUnderlyingType: case NI_System_Type_get_TypeHandle: + case NI_System_RuntimeType_get_TypeHandle: case NI_System_RuntimeTypeHandle_ToIntPtr: // Most atomics are compiled to single instructions From 473a36113c228376e10b6104d836b1a830fdc3a9 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 10 May 2023 00:49:34 +0200 Subject: [PATCH 13/15] Update src/coreclr/jit/importercalls.cpp Co-authored-by: Andy Ayers --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 1921e56856d0..dd4dd449fd55 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3005,7 +3005,7 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, if (arg->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(arg->AsCall())) { impPopStack(); - // Bash the RET_EXPR to no-op since it's unused now + // Bash the RET_EXPR's call to no-op since it's unused now op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP(); // Skip roundtrip and return the type handle directly retNode = arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); From 0442988c6548589648db0e0e26db1a47b3b26099 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 10 May 2023 17:21:08 +0200 Subject: [PATCH 14/15] formatting --- src/coreclr/jit/importercalls.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index a9891ee7f75a..247f70bf6f60 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -225,8 +225,8 @@ var_types Compiler::impImportCall(OPCODE opcode, const bool isTailCall = canTailCall && (tailCallFlags != 0); call = impIntrinsic(newobjThis, clsHnd, methHnd, sig, mflags, pResolvedToken, isReadonlyCall, isTailCall, - opcode == CEE_CALLVIRT, pConstrainedResolvedToken, callInfo->thisTransform, - &ni, &isSpecialIntrinsic); + opcode == CEE_CALLVIRT, pConstrainedResolvedToken, callInfo->thisTransform, &ni, + &isSpecialIntrinsic); if (compDonotInline()) { From 0e4b625ff4d5b4447cec5eaa193fe625c26d0f75 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 10 May 2023 17:22:36 +0200 Subject: [PATCH 15/15] address feedback --- src/coreclr/jit/importercalls.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 247f70bf6f60..2db73c09d38f 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2978,11 +2978,11 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, op1 = op1->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); retNode = op1; } - else if (op1->OperIs(GT_RET_EXPR)) + else if (op1->OperIs(GT_CALL, GT_RET_EXPR)) { // Skip roundtrip "handle -> RuntimeType -> handle" for // RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle) - GenTreeCall* call = op1->AsRetExpr()->gtInlineCandidate; + GenTreeCall* call = op1->IsCall() ? op1->AsCall() : op1->AsRetExpr()->gtInlineCandidate; if (lookupNamedIntrinsic(call->gtCallMethHnd) == NI_System_RuntimeType_get_TypeHandle) { // Check that the arg is CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE helper call @@ -2990,8 +2990,11 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis, if (arg->IsHelperCall() && gtIsTypeHandleToRuntimeTypeHelper(arg->AsCall())) { impPopStack(); - // Bash the RET_EXPR's call to no-op since it's unused now - op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP(); + if (op1->OperIs(GT_RET_EXPR)) + { + // Bash the RET_EXPR's call to no-op since it's unused now + op1->AsRetExpr()->gtInlineCandidate->gtBashToNOP(); + } // Skip roundtrip and return the type handle directly retNode = arg->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); }