From af57efd7ea19714ed089e68b41ed84b6f27260b4 Mon Sep 17 00:00:00 2001 From: vsadov Date: Fri, 14 Feb 2020 17:30:20 -0800 Subject: [PATCH 1/7] GC polling in unboxing JIT helpers --- .../Runtime/CompilerServices/CastHelpers.cs | 15 ++++ src/coreclr/src/inc/jithelpers.h | 2 +- src/coreclr/src/vm/ecall.cpp | 4 + src/coreclr/src/vm/ecalllist.h | 1 + src/coreclr/src/vm/jithelpers.cpp | 81 +++++-------------- src/coreclr/src/vm/jitinterface.h | 1 + src/coreclr/src/vm/metasig.h | 1 + src/coreclr/src/vm/mscorlib.h | 1 + 8 files changed, 45 insertions(+), 61 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 35df17555f5669..81f77715341744 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -165,6 +165,9 @@ private static CastResult TryGet(nuint source, nuint target) [MethodImpl(MethodImplOptions.InternalCall)] private static extern object ChkCastAny_NoCacheLookup(void* toTypeHnd, object obj); + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern ref byte Unbox_Helper(void* toTypeHnd, object obj); + // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, // this test must deal with all kinds of type tests @@ -476,5 +479,17 @@ private static CastResult TryGet(nuint source, nuint target) slowPath: return ChkCastHelper(toTypeHnd, obj); } + + [DebuggerHidden] + [StackTraceHidden] + [DebuggerStepThrough] + private static ref byte Unbox(void* toTypeHnd, object obj) + { + // this will NRE if obj is null, as expected. + if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) + return ref obj.GetRawData(); + + return ref Unbox_Helper(toTypeHnd, obj); + } } } diff --git a/src/coreclr/src/inc/jithelpers.h b/src/coreclr/src/inc/jithelpers.h index 73b5d2b54cf070..feeb880ac8a048 100644 --- a/src/coreclr/src/inc/jithelpers.h +++ b/src/coreclr/src/inc/jithelpers.h @@ -104,7 +104,7 @@ DYNAMICJITHELPER(CORINFO_HELP_BOX, JIT_Box, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_BOX_NULLABLE, JIT_Box, CORINFO_HELP_SIG_REG_ONLY) - JITHELPER(CORINFO_HELP_UNBOX, JIT_Unbox, CORINFO_HELP_SIG_REG_ONLY) + DYNAMICJITHELPER(CORINFO_HELP_UNBOX, NULL, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_UNBOX_NULLABLE, JIT_Unbox_Nullable, CORINFO_HELP_SIG_4_STACK) JITHELPER(CORINFO_HELP_GETREFANY, JIT_GetRefAny, CORINFO_HELP_SIG_8_STACK) diff --git a/src/coreclr/src/vm/ecall.cpp b/src/coreclr/src/vm/ecall.cpp index 4d0ad323f23070..692b058e4d378d 100644 --- a/src/coreclr/src/vm/ecall.cpp +++ b/src/coreclr/src/vm/ecall.cpp @@ -191,6 +191,10 @@ void ECall::PopulateManagedCastHelpers() pDest = pMD->GetMultiCallableAddrOfCode(); SetJitHelperFunction(CORINFO_HELP_CHKCASTCLASS_SPECIAL, pDest); + pMD = MscorlibBinder::GetMethod((BinderMethodID)(METHOD__CASTHELPERS__UNBOX)); + pDest = pMD->GetMultiCallableAddrOfCode(); + SetJitHelperFunction(CORINFO_HELP_UNBOX, pDest); + #endif //CROSSGEN_COMPILE } diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 8190151176651a..1e7163db4d1176 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -717,6 +717,7 @@ FCFuncEnd() FCFuncStart(gCastHelpers) FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup) FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup) + FCFuncElement("Unbox_Helper", ::Unbox_Helper) FCFuncEnd() FCFuncStart(gArrayFuncs) diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index 07be69a245fd66..d736b2e441b01e 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -2972,6 +2972,7 @@ NOINLINE HCIMPL3(VOID, JIT_Unbox_Nullable_Framed, void * destPtr, MethodTable* t { COMPlusThrowInvalidCastException(&objRef, TypeHandle(typeMT)); } + HELPER_METHOD_POLL(); HELPER_METHOD_FRAME_END(); } HCIMPLEND @@ -2991,6 +2992,7 @@ HCIMPL3(VOID, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Obj if (Nullable::UnBoxNoGC(destPtr, objRef, typeMT)) { // exact match (type equivalence not needed) + FC_GC_POLL(); return; } @@ -3001,16 +3003,30 @@ HCIMPL3(VOID, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Obj HCIMPLEND /*************************************************************/ -/* framed helper that handles full-blown type equivalence */ -NOINLINE HCIMPL2(LPVOID, JIT_Unbox_Helper_Framed, CORINFO_CLASS_HANDLE type, Object* obj) +/* framed Unbox helper that handles enums and full-blown type equivalence */ +NOINLINE HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) { FCALL_CONTRACT; LPVOID result = NULL; + TypeHandle typeHnd(type); + // boxable types have method tables + _ASSERTE(!typeHnd.IsTypeDesc()); + MethodTable* pMT1 = typeHnd.AsMethodTable(); + MethodTable* pMT2 = obj->GetMethodTable(); + OBJECTREF objRef = ObjectToOBJECTREF(obj); HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - if (TypeHandle(type).IsEquivalentTo(objRef->GetTypeHandle())) + + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive())) + { + // we allow enums and their primtive type to be interchangable + result = objRef->GetData(); + } + else if (pMT1->IsEquivalentTo(pMT2)) { // the structures are equivalent result = objRef->GetData(); @@ -3019,69 +3035,14 @@ NOINLINE HCIMPL2(LPVOID, JIT_Unbox_Helper_Framed, CORINFO_CLASS_HANDLE type, Obj { COMPlusThrowInvalidCastException(&objRef, TypeHandle(type)); } + + HELPER_METHOD_POLL(); HELPER_METHOD_FRAME_END(); return result; } HCIMPLEND -/*************************************************************/ -/* the uncommon case for the helper below (allowing enums to be unboxed - as their underlying type */ -LPVOID __fastcall JIT_Unbox_Helper(CORINFO_CLASS_HANDLE type, Object* obj) -{ - FCALL_CONTRACT; - - TypeHandle typeHnd(type); - - CorElementType type1 = typeHnd.GetInternalCorElementType(); - - // we allow enums and their primtive type to be interchangable - - MethodTable* pMT2 = obj->GetMethodTable(); - CorElementType type2 = pMT2->GetInternalCorElementType(); - if (type1 == type2) - { - MethodTable* pMT1 = typeHnd.AsMethodTable(); - if (pMT1 && (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) - { - _ASSERTE(CorTypeInfo::IsPrimitiveType_NoThrow(type1)); - return(obj->GetData()); - } - } - - // Even less common cases (type equivalence) go to a framed helper. - ENDFORBIDGC(); - return HCCALL2(JIT_Unbox_Helper_Framed, type, obj); -} - -/*************************************************************/ -HCIMPL2(LPVOID, JIT_Unbox, CORINFO_CLASS_HANDLE type, Object* obj) -{ - FCALL_CONTRACT; - - TypeHandle typeHnd(type); - VALIDATEOBJECT(obj); - _ASSERTE(!typeHnd.IsTypeDesc()); // boxable types have method tables - - // This has been tuned so that branch predictions are good - // (fall through for forward branches) for the common case - if (obj != NULL) { - if (obj->GetMethodTable() == typeHnd.AsMethodTable()) - return(obj->GetData()); - else { - // Stuff the uncommon case into a helper so that - // its register needs don't cause spills that effect - // the common case above. - return JIT_Unbox_Helper(type, obj); - } - } - - FCThrow(kNullReferenceException); -} -HCIMPLEND - /*************************************************************/ HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByRef) { diff --git a/src/coreclr/src/vm/jitinterface.h b/src/coreclr/src/vm/jitinterface.h index 0536cca7679343..dd5d4fc734d7e7 100644 --- a/src/coreclr/src/vm/jitinterface.h +++ b/src/coreclr/src/vm/jitinterface.h @@ -246,6 +246,7 @@ extern "C" FCDECL2(VOID, JIT_WriteBarrierEnsureNonHeapTarget, Object **dst, Obje extern "C" FCDECL2(Object*, ChkCastAny_NoCacheLookup, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL2(Object*, IsInstanceOfAny_NoCacheLookup, CORINFO_CLASS_HANDLE type, Object* obj); +extern "C" FCDECL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj); extern "C" FCDECL1(void, JIT_InternalThrow, unsigned exceptNum); extern "C" FCDECL1(void*, JIT_InternalThrowFromHelper, unsigned exceptNum); diff --git a/src/coreclr/src/vm/metasig.h b/src/coreclr/src/vm/metasig.h index e4b2be2dd5f20d..f6309e352407c8 100644 --- a/src/coreclr/src/vm/metasig.h +++ b/src/coreclr/src/vm/metasig.h @@ -244,6 +244,7 @@ DEFINE_METASIG(SM(IntPtr_IntPtr_Int_Int_IntPtr_RetVoid, I I i i I, v)) DEFINE_METASIG(SM(IntPtr_RefObj_IntPtr_Obj_RetVoid, I r(j) I j, v)) DEFINE_METASIG(SM(Obj_Int_RetVoid, j i,v)) DEFINE_METASIG(SM(PtrVoid_Obj_RetObj, P(v) j, j)) +DEFINE_METASIG(SM(PtrVoid_Obj_RetRefByte, P(v) j, r(b))) DEFINE_METASIG(SM(Flt_RetFlt, f, f)) DEFINE_METASIG(SM(Dbl_RetDbl, d, d)) diff --git a/src/coreclr/src/vm/mscorlib.h b/src/coreclr/src/vm/mscorlib.h index 397f8b86b61a7d..dfcf9bc704a18b 100644 --- a/src/coreclr/src/vm/mscorlib.h +++ b/src/coreclr/src/vm/mscorlib.h @@ -1461,6 +1461,7 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_Ptr DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj) DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj) +DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte) DEFINE_CLASS_U(CompilerServices, LAHashDependentHashTracker, LAHashDependentHashTrackerObject) DEFINE_FIELD_U(_dependentHandle, LAHashDependentHashTrackerObject,_dependentHandle) From 367e70ecd77fe819ca76f10262ab20a44ede67ec Mon Sep 17 00:00:00 2001 From: vsadov Date: Sat, 15 Feb 2020 18:00:50 -0800 Subject: [PATCH 2/7] PR feedback --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 2 +- src/coreclr/src/vm/interpreter.cpp | 4 ++-- src/coreclr/src/vm/jithelpers.cpp | 2 +- src/coreclr/src/vm/methodtable.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 81f77715341744..b0905c93f506cd 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -485,7 +485,7 @@ private static CastResult TryGet(nuint source, nuint target) [DebuggerStepThrough] private static ref byte Unbox(void* toTypeHnd, object obj) { - // this will NRE if obj is null, as expected. + // this will throw NullReferenceException if obj is null, attributed to the user code, as expected. if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) return ref obj.GetRawData(); diff --git a/src/coreclr/src/vm/interpreter.cpp b/src/coreclr/src/vm/interpreter.cpp index 662e57354f97ca..35ad41ce2a699a 100644 --- a/src/coreclr/src/vm/interpreter.cpp +++ b/src/coreclr/src/vm/interpreter.cpp @@ -8534,7 +8534,7 @@ void Interpreter::Unbox() CorElementType type1 = pMT1->GetInternalCorElementType(); CorElementType type2 = pMT2->GetInternalCorElementType(); - // we allow enums and their primtive type to be interchangable + // we allow enums and their primitive type to be interchangable if (type1 == type2) { if ((pMT1->IsEnum() || pMT1->IsTruePrimitive()) && @@ -8698,7 +8698,7 @@ void Interpreter::UnboxAny() CorElementType type1 = pMT1->GetInternalCorElementType(); CorElementType type2 = pMT2->GetInternalCorElementType(); - // we allow enums and their primtive type to be interchangable + // we allow enums and their primitive type to be interchangable if (type1 == type2) { if ((pMT1->IsEnum() || pMT1->IsTruePrimitive()) && diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index d736b2e441b01e..0473d5590b5730 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3023,7 +3023,7 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && (pMT2->IsEnum() || pMT2->IsTruePrimitive())) { - // we allow enums and their primtive type to be interchangable + // we allow enums and their primitive type to be interchangable result = objRef->GetData(); } else if (pMT1->IsEquivalentTo(pMT2)) diff --git a/src/coreclr/src/vm/methodtable.h b/src/coreclr/src/vm/methodtable.h index d5bd84adc2caee..0c701489277800 100644 --- a/src/coreclr/src/vm/methodtable.h +++ b/src/coreclr/src/vm/methodtable.h @@ -2652,7 +2652,7 @@ class MethodTable // GetInternalCorElementType() retrieves the internal representation of the type. It's not always // appropiate to use this. For example, we treat enums as their underlying type or some structs are // optimized to be ints. To get the signature type or the verifier type (same as signature except for - // enums are normalized to the primtive type that underlies them), use the APIs in Typehandle.h + // enums are normalized to the primitive type that underlies them), use the APIs in Typehandle.h // // * code:TypeHandle.GetSignatureCorElementType() // * code:TypeHandle.GetVerifierCorElementType() From 0af207a8a7a4d7e4536c347559e3302ab0bc89d9 Mon Sep 17 00:00:00 2001 From: vsadov Date: Sat, 15 Feb 2020 20:44:43 -0800 Subject: [PATCH 3/7] more PR feedback --- src/coreclr/src/vm/jithelpers.cpp | 41 +++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index 0473d5590b5730..35910831da7808 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3004,24 +3004,17 @@ HCIMPLEND /*************************************************************/ /* framed Unbox helper that handles enums and full-blown type equivalence */ -NOINLINE HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) +NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) { FCALL_CONTRACT; LPVOID result = NULL; - - TypeHandle typeHnd(type); - // boxable types have method tables - _ASSERTE(!typeHnd.IsTypeDesc()); - MethodTable* pMT1 = typeHnd.AsMethodTable(); MethodTable* pMT2 = obj->GetMethodTable(); OBJECTREF objRef = ObjectToOBJECTREF(obj); HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) + if (pMT1->GetVerifierCorElementType() == pMT2->GetVerifierCorElementType()) { // we allow enums and their primitive type to be interchangable result = objRef->GetData(); @@ -3033,7 +3026,7 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) } else { - COMPlusThrowInvalidCastException(&objRef, TypeHandle(type)); + COMPlusThrowInvalidCastException(&objRef, TypeHandle(pMT1)); } HELPER_METHOD_POLL(); @@ -3043,6 +3036,34 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) } HCIMPLEND +/*************************************************************/ +/* Unbox helper that handles enums */ +HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) +{ + FCALL_CONTRACT; + + TypeHandle typeHnd(type); + // boxable types have method tables + _ASSERTE(!typeHnd.IsTypeDesc()); + + MethodTable* pMT1 = typeHnd.AsMethodTable(); + // must be a value type + _ASSERTE(pMT1->IsValueType()); + + MethodTable* pMT2 = obj->GetMethodTable(); + + // we allow enums and their primitive type to be interchangable. + // if suspension is requested, defer to the framed helper. + if (pMT1->GetVerifierCorElementType() == pMT2->GetVerifierCorElementType() && + g_TrapReturningThreads.LoadWithoutBarrier() == 0) + { + return obj->GetData(); + } + + return Unbox_Helper_Framed(pMT1, obj); +} +HCIMPLEND + /*************************************************************/ HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByRef) { From ec839d8b72c0cf9778eb6bffa808246b20996816 Mon Sep 17 00:00:00 2001 From: vsadov Date: Sat, 15 Feb 2020 23:37:28 -0800 Subject: [PATCH 4/7] PR feedback --- src/coreclr/src/vm/interpreter.cpp | 14 ++++---------- src/coreclr/src/vm/jithelpers.cpp | 8 ++++++-- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/coreclr/src/vm/interpreter.cpp b/src/coreclr/src/vm/interpreter.cpp index 35ad41ce2a699a..467eb65a266455 100644 --- a/src/coreclr/src/vm/interpreter.cpp +++ b/src/coreclr/src/vm/interpreter.cpp @@ -8695,17 +8695,11 @@ void Interpreter::UnboxAny() } else { - CorElementType type1 = pMT1->GetInternalCorElementType(); - CorElementType type2 = pMT2->GetInternalCorElementType(); - - // we allow enums and their primitive type to be interchangable - if (type1 == type2) + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive())) { - if ((pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) - { - res = OpStackGet(tos)->UnBox(); - } + res = OpStackGet(tos)->UnBox(); } } diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index 35910831da7808..c5d1146ca72c62 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3014,7 +3014,9 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) OBJECTREF objRef = ObjectToOBJECTREF(obj); HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - if (pMT1->GetVerifierCorElementType() == pMT2->GetVerifierCorElementType()) + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive())) { // we allow enums and their primitive type to be interchangable result = objRef->GetData(); @@ -3054,7 +3056,9 @@ HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) // we allow enums and their primitive type to be interchangable. // if suspension is requested, defer to the framed helper. - if (pMT1->GetVerifierCorElementType() == pMT2->GetVerifierCorElementType() && + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive()) && g_TrapReturningThreads.LoadWithoutBarrier() == 0) { return obj->GetData(); From 55a812c3b7507aabaecfbb43404b99238c663360 Mon Sep 17 00:00:00 2001 From: vsadov Date: Sun, 16 Feb 2020 00:10:57 -0800 Subject: [PATCH 5/7] more PR feedback --- src/coreclr/src/vm/interpreter.cpp | 5 ++--- src/coreclr/src/vm/jithelpers.cpp | 10 ++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/coreclr/src/vm/interpreter.cpp b/src/coreclr/src/vm/interpreter.cpp index 467eb65a266455..c83edef78e9146 100644 --- a/src/coreclr/src/vm/interpreter.cpp +++ b/src/coreclr/src/vm/interpreter.cpp @@ -8695,9 +8695,8 @@ void Interpreter::UnboxAny() } else { - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) + CorElementType corElementType = pMT1->GetInternalCorElementType(); + if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType()) { res = OpStackGet(tos)->UnBox(); } diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index c5d1146ca72c62..8326bb39c93f0d 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3014,9 +3014,8 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) OBJECTREF objRef = ObjectToOBJECTREF(obj); HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) + CorElementType corElementType = pMT1->GetInternalCorElementType(); + if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType()) { // we allow enums and their primitive type to be interchangable result = objRef->GetData(); @@ -3056,9 +3055,8 @@ HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) // we allow enums and their primitive type to be interchangable. // if suspension is requested, defer to the framed helper. - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive()) && + CorElementType corElementType = pMT1->GetInternalCorElementType(); + if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType() && g_TrapReturningThreads.LoadWithoutBarrier() == 0) { return obj->GetData(); From 7c325e060a661f9782d2682fe5254e946b7e8afb Mon Sep 17 00:00:00 2001 From: vsadov Date: Sun, 16 Feb 2020 08:11:10 -0800 Subject: [PATCH 6/7] couple fixes (missed ENDFORBIDGC and moving GC poll earlier) --- src/coreclr/src/vm/jithelpers.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index 8326bb39c93f0d..a34459fd220b0a 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3013,6 +3013,7 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) OBJECTREF objRef = ObjectToOBJECTREF(obj); HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); + HELPER_METHOD_POLL(); CorElementType corElementType = pMT1->GetInternalCorElementType(); if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType()) @@ -3029,8 +3030,6 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) { COMPlusThrowInvalidCastException(&objRef, TypeHandle(pMT1)); } - - HELPER_METHOD_POLL(); HELPER_METHOD_FRAME_END(); return result; @@ -3062,7 +3061,9 @@ HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) return obj->GetData(); } - return Unbox_Helper_Framed(pMT1, obj); + // Fall back to a framed helper that can also handle GC suspension and type equivalence. + ENDFORBIDGC(); + return HCCALL2(Unbox_Helper_Framed, pMT1, obj); } HCIMPLEND From 3f71c1dca3294342d6789215db0100a0f39de1bd Mon Sep 17 00:00:00 2001 From: vsadov Date: Sun, 16 Feb 2020 12:55:21 -0800 Subject: [PATCH 7/7] further tuning the type check for enums --- src/coreclr/src/vm/interpreter.cpp | 5 +++-- src/coreclr/src/vm/jithelpers.cpp | 12 +++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/coreclr/src/vm/interpreter.cpp b/src/coreclr/src/vm/interpreter.cpp index c83edef78e9146..7386ee7ec98d09 100644 --- a/src/coreclr/src/vm/interpreter.cpp +++ b/src/coreclr/src/vm/interpreter.cpp @@ -8695,8 +8695,9 @@ void Interpreter::UnboxAny() } else { - CorElementType corElementType = pMT1->GetInternalCorElementType(); - if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType()) + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive())) { res = OpStackGet(tos)->UnBox(); } diff --git a/src/coreclr/src/vm/jithelpers.cpp b/src/coreclr/src/vm/jithelpers.cpp index a34459fd220b0a..57c314fe0feeb8 100644 --- a/src/coreclr/src/vm/jithelpers.cpp +++ b/src/coreclr/src/vm/jithelpers.cpp @@ -3015,8 +3015,9 @@ NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); HELPER_METHOD_POLL(); - CorElementType corElementType = pMT1->GetInternalCorElementType(); - if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType()) + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive())) { // we allow enums and their primitive type to be interchangable result = objRef->GetData(); @@ -3054,9 +3055,10 @@ HCIMPL2(LPVOID, Unbox_Helper, CORINFO_CLASS_HANDLE type, Object* obj) // we allow enums and their primitive type to be interchangable. // if suspension is requested, defer to the framed helper. - CorElementType corElementType = pMT1->GetInternalCorElementType(); - if (corElementType != ELEMENT_TYPE_VALUETYPE && corElementType == pMT2->GetInternalCorElementType() && - g_TrapReturningThreads.LoadWithoutBarrier() == 0) + if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && + (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && + (pMT2->IsEnum() || pMT2->IsTruePrimitive()) && + g_TrapReturningThreads.LoadWithoutBarrier() == 0) { return obj->GetData(); }