From c70a57c15b60052fde84daf20ff3f86ee239908a Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 22 Oct 2024 17:14:16 -0700 Subject: [PATCH 01/17] Move unboxing helpers to managed code --- .../System.Private.CoreLib.csproj | 1 + .../src/System/Array.CoreCLR.cs | 4 +- .../Runtime/CompilerServices/CastHelpers.cs | 17 +- .../RuntimeHelpers.CoreCLR.cs | 9 +- src/coreclr/inc/jithelpers.h | 4 +- src/coreclr/vm/JitQCallHelpers.h | 1 + src/coreclr/vm/comutilnative.cpp | 19 ++ src/coreclr/vm/comutilnative.h | 2 + src/coreclr/vm/corelib.h | 6 +- src/coreclr/vm/ecalllist.h | 4 +- src/coreclr/vm/jithelpers.cpp | 179 ++---------------- src/coreclr/vm/jitinterface.h | 3 - src/coreclr/vm/methodtable.h | 1 - src/coreclr/vm/methodtable.inl | 29 +-- src/coreclr/vm/object.cpp | 60 ------ src/coreclr/vm/object.h | 2 - src/coreclr/vm/object.inl | 12 -- src/coreclr/vm/qcallentrypoints.cpp | 1 + 18 files changed, 65 insertions(+), 289 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index c279b79281b17..497a641685924 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -195,6 +195,7 @@ + diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 7386b1689866a..e035d6a320634 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -253,7 +253,7 @@ private static unsafe void CopyImplUnBoxEachElement(Array sourceArray, int sourc if (pDestMT->IsNullable) { - RuntimeHelpers.Unbox_Nullable(ref dest, pDestMT, obj); + BoxingHelpers.Unbox_Nullable(ref dest, pDestMT, obj); } else if (obj is null || RuntimeHelpers.GetMethodTable(obj) != pDestMT) { @@ -546,7 +546,7 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) { if (pElementMethodTable->IsNullable) { - RuntimeHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); + BoxingHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); } else { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 28bfcdf7de495..76908b3b4ece1 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -8,20 +8,25 @@ namespace System.Runtime.CompilerServices { [StackTraceHidden] [DebuggerStepThrough] - internal static unsafe class CastHelpers + internal static unsafe partial class CastHelpers { // In coreclr the table is allocated and written to on the native side. internal static int[]? s_table; + [LibraryImport(RuntimeHelpers.QCall)] + internal static partial void ThrowInvalidCastException(void* fromTypeHnd, void* toTypeHnd); + + internal static void ThrowInvalidCastException(object fromTypeHnd, void* toTypeHnd) + { + ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromTypeHnd), toTypeHnd); + } + [MethodImpl(MethodImplOptions.InternalCall)] private static extern object IsInstanceOfAny_NoCacheLookup(void* toTypeHnd, object obj); [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); - [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); @@ -365,13 +370,13 @@ internal static unsafe class CastHelpers } [DebuggerHidden] - private static ref byte Unbox(void* toTypeHnd, object obj) + private static ref byte Unbox(MethodTable* toTypeHnd, object obj) { // This will throw NullReferenceException if obj is null. if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) return ref obj.GetRawData(); - return ref Unbox_Helper(toTypeHnd, obj); + return ref BoxingHelpers.Unbox_Helper(toTypeHnd, obj); } [DebuggerHidden] diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 418db32c50bf7..e8c59925d9ae9 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -443,9 +443,6 @@ internal static unsafe bool ObjectHasComponentSize(object obj) [MethodImpl(MethodImplOptions.InternalCall)] internal static extern unsafe object? Box(MethodTable* methodTable, ref byte data); - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern unsafe void Unbox_Nullable(ref byte destination, MethodTable* toTypeHnd, object? obj); - // Given an object reference, returns its MethodTable*. // // WARNING: The caller has to ensure that MethodTable* does not get unloaded. The most robust way @@ -872,6 +869,12 @@ public TypeHandle GetArrayElementTypeHandle() /// [MethodImpl(MethodImplOptions.InternalCall)] public extern MethodTable* GetMethodTableMatchingParentClass(MethodTable* parent); + + [MethodImpl(MethodImplOptions.InternalCall)] + public extern ref byte GetNullableValueFieldReferenceAndSize(ref byte nullableAddr, out uint size); + + [MethodImpl(MethodImplOptions.InternalCall)] + public extern MethodTable* InstantiationArg0(); } // Subset of src\vm\methodtable.h diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 8b2a551fcfdb8..35d41fb7413f8 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -114,8 +114,8 @@ DYNAMICJITHELPER(CORINFO_HELP_BOX, JIT_Box, METHOD__NIL) JITHELPER(CORINFO_HELP_BOX_NULLABLE, JIT_Box, METHOD__NIL) DYNAMICJITHELPER(CORINFO_HELP_UNBOX, NULL, METHOD__CASTHELPERS__UNBOX) - JITHELPER(CORINFO_HELP_UNBOX_TYPETEST, JIT_Unbox_TypeTest, METHOD__NIL) - JITHELPER(CORINFO_HELP_UNBOX_NULLABLE, JIT_Unbox_Nullable, METHOD__NIL) + DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__BOXINGHELPERS__UNBOX_TYPETEST) + DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__BOXINGHELPERS__UNBOX_NULLABLE) JITHELPER(CORINFO_HELP_GETREFANY, JIT_GetRefAny, METHOD__NIL) DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, METHOD__CASTHELPERS__STELEMREF) diff --git a/src/coreclr/vm/JitQCallHelpers.h b/src/coreclr/vm/JitQCallHelpers.h index fbef97b52a38e..0263b6729150f 100644 --- a/src/coreclr/vm/JitQCallHelpers.h +++ b/src/coreclr/vm/JitQCallHelpers.h @@ -21,5 +21,6 @@ class MethodDesc; extern "C" void * QCALLTYPE ResolveVirtualFunctionPointer(QCall::ObjectHandleOnStack obj, CORINFO_CLASS_HANDLE classHnd, CORINFO_METHOD_HANDLE methodHnd); extern "C" CORINFO_GENERIC_HANDLE QCALLTYPE GenericHandleWorker(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule); extern "C" void QCALLTYPE InitClassHelper(MethodTable* pMT); +extern "C" void QCALLTYPE ThrowInvalidCastException(CORINFO_CLASS_HANDLE pTargetType, CORINFO_CLASS_HANDLE pSourceType); #endif //_JITQCALLHELPERS_H diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 2ef7785294266..d0fbac7ec5858 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1818,6 +1818,25 @@ FCIMPL2(MethodTable*, MethodTableNative::GetMethodTableMatchingParentClass, Meth } FCIMPLEND +FCIMPL3(uint8_t*, MethodTableNative::GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize) +{ + FCALL_CONTRACT; + _ASSERTE(Nullable::IsNullableType(mt)); + _ASSERTE(strcmp(mt->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0); + + *pSize = mt->GetInstantiation()[0].AsMethodTable()->GetNumInstanceFieldBytes(); + return nullableAddr + mt->GetApproxFieldDescListRaw()[1].GetOffset(); +} +FCIMPLEND + +FCIMPL1(MethodTable*, MethodTableNative::InstantiationArg0, MethodTable* mt); +{ + FCALL_CONTRACT; + + return mt->GetInstantiation()[0].AsMethodTable(); +} +FCIMPLEND + extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb) { QCALL_CONTRACT; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index b2acd9a2cb45d..f2d0c54fee5b5 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -259,6 +259,8 @@ class MethodTableNative { static FCDECL1(UINT32, GetNumInstanceFieldBytes, MethodTable* mt); static FCDECL1(CorElementType, GetPrimitiveCorElementType, MethodTable* mt); static FCDECL2(MethodTable*, GetMethodTableMatchingParentClass, MethodTable* mt, MethodTable* parent); + static FCDECL3(uint8_t*, GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize); + static FCDECL1(MethodTable*, InstantiationArg0, MethodTable* mt); }; extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index b4302ef9bb289..5baffe9299475 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1164,11 +1164,15 @@ 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_METHOD(CASTHELPERS, UNBOX, Unbox, NoSig) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) DEFINE_METHOD(CASTHELPERS, ARRAYTYPECHECK, ArrayTypeCheck, SM_Obj_Array_RetVoid) +DEFINE_CLASS(BOXINGHELPERS, CompilerServices, BoxingHelpers) +DEFINE_METHOD(BOXINGHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) +DEFINE_METHOD(BOXINGHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) + DEFINE_CLASS(VIRTUALDISPATCHHELPERS, CompilerServices, VirtualDispatchHelpers) DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER, VirtualFunctionPointer, NoSig) DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER_DYNAMIC, VirtualFunctionPointer_Dynamic, NoSig) diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 1ea05d6f692d8..f85ea048833fd 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -269,8 +269,6 @@ FCFuncEnd() FCFuncStart(gCastHelpers) FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup) FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup) - FCFuncElement("Unbox_Helper", ::Unbox_Helper) - FCFuncElement("JIT_Unbox_TypeTest", ::JIT_Unbox_TypeTest) FCFuncElement("WriteBarrier", ::WriteBarrier_Helper) FCFuncEnd() @@ -356,13 +354,13 @@ FCFuncStart(gRuntimeHelpers) FCFuncElement("AllocTailCallArgBufferWorker", TailCallHelp::AllocTailCallArgBufferWorker) FCFuncElement("GetTailCallInfo", TailCallHelp::GetTailCallInfo) FCFuncElement("Box", JIT_Box) - FCFuncElement("Unbox_Nullable", JIT_Unbox_Nullable) FCFuncEnd() FCFuncStart(gMethodTableFuncs) FCFuncElement("GetNumInstanceFieldBytes", MethodTableNative::GetNumInstanceFieldBytes) FCFuncElement("GetPrimitiveCorElementType", MethodTableNative::GetPrimitiveCorElementType) FCFuncElement("GetMethodTableMatchingParentClass", MethodTableNative::GetMethodTableMatchingParentClass) + FCFuncElement("GetNullableValueFieldReferenceAndSize", MethodTableNative::GetNullableValueFieldReferenceAndSize) FCFuncEnd() FCFuncStart(gStubHelperFuncs) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 182cdaa539002..22709ea72d519 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1657,172 +1657,6 @@ HCIMPL2(Object*, JIT_Box, CORINFO_CLASS_HANDLE type, void* unboxedData) } HCIMPLEND -/*************************************************************/ -NOINLINE HCIMPL3(VOID, JIT_Unbox_Nullable_Framed, void * destPtr, MethodTable* typeMT, OBJECTREF objRef) -{ - FCALL_CONTRACT; - - HELPER_METHOD_FRAME_BEGIN_1(objRef); - if (!Nullable::UnBox(destPtr, objRef, typeMT)) - { - COMPlusThrowInvalidCastException(&objRef, TypeHandle(typeMT)); - } - HELPER_METHOD_POLL(); - HELPER_METHOD_FRAME_END(); -} -HCIMPLEND - -/*************************************************************/ -HCIMPL3(VOID, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj) -{ - FCALL_CONTRACT; - - TypeHandle typeHnd(type); - _ASSERTE(Nullable::IsNullableType(typeHnd)); - - MethodTable* typeMT = typeHnd.AsMethodTable(); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - - if (Nullable::UnBoxNoGC(destPtr, objRef, typeMT)) - { - // exact match (type equivalence not needed) - FC_GC_POLL(); - return; - } - - // Fall back to a framed helper that handles type equivalence. - ENDFORBIDGC(); - HCCALL3(JIT_Unbox_Nullable_Framed, destPtr, typeMT, objRef); -} -HCIMPLEND - -/*************************************************************/ -/* framed Unbox helper that handles enums and full-blown type equivalence */ -NOINLINE HCIMPL2(LPVOID, Unbox_Helper_Framed, MethodTable* pMT1, Object* obj) -{ - FCALL_CONTRACT; - - LPVOID result = NULL; - MethodTable* pMT2 = obj->GetMethodTable(); - - OBJECTREF objRef = ObjectToOBJECTREF(obj); - HELPER_METHOD_FRAME_BEGIN_RET_1(objRef); - HELPER_METHOD_POLL(); - - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) - { - // we allow enums and their primitive type to be interchangeable - result = objRef->GetData(); - } - else if (pMT1->IsEquivalentTo(pMT2)) - { - // the structures are equivalent - result = objRef->GetData(); - } - else - { - COMPlusThrowInvalidCastException(&objRef, TypeHandle(pMT1)); - } - HELPER_METHOD_FRAME_END(); - - return result; -} -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 interchangeable. - // if suspension is requested, defer to the framed helper. - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive()) && - g_TrapReturningThreads == 0) - { - return obj->GetData(); - } - - // Fall back to a framed helper that can also handle GC suspension and type equivalence. - ENDFORBIDGC(); - return HCCALL2(Unbox_Helper_Framed, pMT1, obj); -} -HCIMPLEND - -/* framed Unbox type test helper that handles enums and full-blown type equivalence */ -NOINLINE HCIMPL2(void, JIT_Unbox_TypeTest_Framed, MethodTable* pMT1, MethodTable* pMT2) -{ - FCALL_CONTRACT; - - HELPER_METHOD_FRAME_BEGIN_0(); - HELPER_METHOD_POLL(); - - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive())) - { - // type test test passes - } - else if (pMT1->IsEquivalentTo(pMT2)) - { - // the structures are equivalent - } - else - { - COMPlusThrowInvalidCastException(TypeHandle(pMT2), TypeHandle(pMT1)); - } - HELPER_METHOD_FRAME_END(); -} -HCIMPLEND - -/*************************************************************/ -/* Unbox type test that handles enums */ -HCIMPL2(void, JIT_Unbox_TypeTest, CORINFO_CLASS_HANDLE type, CORINFO_CLASS_HANDLE boxType) -{ - 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()); - - TypeHandle boxTypeHnd(boxType); - MethodTable* pMT2 = boxTypeHnd.AsMethodTable(); - - // we allow enums and their primitive type to be interchangeable. - // if suspension is requested, defer to the framed helper. - if (pMT1->GetInternalCorElementType() == pMT2->GetInternalCorElementType() && - (pMT1->IsEnum() || pMT1->IsTruePrimitive()) && - (pMT2->IsEnum() || pMT2->IsTruePrimitive()) && - g_TrapReturningThreads == 0) - { - return; - } - - // Fall back to a framed helper that can also handle GC suspension and type equivalence. - ENDFORBIDGC(); - HCCALL2(JIT_Unbox_TypeTest_Framed, pMT1, pMT2); -} -HCIMPLEND - /*************************************************************/ HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByRef) { @@ -1850,6 +1684,19 @@ HCIMPL2(BOOL, JIT_IsInstanceOfException, CORINFO_CLASS_HANDLE type, Object* obj) } HCIMPLEND +extern "C" void QCALLTYPE ThrowInvalidCastException(CORINFO_CLASS_HANDLE pTargetType, CORINFO_CLASS_HANDLE pSourceType) +{ + QCALL_CONTRACT; + + BEGIN_QCALL; + + TypeHandle targetType(pTargetType); + TypeHandle sourceType(pSourceType); + + COMPlusThrowInvalidCastException(sourceType, targetType); + + END_QCALL; +} //======================================================================== // diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 2b7fd04302963..84d70e852020b 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -232,9 +232,6 @@ 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" FCDECL2(void, JIT_Unbox_TypeTest, CORINFO_CLASS_HANDLE type, CORINFO_CLASS_HANDLE boxType); -extern "C" FCDECL3(void, JIT_Unbox_Nullable, void * destPtr, CORINFO_CLASS_HANDLE type, Object* obj); // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index dd39d50046461..3e14bffeef8b9 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2676,7 +2676,6 @@ class MethodTable OBJECTREF Box(void* data); OBJECTREF FastBox(void** data); #ifndef DACCESS_COMPILE - BOOL UnBoxInto(void *dest, OBJECTREF src); void UnBoxIntoUnchecked(void *dest, OBJECTREF src); #endif diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl index 7710efe98d048..85e8a5fdda883 100644 --- a/src/coreclr/vm/methodtable.inl +++ b/src/coreclr/vm/methodtable.inl @@ -1241,31 +1241,6 @@ inline OBJECTREF MethodTable::AllocateNoChecks() #ifndef DACCESS_COMPILE -//========================================================================================== -// unbox src into dest, making sure src is of the correct type. - -inline BOOL MethodTable::UnBoxInto(void *dest, OBJECTREF src) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - if (Nullable::IsNullableType(TypeHandle(this))) - return Nullable::UnBoxNoGC(dest, src, this); - else - { - if (src == NULL || src->GetMethodTable() != this) - return FALSE; - - CopyValueClass(dest, src->UnBox(), this); - } - return TRUE; -} - //========================================================================================== // unbox src into dest, No checks are done @@ -1280,9 +1255,7 @@ inline void MethodTable::UnBoxIntoUnchecked(void *dest, OBJECTREF src) CONTRACTL_END; if (Nullable::IsNullableType(TypeHandle(this))) { - BOOL ret; - ret = Nullable::UnBoxNoGC(dest, src, this); - _ASSERTE(ret); + Nullable::UnBoxNoCheck(dest, src, this); } else { diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 050f16e09ad04..6049568e4a674 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1599,19 +1599,6 @@ BOOL Nullable::IsNullableForTypeHelper(MethodTable* nullableMT, MethodTable* par return TypeHandle(paramMT).IsEquivalentTo(nullableMT->GetInstantiation()[0]); } -//=============================================================================== -// Returns true if nullableMT is Nullable for T == paramMT - -BOOL Nullable::IsNullableForTypeHelperNoGC(MethodTable* nullableMT, MethodTable* paramMT) -{ - LIMITED_METHOD_CONTRACT; - if (!nullableMT->IsNullable()) - return FALSE; - - // we require an exact match of the parameter types - return TypeHandle(paramMT) == nullableMT->GetInstantiation()[0]; -} - //=============================================================================== int32_t Nullable::GetValueAddrOffset(MethodTable* nullableMT) { @@ -1736,53 +1723,6 @@ BOOL Nullable::UnBox(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) return fRet; } -//=============================================================================== -// Special Logic to unbox a boxed T as a nullable -// Does not handle type equivalence (may conservatively return FALSE) -BOOL Nullable::UnBoxNoGC(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - Nullable* dest = (Nullable*) destPtr; - - // We should only get here if we are unboxing a T as a Nullable - _ASSERTE(IsNullableType(destMT)); - - // We better have a concrete instantiation, or our field offset asserts are not useful - _ASSERTE(!destMT->ContainsGenericVariables()); - - if (boxedVal == NULL) - { - // Logically we are doing *dest->HasValueAddr(destMT) = false; - // We zero out the whole structure because it may contain GC references - // and these need to be initialized to zero. (could optimize in the non-GC case) - InitValueClass(destPtr, destMT); - } - else - { - if (!IsNullableForTypeNoGC(destMT, boxedVal->GetMethodTable())) - { - // For safety's sake, also allow true nullables to be unboxed normally. - // This should not happen normally, but we want to be robust - if (destMT == boxedVal->GetMethodTable()) - { - CopyValueClass(dest, boxedVal->GetData(), destMT); - return TRUE; - } - return FALSE; - } - - *dest->HasValueAddr(destMT) = true; - CopyValueClass(dest->ValueAddr(destMT), boxedVal->UnBox(), boxedVal->GetMethodTable()); - } - return TRUE; -} - //=============================================================================== // Special Logic to unbox a boxed T as a nullable // Does not do any type checks. diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index fca89bc39b24a..feeb12dcea527 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -2545,7 +2545,6 @@ class Nullable { static void CheckFieldOffsets(TypeHandle nullableType); static BOOL IsNullableType(TypeHandle nullableType); static BOOL IsNullableForType(TypeHandle nullableType, MethodTable* paramMT); - static BOOL IsNullableForTypeNoGC(TypeHandle nullableType, MethodTable* paramMT); static OBJECTREF Box(void* src, MethodTable* nullable); static BOOL UnBox(void* dest, OBJECTREF boxedVal, MethodTable* destMT); @@ -2572,7 +2571,6 @@ class Nullable { private: static BOOL IsNullableForTypeHelper(MethodTable* nullableMT, MethodTable* paramMT); - static BOOL IsNullableForTypeHelperNoGC(MethodTable* nullableMT, MethodTable* paramMT); CLR_BOOL* HasValueAddr(MethodTable* nullableMT); void* ValueAddr(MethodTable* nullableMT); diff --git a/src/coreclr/vm/object.inl b/src/coreclr/vm/object.inl index c78222529c309..0c45e030a9024 100644 --- a/src/coreclr/vm/object.inl +++ b/src/coreclr/vm/object.inl @@ -215,18 +215,6 @@ __forceinline BOOL Nullable::IsNullableForType(TypeHandle type, MethodTable* par return Nullable::IsNullableForTypeHelper(type.AsMethodTable(), paramMT); } -//=============================================================================== -// Returns true if this pMT is Nullable for T == paramMT - -__forceinline BOOL Nullable::IsNullableForTypeNoGC(TypeHandle type, MethodTable* paramMT) -{ - if (type.IsTypeDesc()) - return FALSE; - if (!type.AsMethodTable()->HasInstantiation()) // shortcut, if it is not generic it can't be Nullable - return FALSE; - return Nullable::IsNullableForTypeHelperNoGC(type.AsMethodTable(), paramMT); -} - //=============================================================================== // Returns true if this type is Nullable for some T. diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 356ce7a1792d4..d0138306e4828 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -503,6 +503,7 @@ static const Entry s_QCall[] = DllImportEntry(InitClassHelper) DllImportEntry(ResolveVirtualFunctionPointer) DllImportEntry(GenericHandleWorker) + DllImportEntry(ThrowInvalidCastException) }; const void* QCallResolveDllImport(const char* name) From f1258a4324a82c0dbc0b24994667431739bdcf2b Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 22 Oct 2024 17:16:31 -0700 Subject: [PATCH 02/17] Add boxinghelpers.cs --- .../Runtime/CompilerServices/BoxingHelpers.cs | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs new file mode 100644 index 0000000000000..0ab6b26218f1e --- /dev/null +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs @@ -0,0 +1,124 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Runtime.InteropServices; + +namespace System.Runtime.CompilerServices +{ + [StackTraceHidden] + [DebuggerStepThrough] + internal static unsafe partial class BoxingHelpers + { + [DebuggerHidden] + private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) + { + uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); + if (((uint)Unsafe.AsPointer(ref destBytes) | numInstanceFieldBytes & ((uint)sizeof(void*) - 1)) != 0) + { + // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte + // And we do not need to concern ourselves with references + SpanHelpers.ClearWithoutReferences(ref destBytes, numInstanceFieldBytes); + } + else + { + // Otherwise, use the helper which is safe for that situation + SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); + } + } + + [DebuggerHidden] + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) + { + if (pMTa == pMTb) + { + return true; + } + + if (pMTa->HasTypeEquivalence && pMTb->HasTypeEquivalence) + { + return false; + } + + return RuntimeHelpers.AreTypesEquivalent(pMTa, pMTb); + } + + [DebuggerHidden] + private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) + { + if (!typeMT->IsNullable) + { + return false; + } + + MethodTable *pMTNullableArg = typeMT->InstantiationArg0(); + if (pMTNullableArg == boxedMT) + { + return true; + } + else + { + return AreTypesEquivalent(pMTNullableArg, boxedMT); + } + } + + [DebuggerHidden] + internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) + { + if (obj == null) + { + InitValueClass(ref destPtr, typeMT); + } + else + { + if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) + { + // For safety's sake, also allow true nullables to be unboxed normally. + // This should not happen normally, but we want to be robust + if (typeMT == RuntimeHelpers.GetMethodTable(obj)) + { + Unsafe.CopyBlockUnaligned(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); + return; + } + CastHelpers.ThrowInvalidCastException(obj, typeMT); + } + + // Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object. + *(bool*)destPtr = true; + ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size); + Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size); + } + } + + internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) + { + // must be a value type + Debug.Assert(pMT1->IsValueType); + + MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); + if ((pMT1->IsPrimitive && pMT2->IsPrimitive && + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || + AreTypesEquivalent(pMT1, pMT2)) + { + return ref RuntimeHelpers.GetRawData(obj); + } + + CastHelpers.ThrowInvalidCastException(obj, pMT1); + return ref Unsafe.AsRef(null); + } + + internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) + { + if (pMT1 == pMT2 || + (pMT1->IsPrimitive && pMT2->IsPrimitive && + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || + AreTypesEquivalent(pMT1, pMT2)) + { + return; + } + + CastHelpers.ThrowInvalidCastException(pMT1, pMT2); + } + } +} From 02636f89e7fc9c4560120a3826d241807be92684 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 24 Oct 2024 10:10:31 -0700 Subject: [PATCH 03/17] Fix issues noted in CI/Review --- .../src/System/Runtime/CompilerServices/BoxingHelpers.cs | 2 +- src/coreclr/vm/ecalllist.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs index 0ab6b26218f1e..166ee0424e2d3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs @@ -85,7 +85,7 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec } // Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object. - *(bool*)destPtr = true; + Unsafe.As(ref destPtr) = true; ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size); Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size); } diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index f85ea048833fd..5f4ad9cbcb771 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -361,6 +361,7 @@ FCFuncStart(gMethodTableFuncs) FCFuncElement("GetPrimitiveCorElementType", MethodTableNative::GetPrimitiveCorElementType) FCFuncElement("GetMethodTableMatchingParentClass", MethodTableNative::GetMethodTableMatchingParentClass) FCFuncElement("GetNullableValueFieldReferenceAndSize", MethodTableNative::GetNullableValueFieldReferenceAndSize) + FCFuncElement("InstantiationArg0", MethodTableNative::InstantiationArg0) FCFuncEnd() FCFuncStart(gStubHelperFuncs) From d8e01e3642c564a2caa041bd8cfb3ebd0ce0358c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 24 Oct 2024 13:38:11 -0700 Subject: [PATCH 04/17] Fix more issues found in CI --- .../src/System/Runtime/CompilerServices/BoxingHelpers.cs | 2 +- src/coreclr/vm/jithelpers.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs index 166ee0424e2d3..7af172a3a02b4 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs @@ -36,7 +36,7 @@ private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) return true; } - if (pMTa->HasTypeEquivalence && pMTb->HasTypeEquivalence) + if (!pMTa->HasTypeEquivalence || !pMTb->HasTypeEquivalence) { return false; } diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 22709ea72d519..0dd85f8334f52 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1693,6 +1693,8 @@ extern "C" void QCALLTYPE ThrowInvalidCastException(CORINFO_CLASS_HANDLE pTarget TypeHandle targetType(pTargetType); TypeHandle sourceType(pSourceType); + GCX_COOP(); + COMPlusThrowInvalidCastException(sourceType, targetType); END_QCALL; From b2c6991bf72fd9bd21bae59b9a872705c34044c8 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 25 Oct 2024 10:58:55 -0700 Subject: [PATCH 05/17] Fix issue where UnboxNoCheck writes too many bytes when unboxing a true boxed nullable --- src/coreclr/vm/object.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 6049568e4a674..f502c3f9cb337 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1757,6 +1757,7 @@ void Nullable::UnBoxNoCheck(void* destPtr, OBJECTREF boxedVal, MethodTable* dest // For safety's sake, also allow true nullables to be unboxed normally. // This should not happen normally, but we want to be robust CopyValueClass(dest, boxedVal->GetData(), destMT); + return; } *dest->HasValueAddr(destMT) = true; From 12d17e56c77dfcc13cd18f08c214d340b0addfce Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 28 Oct 2024 09:49:37 -0700 Subject: [PATCH 06/17] Move unboxing helpers to CastHelpers class --- .../System.Private.CoreLib.csproj | 1 - .../src/System/Array.CoreCLR.cs | 4 +- .../Runtime/CompilerServices/BoxingHelpers.cs | 124 ------------------ .../Runtime/CompilerServices/CastHelpers.cs | 117 ++++++++++++++++- src/coreclr/inc/jithelpers.h | 4 +- src/coreclr/vm/corelib.h | 6 +- 6 files changed, 122 insertions(+), 134 deletions(-) delete mode 100644 src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 497a641685924..c279b79281b17 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -195,7 +195,6 @@ - diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs index e035d6a320634..230355f9df99a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -253,7 +253,7 @@ private static unsafe void CopyImplUnBoxEachElement(Array sourceArray, int sourc if (pDestMT->IsNullable) { - BoxingHelpers.Unbox_Nullable(ref dest, pDestMT, obj); + CastHelpers.Unbox_Nullable(ref dest, pDestMT, obj); } else if (obj is null || RuntimeHelpers.GetMethodTable(obj) != pDestMT) { @@ -546,7 +546,7 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) { if (pElementMethodTable->IsNullable) { - BoxingHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); + CastHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); } else { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs deleted file mode 100644 index 7af172a3a02b4..0000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/BoxingHelpers.cs +++ /dev/null @@ -1,124 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Runtime.InteropServices; - -namespace System.Runtime.CompilerServices -{ - [StackTraceHidden] - [DebuggerStepThrough] - internal static unsafe partial class BoxingHelpers - { - [DebuggerHidden] - private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) - { - uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); - if (((uint)Unsafe.AsPointer(ref destBytes) | numInstanceFieldBytes & ((uint)sizeof(void*) - 1)) != 0) - { - // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte - // And we do not need to concern ourselves with references - SpanHelpers.ClearWithoutReferences(ref destBytes, numInstanceFieldBytes); - } - else - { - // Otherwise, use the helper which is safe for that situation - SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); - } - } - - [DebuggerHidden] - [MethodImpl(MethodImplOptions.NoInlining)] - private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) - { - if (pMTa == pMTb) - { - return true; - } - - if (!pMTa->HasTypeEquivalence || !pMTb->HasTypeEquivalence) - { - return false; - } - - return RuntimeHelpers.AreTypesEquivalent(pMTa, pMTb); - } - - [DebuggerHidden] - private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) - { - if (!typeMT->IsNullable) - { - return false; - } - - MethodTable *pMTNullableArg = typeMT->InstantiationArg0(); - if (pMTNullableArg == boxedMT) - { - return true; - } - else - { - return AreTypesEquivalent(pMTNullableArg, boxedMT); - } - } - - [DebuggerHidden] - internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) - { - if (obj == null) - { - InitValueClass(ref destPtr, typeMT); - } - else - { - if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) - { - // For safety's sake, also allow true nullables to be unboxed normally. - // This should not happen normally, but we want to be robust - if (typeMT == RuntimeHelpers.GetMethodTable(obj)) - { - Unsafe.CopyBlockUnaligned(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); - return; - } - CastHelpers.ThrowInvalidCastException(obj, typeMT); - } - - // Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object. - Unsafe.As(ref destPtr) = true; - ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size); - Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size); - } - } - - internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) - { - // must be a value type - Debug.Assert(pMT1->IsValueType); - - MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); - if ((pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || - AreTypesEquivalent(pMT1, pMT2)) - { - return ref RuntimeHelpers.GetRawData(obj); - } - - CastHelpers.ThrowInvalidCastException(obj, pMT1); - return ref Unsafe.AsRef(null); - } - - internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) - { - if (pMT1 == pMT2 || - (pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || - AreTypesEquivalent(pMT1, pMT2)) - { - return; - } - - CastHelpers.ThrowInvalidCastException(pMT1, pMT2); - } - } -} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 76908b3b4ece1..ee176f27a2496 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -376,7 +376,7 @@ private static ref byte Unbox(MethodTable* toTypeHnd, object obj) if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd) return ref obj.GetRawData(); - return ref BoxingHelpers.Unbox_Helper(toTypeHnd, obj); + return ref Unbox_Helper(toTypeHnd, obj); } [DebuggerHidden] @@ -497,5 +497,120 @@ private static unsafe void ArrayTypeCheck_Helper(object obj, void* elementType) ThrowArrayMismatchException(); } } + + // Helpers for Unboxing + [DebuggerHidden] + [MethodImpl(MethodImplOptions.NoInlining)] + private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) + { + uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); + if ((((uint)Unsafe.AsPointer(ref destBytes) | numInstanceFieldBytes) & ((uint)sizeof(void*) - 1)) != 0) + { + // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte + // And we do not need to concern ourselves with references + SpanHelpers.ClearWithoutReferences(ref destBytes, numInstanceFieldBytes); + } + else + { + // Otherwise, use the helper which is safe for that situation + SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); + } + } + + [DebuggerHidden] + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) + { + if (pMTa == pMTb) + { + return true; + } + + if (!pMTa->HasTypeEquivalence || !pMTb->HasTypeEquivalence) + { + return false; + } + + return RuntimeHelpers.AreTypesEquivalent(pMTa, pMTb); + } + + [DebuggerHidden] + private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) + { + if (!typeMT->IsNullable) + { + return false; + } + + MethodTable *pMTNullableArg = typeMT->InstantiationArg0(); + if (pMTNullableArg == boxedMT) + { + return true; + } + else + { + return AreTypesEquivalent(pMTNullableArg, boxedMT); + } + } + + [DebuggerHidden] + internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) + { + if (obj == null) + { + InitValueClass(ref destPtr, typeMT); + } + else + { + if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) + { + // For safety's sake, also allow true nullables to be unboxed normally. + // This should not happen normally, but we want to be robust + if (typeMT == RuntimeHelpers.GetMethodTable(obj)) + { + Unsafe.CopyBlockUnaligned(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); + return; + } + CastHelpers.ThrowInvalidCastException(obj, typeMT); + } + + // Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object. + Unsafe.As(ref destPtr) = true; + ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size); + Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size); + } + } + + [DebuggerHidden] + internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) + { + // must be a value type + Debug.Assert(pMT1->IsValueType); + + MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); + if ((pMT1->IsPrimitive && pMT2->IsPrimitive && + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || + AreTypesEquivalent(pMT1, pMT2)) + { + return ref RuntimeHelpers.GetRawData(obj); + } + + CastHelpers.ThrowInvalidCastException(obj, pMT1); + return ref Unsafe.AsRef(null); + } + + [DebuggerHidden] + internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) + { + if (pMT1 == pMT2 || + (pMT1->IsPrimitive && pMT2->IsPrimitive && + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || + AreTypesEquivalent(pMT1, pMT2)) + { + return; + } + + CastHelpers.ThrowInvalidCastException(pMT1, pMT2); + } } } diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index 35d41fb7413f8..ac7e32d0b9501 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -114,8 +114,8 @@ DYNAMICJITHELPER(CORINFO_HELP_BOX, JIT_Box, METHOD__NIL) JITHELPER(CORINFO_HELP_BOX_NULLABLE, JIT_Box, METHOD__NIL) DYNAMICJITHELPER(CORINFO_HELP_UNBOX, NULL, METHOD__CASTHELPERS__UNBOX) - DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__BOXINGHELPERS__UNBOX_TYPETEST) - DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__BOXINGHELPERS__UNBOX_NULLABLE) + DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__CASTHELPERS__UNBOX_TYPETEST) + DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__CASTHELPERS__UNBOX_NULLABLE) JITHELPER(CORINFO_HELP_GETREFANY, JIT_GetRefAny, METHOD__NIL) DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, METHOD__CASTHELPERS__STELEMREF) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 5baffe9299475..8e14d7485074b 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -1168,10 +1168,8 @@ DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, NoSig) DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid) DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj) DEFINE_METHOD(CASTHELPERS, ARRAYTYPECHECK, ArrayTypeCheck, SM_Obj_Array_RetVoid) - -DEFINE_CLASS(BOXINGHELPERS, CompilerServices, BoxingHelpers) -DEFINE_METHOD(BOXINGHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) -DEFINE_METHOD(BOXINGHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) +DEFINE_METHOD(CASTHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig) +DEFINE_METHOD(CASTHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig) DEFINE_CLASS(VIRTUALDISPATCHHELPERS, CompilerServices, VirtualDispatchHelpers) DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER, VirtualFunctionPointer, NoSig) From 3f281b9d8f0ab28ee82611537332259bd4671d9c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 14:27:38 -0700 Subject: [PATCH 07/17] Before conversion to ref byte everywhere --- .../src/System/Array.CoreCLR.cs | 4 +- .../src/System/Delegate.CoreCLR.cs | 5 +- .../Runtime/CompilerServices/CastHelpers.cs | 70 ++++++++++++++++++- .../RuntimeHelpers.CoreCLR.cs | 17 +++++ src/coreclr/vm/ecalllist.h | 1 + src/coreclr/vm/jithelpers.cpp | 8 +++ src/coreclr/vm/jitinterface.h | 3 + src/coreclr/vm/methodtablebuilder.cpp | 1 + src/coreclr/vm/object.cpp | 15 ++++ src/coreclr/vm/object.h | 2 +- 10 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 230355f9df99a..c420b96f39576 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -253,7 +253,7 @@ private static unsafe void CopyImplUnBoxEachElement(Array sourceArray, int sourc if (pDestMT->IsNullable) { - CastHelpers.Unbox_Nullable(ref dest, pDestMT, obj); + CastHelpers.Unbox_Nullable_Ref(ref dest, pDestMT, obj); } else if (obj is null || RuntimeHelpers.GetMethodTable(obj) != pDestMT) { @@ -546,7 +546,7 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) { if (pElementMethodTable->IsNullable) { - CastHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); + CastHelpers.Unbox_Nullable_Ref(ref offsetDataRef, pElementMethodTable, value); } else { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs index 664f218be6633..957197f0f5b9d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs @@ -458,7 +458,7 @@ internal static unsafe bool InternalEqualTypes(object a, object b) { if (a.GetType() == b.GetType()) return true; - +#if FEATURE_TYPEEQUIVALENCE MethodTable* pMTa = RuntimeHelpers.GetMethodTable(a); MethodTable* pMTb = RuntimeHelpers.GetMethodTable(b); @@ -474,6 +474,9 @@ internal static unsafe bool InternalEqualTypes(object a, object b) GC.KeepAlive(b); return ret; +#else + return false; +#endif // FEATURE_TYPEEQUIVALENCE } // Used by the ctor. Do not call directly. diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index ee176f27a2496..d2da893f04fa5 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -30,6 +30,9 @@ internal static void ThrowInvalidCastException(object fromTypeHnd, void* toTypeH [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); + [MethodImpl(MethodImplOptions.InternalCall)] + private static extern void FillNullable(ref byte destPtr, MethodTable* typeMT, 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 @@ -500,7 +503,7 @@ private static unsafe void ArrayTypeCheck_Helper(object obj, void* elementType) // Helpers for Unboxing [DebuggerHidden] - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) { uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); @@ -517,6 +520,25 @@ private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) } } + [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static unsafe void InitValueClassPtr(byte* destBytes, MethodTable *pMT) + { + uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); + if ((((uint)destBytes | numInstanceFieldBytes) & ((uint)sizeof(void*) - 1)) != 0) + { + // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte + // And we do not need to concern ourselves with references + SpanHelpers.ClearWithoutReferences(ref Unsafe.AsRef(destBytes), numInstanceFieldBytes); + } + else + { + // Otherwise, use the helper which is safe for that situation + SpanHelpers.ClearWithReferences(ref Unsafe.AsRef(destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); + } + } + +#if FEATURE_TYPEEQUIVALENCE [DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) @@ -533,8 +555,10 @@ private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb) return RuntimeHelpers.AreTypesEquivalent(pMTa, pMTb); } +#endif // FEATURE_TYPEEQUIVALENCE [DebuggerHidden] + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) { if (!typeMT->IsNullable) @@ -542,19 +566,24 @@ private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) return false; } - MethodTable *pMTNullableArg = typeMT->InstantiationArg0(); + Debug.Assert(typeMT->InstantiationArg0() == **typeMT->PerInstInfo); + MethodTable *pMTNullableArg = **typeMT->PerInstInfo; if (pMTNullableArg == boxedMT) { return true; } else { +#if FEATURE_TYPEEQUIVALENCE return AreTypesEquivalent(pMTNullableArg, boxedMT); +#else + return false; +#endif // FEATURE_TYPEEQUIVALENCE } } [DebuggerHidden] - internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) + internal static void Unbox_Nullable_Ref(ref byte destPtr, MethodTable* typeMT, object? obj) { if (obj == null) { @@ -581,6 +610,41 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec } } + [DebuggerHidden] + internal static void Unbox_Nullable(byte* destPtr, MethodTable* typeMT, object? obj) + { + if (obj == null) + { + if (!typeMT->ContainsGCPointers) + { + SpanHelpers.ClearWithoutReferences(ref Unsafe.AsRef(destPtr), typeMT->GetNumInstanceFieldBytes()); + } + else + { + // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass + nuint numInstanceFieldBytes = typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr)); + // Otherwise, use the helper which is safe for that situation + SpanHelpers.ClearWithReferences(ref Unsafe.AsRef(destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); + } + } + else + { + if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) + { + // For safety's sake, also allow true nullables to be unboxed normally. + // This should not happen normally, but we want to be robust + if (typeMT == RuntimeHelpers.GetMethodTable(obj)) + { + Buffer.BulkMoveWithWriteBarrier(ref Unsafe.AsRef(destPtr), ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); + return; + } + CastHelpers.ThrowInvalidCastException(obj, typeMT); + } + + FillNullable(ref Unsafe.AsRef(destPtr), typeMT, obj); + } + } + [DebuggerHidden] internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index e8c59925d9ae9..3746bfa7360ff 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -698,6 +698,16 @@ internal unsafe struct MethodTable [FieldOffset(ElementTypeOffset)] public void* ElementType; + /// + /// The PerInstInfo is used to describe the generic arguments and dictionary of this type. + /// It points as a PerInstInfo, which is an array of pointers to generic dictionaries, which then point + /// to the actual type arguments + the contents of the generic dictionary. The size of the PerInstInfo is + /// defined in the negative space of that structure, and the size of the generic dictionary is described + /// in the DictionaryLayout of the associated canonical MethodTable. + /// + [FieldOffset(ElementTypeOffset)] + public MethodTable*** PerInstInfo; + /// /// This interface map used to list out the set of interfaces. Only meaningful if InterfaceCount is non-zero. /// @@ -772,7 +782,9 @@ internal unsafe struct MethodTable public bool NonTrivialInterfaceCast => (Flags & enum_flag_NonTrivialInterfaceCast) != 0; +#if FEATURE_TYPEEQUIVALENCE public bool HasTypeEquivalence => (Flags & enum_flag_HasTypeEquivalence) != 0; +#endif // FEATURE_TYPEEQUIVALENCE public bool HasFinalizer => (Flags & enum_flag_HasFinalizer) != 0; @@ -873,6 +885,11 @@ public TypeHandle GetArrayElementTypeHandle() [MethodImpl(MethodImplOptions.InternalCall)] public extern ref byte GetNullableValueFieldReferenceAndSize(ref byte nullableAddr, out uint size); +// [MethodImpl(MethodImplOptions.InternalCall)] +// public extern byte* GetNullableValueFieldReferenceAndSize(byte* nullableAddr, out uint size); + [MethodImpl(MethodImplOptions.InternalCall)] + public extern ref byte GetNullableValueFieldReferenceAndSize(byte* nullableAddr, out uint size); + [MethodImpl(MethodImplOptions.InternalCall)] public extern MethodTable* InstantiationArg0(); } diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 5f4ad9cbcb771..cc7fc938a5c6b 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -270,6 +270,7 @@ FCFuncStart(gCastHelpers) FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup) FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup) FCFuncElement("WriteBarrier", ::WriteBarrier_Helper) + FCFuncElement("FillNullable", ::FillNullable) FCFuncEnd() FCFuncStart(gArrayFuncs) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 0dd85f8334f52..e7fb491260012 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1056,6 +1056,14 @@ BOOL ObjIsInstanceOf(Object* pObject, TypeHandle toTypeHnd, BOOL throwCastExcept return ObjIsInstanceOfCore(pObject, toTypeHnd, throwCastException); } +HCIMPL3(VOID, FillNullable, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom) +{ + FCALL_CONTRACT; + + Nullable::UnboxWriteValue(destPtr, ObjectToOBJECTREF(objToCopyFrom), typeMT); +} +HCIMPLEND + HCIMPL2(Object*, ChkCastAny_NoCacheLookup, CORINFO_CLASS_HANDLE type, Object* obj) { FCALL_CONTRACT; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 84d70e852020b..f406ce98d4201 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -233,9 +233,12 @@ 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" FCDECL3(VOID, FillNullable, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom); + // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location extern "C" FCDECL2(VOID, JIT_WriteBarrier_Callable, Object **dst, Object *ref); + #define WriteBarrier_Helper JIT_WriteBarrier_Callable #ifdef TARGET_AMD64 diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index f4393ea494220..457065aa19358 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1892,6 +1892,7 @@ MethodTableBuilder::BuildMethodTableThrowing( // TODO: fix it so that we emit them in the correct order in the first place. if (pMT->ContainsGCPointers()) { + _ASSERTE(pMT->GetClass()->GetBaseSizePadding() == 0); CGCDesc* gcDesc = CGCDesc::GetCGCDescFromMT(pMT); qsort(gcDesc->GetLowestSeries(), (int)gcDesc->GetNumSeries(), sizeof(CGCDescSeries), compareCGCDescSeries); } diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index f502c3f9cb337..14486c564d449 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1765,6 +1765,21 @@ void Nullable::UnBoxNoCheck(void* destPtr, OBJECTREF boxedVal, MethodTable* dest } } +void Nullable::UnboxWriteValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_COOPERATIVE; + } + CONTRACTL_END; + Nullable* dest = (Nullable*) destPtr; + + *dest->HasValueAddr(destMT) = true; + CopyValueClass(dest->ValueAddr(destMT), boxedVal->UnBox(), boxedVal->GetMethodTable()); +} + //=============================================================================== // a boxed Nullable should either be null or a boxed T, but sometimes it is // useful to have a 'true' boxed Nullable (that is it has two fields). This diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index feeb12dcea527..7dd20561857f5 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -2551,7 +2551,7 @@ class Nullable { static BOOL UnBoxNoGC(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static void UnBoxNoCheck(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static OBJECTREF BoxedNullableNull(TypeHandle nullableType) { return NULL; } - + static void UnboxWriteValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT); // if 'Obj' is a true boxed nullable, return the form we want (either null or a boxed T) static OBJECTREF NormalizeBox(OBJECTREF obj); From 8dfaedcdabad67028cd2234fe012a43335f874c3 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 14:57:06 -0700 Subject: [PATCH 08/17] It should all work now, and be fast --- .../src/System/Array.CoreCLR.cs | 4 +- .../Runtime/CompilerServices/CastHelpers.cs | 56 +++++++------------ src/coreclr/vm/ecalllist.h | 2 +- src/coreclr/vm/jithelpers.cpp | 4 +- src/coreclr/vm/jitinterface.h | 2 +- src/coreclr/vm/object.cpp | 2 +- src/coreclr/vm/object.h | 2 +- 7 files changed, 28 insertions(+), 44 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs index c420b96f39576..230355f9df99a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -253,7 +253,7 @@ private static unsafe void CopyImplUnBoxEachElement(Array sourceArray, int sourc if (pDestMT->IsNullable) { - CastHelpers.Unbox_Nullable_Ref(ref dest, pDestMT, obj); + CastHelpers.Unbox_Nullable(ref dest, pDestMT, obj); } else if (obj is null || RuntimeHelpers.GetMethodTable(obj) != pDestMT) { @@ -546,7 +546,7 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) { if (pElementMethodTable->IsNullable) { - CastHelpers.Unbox_Nullable_Ref(ref offsetDataRef, pElementMethodTable, value); + CastHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value); } else { diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index d2da893f04fa5..75258e3055455 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -31,7 +31,7 @@ internal static void ThrowInvalidCastException(object fromTypeHnd, void* toTypeH private static extern void WriteBarrier(ref object? dst, object? obj); [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void FillNullable(ref byte destPtr, MethodTable* typeMT, object obj); + private static extern void UnboxNullableValue(ref byte destPtr, MethodTable* typeMT, object obj); // IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) // Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, @@ -566,6 +566,9 @@ private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) return false; } + // Normally getting the first generic argument involves checking the PerInstInfo to get the count of generic dictionaries + // in the hierarchy, and then doing a bit of math to find the right dictionary, but since we know this is nullable + // we can do a simple double deference to do the same thing. Debug.Assert(typeMT->InstantiationArg0() == **typeMT->PerInstInfo); MethodTable *pMTNullableArg = **typeMT->PerInstInfo; if (pMTNullableArg == boxedMT) @@ -583,65 +586,46 @@ private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) } [DebuggerHidden] - internal static void Unbox_Nullable_Ref(ref byte destPtr, MethodTable* typeMT, object? obj) + [MethodImpl(MethodImplOptions.NoInlining)] + private static void Unbox_Nullable_NotIsNullableForType(ref byte destPtr, MethodTable* typeMT, object obj) { - if (obj == null) - { - InitValueClass(ref destPtr, typeMT); - } - else + // For safety's sake, also allow true nullables to be unboxed normally. + // This should not happen normally, but we want to be robust + if (typeMT == RuntimeHelpers.GetMethodTable(obj)) { - if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) - { - // For safety's sake, also allow true nullables to be unboxed normally. - // This should not happen normally, but we want to be robust - if (typeMT == RuntimeHelpers.GetMethodTable(obj)) - { - Unsafe.CopyBlockUnaligned(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); - return; - } - CastHelpers.ThrowInvalidCastException(obj, typeMT); - } - - // Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object. - Unsafe.As(ref destPtr) = true; - ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size); - Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size); + Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); + return; } + CastHelpers.ThrowInvalidCastException(obj, typeMT); } [DebuggerHidden] - internal static void Unbox_Nullable(byte* destPtr, MethodTable* typeMT, object? obj) + internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) { if (obj == null) { if (!typeMT->ContainsGCPointers) { - SpanHelpers.ClearWithoutReferences(ref Unsafe.AsRef(destPtr), typeMT->GetNumInstanceFieldBytes()); + SpanHelpers.ClearWithoutReferences(ref destPtr, typeMT->GetNumInstanceFieldBytes()); } else { // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass nuint numInstanceFieldBytes = typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr)); // Otherwise, use the helper which is safe for that situation - SpanHelpers.ClearWithReferences(ref Unsafe.AsRef(destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); + SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); } } else { if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) { - // For safety's sake, also allow true nullables to be unboxed normally. - // This should not happen normally, but we want to be robust - if (typeMT == RuntimeHelpers.GetMethodTable(obj)) - { - Buffer.BulkMoveWithWriteBarrier(ref Unsafe.AsRef(destPtr), ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); - return; - } - CastHelpers.ThrowInvalidCastException(obj, typeMT); + Unbox_Nullable_NotIsNullableForType(ref destPtr, typeMT, obj); + } + else + { + UnboxNullableValue(ref destPtr, typeMT, obj); } - - FillNullable(ref Unsafe.AsRef(destPtr), typeMT, obj); } } diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index cc7fc938a5c6b..2e6f3fe5c7628 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -270,7 +270,7 @@ FCFuncStart(gCastHelpers) FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup) FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup) FCFuncElement("WriteBarrier", ::WriteBarrier_Helper) - FCFuncElement("FillNullable", ::FillNullable) + FCFuncElement("UnboxNullableValue", ::UnboxNullableValue) FCFuncEnd() FCFuncStart(gArrayFuncs) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index e7fb491260012..bcef6aecb4b9d 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -1056,11 +1056,11 @@ BOOL ObjIsInstanceOf(Object* pObject, TypeHandle toTypeHnd, BOOL throwCastExcept return ObjIsInstanceOfCore(pObject, toTypeHnd, throwCastException); } -HCIMPL3(VOID, FillNullable, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom) +HCIMPL3(VOID, UnboxNullableValue, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom) { FCALL_CONTRACT; - Nullable::UnboxWriteValue(destPtr, ObjectToOBJECTREF(objToCopyFrom), typeMT); + Nullable::UnboxNullableValue(destPtr, ObjectToOBJECTREF(objToCopyFrom), typeMT); } HCIMPLEND diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index f406ce98d4201..68a18e1274c4a 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -233,7 +233,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" FCDECL3(VOID, FillNullable, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom); +extern "C" FCDECL3(VOID, UnboxNullableValue, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom); // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 14486c564d449..4d61b22ae280a 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1765,7 +1765,7 @@ void Nullable::UnBoxNoCheck(void* destPtr, OBJECTREF boxedVal, MethodTable* dest } } -void Nullable::UnboxWriteValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) +void Nullable::UnboxNullableValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) { CONTRACTL { diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index 7dd20561857f5..a8f3d0374ca37 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -2551,7 +2551,7 @@ class Nullable { static BOOL UnBoxNoGC(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static void UnBoxNoCheck(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static OBJECTREF BoxedNullableNull(TypeHandle nullableType) { return NULL; } - static void UnboxWriteValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT); + static void UnboxNullableValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT); // if 'Obj' is a true boxed nullable, return the form we want (either null or a boxed T) static OBJECTREF NormalizeBox(OBJECTREF obj); From be624da828a91c565ad0b277afd9fa990834d37d Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 16:17:19 -0700 Subject: [PATCH 09/17] Cleanup dead code --- .../CompilerServices/RuntimeHelpers.CoreCLR.cs | 8 -------- src/coreclr/vm/comutilnative.cpp | 11 ----------- src/coreclr/vm/comutilnative.h | 1 - src/coreclr/vm/ecalllist.h | 1 - src/coreclr/vm/methodtablebuilder.cpp | 2 +- 5 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 3746bfa7360ff..598db5422347e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -882,14 +882,6 @@ public TypeHandle GetArrayElementTypeHandle() [MethodImpl(MethodImplOptions.InternalCall)] public extern MethodTable* GetMethodTableMatchingParentClass(MethodTable* parent); - [MethodImpl(MethodImplOptions.InternalCall)] - public extern ref byte GetNullableValueFieldReferenceAndSize(ref byte nullableAddr, out uint size); - -// [MethodImpl(MethodImplOptions.InternalCall)] -// public extern byte* GetNullableValueFieldReferenceAndSize(byte* nullableAddr, out uint size); - [MethodImpl(MethodImplOptions.InternalCall)] - public extern ref byte GetNullableValueFieldReferenceAndSize(byte* nullableAddr, out uint size); - [MethodImpl(MethodImplOptions.InternalCall)] public extern MethodTable* InstantiationArg0(); } diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index d0fbac7ec5858..123bf906aef7a 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1818,17 +1818,6 @@ FCIMPL2(MethodTable*, MethodTableNative::GetMethodTableMatchingParentClass, Meth } FCIMPLEND -FCIMPL3(uint8_t*, MethodTableNative::GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize) -{ - FCALL_CONTRACT; - _ASSERTE(Nullable::IsNullableType(mt)); - _ASSERTE(strcmp(mt->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0); - - *pSize = mt->GetInstantiation()[0].AsMethodTable()->GetNumInstanceFieldBytes(); - return nullableAddr + mt->GetApproxFieldDescListRaw()[1].GetOffset(); -} -FCIMPLEND - FCIMPL1(MethodTable*, MethodTableNative::InstantiationArg0, MethodTable* mt); { FCALL_CONTRACT; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index f2d0c54fee5b5..e01e0aa782d34 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -259,7 +259,6 @@ class MethodTableNative { static FCDECL1(UINT32, GetNumInstanceFieldBytes, MethodTable* mt); static FCDECL1(CorElementType, GetPrimitiveCorElementType, MethodTable* mt); static FCDECL2(MethodTable*, GetMethodTableMatchingParentClass, MethodTable* mt, MethodTable* parent); - static FCDECL3(uint8_t*, GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize); static FCDECL1(MethodTable*, InstantiationArg0, MethodTable* mt); }; diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 2e6f3fe5c7628..169496ae80d6a 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -361,7 +361,6 @@ FCFuncStart(gMethodTableFuncs) FCFuncElement("GetNumInstanceFieldBytes", MethodTableNative::GetNumInstanceFieldBytes) FCFuncElement("GetPrimitiveCorElementType", MethodTableNative::GetPrimitiveCorElementType) FCFuncElement("GetMethodTableMatchingParentClass", MethodTableNative::GetMethodTableMatchingParentClass) - FCFuncElement("GetNullableValueFieldReferenceAndSize", MethodTableNative::GetNullableValueFieldReferenceAndSize) FCFuncElement("InstantiationArg0", MethodTableNative::InstantiationArg0) FCFuncEnd() diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 457065aa19358..ac962d2f40fe0 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1892,7 +1892,7 @@ MethodTableBuilder::BuildMethodTableThrowing( // TODO: fix it so that we emit them in the correct order in the first place. if (pMT->ContainsGCPointers()) { - _ASSERTE(pMT->GetClass()->GetBaseSizePadding() == 0); + _ASSERTE(pMT->GetClass()->GetBaseSizePadding() == 0); // This is dependended on by the System.Runtime.CompilerServices.CastHelpers.IsNullableForType code CGCDesc* gcDesc = CGCDesc::GetCGCDescFromMT(pMT); qsort(gcDesc->GetLowestSeries(), (int)gcDesc->GetNumSeries(), sizeof(CGCDescSeries), compareCGCDescSeries); } From dec085fff7da2161d5c2188e4f9e04b019ef56df Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 16:18:32 -0700 Subject: [PATCH 10/17] Update src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs Co-authored-by: Jan Kotas --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 75258e3055455..7d51cef273373 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -16,9 +16,9 @@ internal static unsafe partial class CastHelpers [LibraryImport(RuntimeHelpers.QCall)] internal static partial void ThrowInvalidCastException(void* fromTypeHnd, void* toTypeHnd); - internal static void ThrowInvalidCastException(object fromTypeHnd, void* toTypeHnd) + internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) { - ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromTypeHnd), toTypeHnd); + ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromType), toTypeHnd); } [MethodImpl(MethodImplOptions.InternalCall)] From e44fab41596e5ab66a4f70b1393134a1bc5fc2c0 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 16:42:40 -0700 Subject: [PATCH 11/17] Fix build break --- .../System/Runtime/CompilerServices/CastHelpers.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 75258e3055455..d2d6fc6970118 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -637,8 +637,11 @@ internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); if ((pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || - AreTypesEquivalent(pMT1, pMT2)) + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) +#if FEATURE_TYPEEQUIVALENCE + || AreTypesEquivalent(pMT1, pMT2) +#endif // FEATURE_TYPEEQUIVALENCE + ) { return ref RuntimeHelpers.GetRawData(obj); } @@ -652,8 +655,11 @@ internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) { if (pMT1 == pMT2 || (pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) || - AreTypesEquivalent(pMT1, pMT2)) + pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) +#if FEATURE_TYPEEQUIVALENCE + || AreTypesEquivalent(pMT1, pMT2) +#endif // FEATURE_TYPEEQUIVALENCE + ) { return; } From 296410517d880e45badd1ae90ca578132b42bcc1 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 30 Oct 2024 16:58:21 -0700 Subject: [PATCH 12/17] And finish up the details around optimizing for the fastest case in Unbox type testing --- .../Runtime/CompilerServices/CastHelpers.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index d2d6fc6970118..d881d4edab350 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -630,6 +630,7 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec } [DebuggerHidden] + [MethodImpl(MethodImplOptions.NoInlining)] internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) { // must be a value type @@ -651,10 +652,10 @@ internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) } [DebuggerHidden] - internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) + [MethodImpl(MethodImplOptions.NoInlining)] + internal static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) { - if (pMT1 == pMT2 || - (pMT1->IsPrimitive && pMT2->IsPrimitive && + if ((pMT1->IsPrimitive && pMT2->IsPrimitive && pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) #if FEATURE_TYPEEQUIVALENCE || AreTypesEquivalent(pMT1, pMT2) @@ -666,5 +667,14 @@ internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) CastHelpers.ThrowInvalidCastException(pMT1, pMT2); } + + [DebuggerHidden] + internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) + { + if (pMT1 == pMT2) + return; + else + Unbox_TypeTest_Helper(pMT1, pMT2); + } } } From 1e51dd25f5e16971f310b9f6111c22faa9304c87 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 31 Oct 2024 14:03:54 -0700 Subject: [PATCH 13/17] - Remove unused helper functions InitValueClass and InitValueClassPtr - Fix assert that was always firing - Tweak code per code review to make the code generator aware that ThrowInvalidCastException is going to throw and should always be in a cold path - Improve the implementation of MethodTable.IsPrimitive. I noticed that this could be a single and+compare instead of the pair it was before. This improves the performance of Unbox_Helper slightly by about 5%. --- .../Runtime/CompilerServices/CastHelpers.cs | 67 +++++-------------- .../RuntimeHelpers.CoreCLR.cs | 3 +- src/coreclr/vm/methodtablebuilder.cpp | 8 ++- 3 files changed, 24 insertions(+), 54 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index a3471f19fcc3b..9d6977533d92d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; namespace System.Runtime.CompilerServices @@ -16,9 +17,11 @@ internal static unsafe partial class CastHelpers [LibraryImport(RuntimeHelpers.QCall)] internal static partial void ThrowInvalidCastException(void* fromTypeHnd, void* toTypeHnd); + [DoesNotReturn] internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) { ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromType), toTypeHnd); + throw null!; // Provide hint to the inliner that this method does not return } [MethodImpl(MethodImplOptions.InternalCall)] @@ -502,42 +505,6 @@ private static unsafe void ArrayTypeCheck_Helper(object obj, void* elementType) } // Helpers for Unboxing - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT) - { - uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); - if ((((uint)Unsafe.AsPointer(ref destBytes) | numInstanceFieldBytes) & ((uint)sizeof(void*) - 1)) != 0) - { - // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte - // And we do not need to concern ourselves with references - SpanHelpers.ClearWithoutReferences(ref destBytes, numInstanceFieldBytes); - } - else - { - // Otherwise, use the helper which is safe for that situation - SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); - } - } - - [DebuggerHidden] - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe void InitValueClassPtr(byte* destBytes, MethodTable *pMT) - { - uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); - if ((((uint)destBytes | numInstanceFieldBytes) & ((uint)sizeof(void*) - 1)) != 0) - { - // If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte - // And we do not need to concern ourselves with references - SpanHelpers.ClearWithoutReferences(ref Unsafe.AsRef(destBytes), numInstanceFieldBytes); - } - else - { - // Otherwise, use the helper which is safe for that situation - SpanHelpers.ClearWithReferences(ref Unsafe.AsRef(destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr)); - } - } - #if FEATURE_TYPEEQUIVALENCE [DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] @@ -591,12 +558,11 @@ private static void Unbox_Nullable_NotIsNullableForType(ref byte destPtr, Method { // For safety's sake, also allow true nullables to be unboxed normally. // This should not happen normally, but we want to be robust - if (typeMT == RuntimeHelpers.GetMethodTable(obj)) + if (typeMT != RuntimeHelpers.GetMethodTable(obj)) { - Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); - return; + CastHelpers.ThrowInvalidCastException(obj, typeMT); } - CastHelpers.ThrowInvalidCastException(obj, typeMT); + Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); } [DebuggerHidden] @@ -637,35 +603,32 @@ internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) Debug.Assert(pMT1->IsValueType); MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); - if ((pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) + if ((!pMT1->IsPrimitive || !pMT2->IsPrimitive || + pMT1->GetPrimitiveCorElementType() != pMT2->GetPrimitiveCorElementType()) #if FEATURE_TYPEEQUIVALENCE - || AreTypesEquivalent(pMT1, pMT2) + && !AreTypesEquivalent(pMT1, pMT2) #endif // FEATURE_TYPEEQUIVALENCE ) { - return ref RuntimeHelpers.GetRawData(obj); + CastHelpers.ThrowInvalidCastException(obj, pMT1); } - CastHelpers.ThrowInvalidCastException(obj, pMT1); - return ref Unsafe.AsRef(null); + return ref RuntimeHelpers.GetRawData(obj); } [DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] internal static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) { - if ((pMT1->IsPrimitive && pMT2->IsPrimitive && - pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) + if ((!pMT1->IsPrimitive || !pMT2->IsPrimitive || + pMT1->GetPrimitiveCorElementType() != pMT2->GetPrimitiveCorElementType()) #if FEATURE_TYPEEQUIVALENCE - || AreTypesEquivalent(pMT1, pMT2) + && !AreTypesEquivalent(pMT1, pMT2) #endif // FEATURE_TYPEEQUIVALENCE ) { - return; + CastHelpers.ThrowInvalidCastException(pMT1, pMT2); } - - CastHelpers.ThrowInvalidCastException(pMT1, pMT2); } [DebuggerHidden] diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index c5db662ff6826..2303e4fea8b34 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -737,6 +737,7 @@ internal unsafe struct MethodTable private const uint enum_flag_Category_Mask = 0x000F0000; private const uint enum_flag_Category_ValueType = 0x00040000; private const uint enum_flag_Category_Nullable = 0x00050000; + private const uint enum_flag_Category_IsPrimitiveMask = 0x000E0000; private const uint enum_flag_Category_PrimitiveValueType = 0x00060000; // sub-category of ValueType, Enum or primitive value type private const uint enum_flag_Category_TruePrimitive = 0x00070000; // sub-category of ValueType, Primitive (ELEMENT_TYPE_I, etc.) private const uint enum_flag_Category_Array = 0x00080000; @@ -829,7 +830,7 @@ public int MultiDimensionalArrayRank public bool IsByRefLike => (Flags & (enum_flag_HasComponentSize | enum_flag_IsByRefLike)) == enum_flag_IsByRefLike; // Warning! UNLIKE the similarly named Reflection api, this method also returns "true" for Enums. - public bool IsPrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_PrimitiveValueType or enum_flag_Category_TruePrimitive; + public bool IsPrimitive => (Flags & enum_flag_Category_IsPrimitiveMask) == enum_flag_Category_PrimitiveValueType; public bool IsTruePrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_TruePrimitive; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index ac962d2f40fe0..cbc7b25acb756 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1892,7 +1892,13 @@ MethodTableBuilder::BuildMethodTableThrowing( // TODO: fix it so that we emit them in the correct order in the first place. if (pMT->ContainsGCPointers()) { - _ASSERTE(pMT->GetClass()->GetBaseSizePadding() == 0); // This is dependended on by the System.Runtime.CompilerServices.CastHelpers.IsNullableForType code +#ifdef _DEBUG + if (pMT->IsValueType()) + { + DWORD baseSizePadding = pMT->GetClass()->GetBaseSizePadding(); + _ASSERTE(baseSizePadding == (sizeof(TADDR) * 2)); // This is dependended on by the System.Runtime.CompilerServices.CastHelpers.IsNullableForType code + } +#endif // _DEBUG CGCDesc* gcDesc = CGCDesc::GetCGCDescFromMT(pMT); qsort(gcDesc->GetLowestSeries(), (int)gcDesc->GetNumSeries(), sizeof(CGCDescSeries), compareCGCDescSeries); } From ee6d42d24cfd44661eed12f8b42d7d45b800c5a8 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 13 Nov 2024 13:07:25 -0800 Subject: [PATCH 14/17] Performance seeking behavior has finished. The perf is amazing --- .../Runtime/CompilerServices/CastHelpers.cs | 22 +++--- .../RuntimeHelpers.CoreCLR.cs | 30 ++++++++- src/coreclr/vm/ecalllist.h | 1 - src/coreclr/vm/jithelpers.cpp | 8 --- src/coreclr/vm/jitinterface.h | 2 - src/coreclr/vm/methodtable.h | 67 ++++++++++++++++++- src/coreclr/vm/methodtablebuilder.cpp | 18 +++++ src/coreclr/vm/object.cpp | 21 ++---- src/coreclr/vm/object.h | 1 - 9 files changed, 128 insertions(+), 42 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 9d6977533d92d..ed65016794f72 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -33,9 +33,6 @@ internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) [MethodImpl(MethodImplOptions.InternalCall)] private static extern void WriteBarrier(ref object? dst, object? obj); - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern void UnboxNullableValue(ref byte destPtr, MethodTable* typeMT, 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 @@ -556,13 +553,13 @@ private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) [MethodImpl(MethodImplOptions.NoInlining)] private static void Unbox_Nullable_NotIsNullableForType(ref byte destPtr, MethodTable* typeMT, object obj) { - // For safety's sake, also allow true nullables to be unboxed normally. - // This should not happen normally, but we want to be robust + // Also allow true nullables to be unboxed normally. + // This should not happen normally, but can happen in debugger scenarios. if (typeMT != RuntimeHelpers.GetMethodTable(obj)) { CastHelpers.ThrowInvalidCastException(obj, typeMT); } - Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); + Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNullableNumInstanceFieldBytes()); } [DebuggerHidden] @@ -572,13 +569,11 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec { if (!typeMT->ContainsGCPointers) { - SpanHelpers.ClearWithoutReferences(ref destPtr, typeMT->GetNumInstanceFieldBytes()); + SpanHelpers.ClearWithoutReferences(ref destPtr, typeMT->GetNullableNumInstanceFieldBytes()); } else { // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass - nuint numInstanceFieldBytes = typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr)); - // Otherwise, use the helper which is safe for that situation SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); } } @@ -590,7 +585,14 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec } else { - UnboxNullableValue(ref destPtr, typeMT, obj); + Unsafe.As(ref destPtr) = true; + ref byte dst = ref Unsafe.Add(ref destPtr, typeMT->NullableValueAddrOffset); + uint valueSize = typeMT->NullableValueSize; + ref byte src = ref RuntimeHelpers.GetRawData(obj); + if (typeMT->ContainsGCPointers) + Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, valueSize); + else + SpanHelpers.Memmove(ref dst, ref src, valueSize); } } } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 2303e4fea8b34..ad6e6039a61ee 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -719,6 +719,25 @@ internal unsafe struct MethodTable [FieldOffset(InterfaceMapOffset)] public MethodTable** InterfaceMap; + /// + /// This is used to hold the nullable unbox data for nullable value types. + /// + [FieldOffset(InterfaceMapOffset)] +#if TARGET_64BIT + public uint NullableValueAddrOffset; +#else + public byte NullableValueAddrOffset; +#endif + +#if TARGET_64BIT + [FieldOffset(InterfaceMapOffset + 4)] + public uint NullableValueSize; +#else + [FieldOffset(InterfaceMapOffset)] + private uint NullableValueSizeEncoded; + public uint NullableValueSize => NullableValueSizeEncoded >> 8; +#endif + // WFLAGS_LOW_ENUM private const uint enum_flag_GenericsMask = 0x00000030; private const uint enum_flag_GenericsMask_NonGeneric = 0x00000000; // no instantiation @@ -825,7 +844,8 @@ public int MultiDimensionalArrayRank public bool IsValueType => (Flags & enum_flag_Category_ValueType_Mask) == enum_flag_Category_ValueType; - public bool IsNullable => (Flags & enum_flag_Category_Mask) == enum_flag_Category_Nullable; + + public bool IsNullable { [MethodImpl(MethodImplOptions.AggressiveInlining)] get { return (Flags & enum_flag_Category_Mask) == enum_flag_Category_Nullable; } } public bool IsByRefLike => (Flags & (enum_flag_HasComponentSize | enum_flag_IsByRefLike)) == enum_flag_IsByRefLike; @@ -890,6 +910,14 @@ public TypeHandle GetArrayElementTypeHandle() [MethodImpl(MethodImplOptions.InternalCall)] public extern MethodTable* InstantiationArg0(); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public uint GetNullableNumInstanceFieldBytes() + { + Debug.Assert(IsNullable); + Debug.Assert((NullableValueAddrOffset + NullableValueSize) == GetNumInstanceFieldBytes()); + return NullableValueAddrOffset + NullableValueSize; + } } [StructLayout(LayoutKind.Sequential)] diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 169496ae80d6a..2f2ae0d584af1 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -270,7 +270,6 @@ FCFuncStart(gCastHelpers) FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup) FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup) FCFuncElement("WriteBarrier", ::WriteBarrier_Helper) - FCFuncElement("UnboxNullableValue", ::UnboxNullableValue) FCFuncEnd() FCFuncStart(gArrayFuncs) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 57a1b4608c9f0..88bb4981d6572 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -781,14 +781,6 @@ BOOL ObjIsInstanceOf(Object* pObject, TypeHandle toTypeHnd, BOOL throwCastExcept return ObjIsInstanceOfCore(pObject, toTypeHnd, throwCastException); } -HCIMPL3(VOID, UnboxNullableValue, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom) -{ - FCALL_CONTRACT; - - Nullable::UnboxNullableValue(destPtr, ObjectToOBJECTREF(objToCopyFrom), typeMT); -} -HCIMPLEND - HCIMPL2(Object*, ChkCastAny_NoCacheLookup, CORINFO_CLASS_HANDLE type, Object* obj) { FCALL_CONTRACT; diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index e79aa940e9821..6038fbfbbcc49 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -233,8 +233,6 @@ 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" FCDECL3(VOID, UnboxNullableValue, uint8_t* destPtr, MethodTable* typeMT, Object* objToCopyFrom); - // ARM64 JIT_WriteBarrier uses speciall ABI and thus is not callable directly // Copied write barriers must be called at a different location extern "C" FCDECL2(VOID, JIT_WriteBarrier_Callable, Object **dst, Object *ref); diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 90a7fb9f085e3..eaa1b430cbcf3 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -2765,7 +2765,7 @@ class MethodTable } // Returns true if this type is Nullable for some T. - inline BOOL IsNullable() + inline BOOL IsNullable() const { LIMITED_METHOD_DAC_CONTRACT; return GetFlag(enum_flag_Category_Mask) == enum_flag_Category_Nullable; @@ -2981,6 +2981,37 @@ class MethodTable OBJECTREF GetManagedClassObject(); OBJECTREF GetManagedClassObjectIfExists(); + // ------------------------------------------------------------------ + // Details about Nullable MethodTables + // ------------------------------------------------------------------ + UINT32 GetNullableValueAddrOffset() const + { + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsNullable()); +#ifndef TARGET_64BIT + return *(BYTE*)&m_encodedNullableUnboxData; +#else + return *(UINT32*)&m_encodedNullableUnboxData; +#endif + } + + UINT32 GetNullableValueSize() const + { + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsNullable()); +#ifndef TARGET_64BIT + return (UINT32)(m_encodedNullableUnboxData >> 8); +#else + return (UINT32)(m_encodedNullableUnboxData >> 32); +#endif + } + + UINT32 GetNullableNumInstanceFieldBytes() const + { + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(IsNullable()); + return GetNullableValueAddrOffset() + GetNullableValueSize(); + } // ------------------------------------------------------------------ // Private part of MethodTable @@ -3798,6 +3829,33 @@ public : return (m_dwFlags2 & (DWORD)mask) == (DWORD)flag; } +#ifndef DACCESS_COMPILE + void SetNullableDetails(UINT16 offsetToValueField, UINT32 sizeOfValueField) + { + STANDARD_VM_CONTRACT; + _ASSERTE(IsNullable()); +#ifndef TARGET_64BIT + if (sizeOfValueField > 0xFFFFFF) + { + // We can't encode the size of the value field in the Nullable MethodTable + // because it's too large. This is a limitation of the encoding. It is not expected + // to impact any real customers, as Nullable should only be used on the stack + // where having a 16MB local would always be a significant problem. Especially oh + // a 32-bit machine. + ThrowHR(COR_E_TYPELOAD); + } + if (offsetToValueField > 255) + { + // If we get here something completely unexpected has happened. We don't expect alignment greater than 128 + ThrowHR(COR_E_TYPELOAD); + } + m_encodedNullableUnboxData = ((TADDR)sizeOfValueField << 8) | (TADDR)offsetToValueField; +#else + m_encodedNullableUnboxData = ((TADDR)sizeOfValueField << 32) | (TADDR)offsetToValueField; +#endif + } +#endif // DACCESS_COMPILE + private: // Low WORD is component size for array and string types (HasComponentSize() returns true). // Used for flags otherwise. @@ -3856,7 +3914,12 @@ public : TADDR m_ElementTypeHnd; }; public: - PTR_InterfaceInfo m_pInterfaceMap; + union + { + PTR_InterfaceInfo m_pInterfaceMap; + TADDR m_encodedNullableUnboxData; // Used for Nullable to represent the offset to the value field, and the size of the value field + }; + // VTable slots go here diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index cbc7b25acb756..35ef3b08378af 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -10068,8 +10068,26 @@ void MethodTableBuilder::CheckForSystemTypes() // Pre-compute whether the class is a Nullable so that code:Nullable::IsNullableType is efficient // This is useful to the performance of boxing/unboxing a Nullable if (GetCl() == g_pNullableClass->GetCl()) + { pMT->SetIsNullable(); + // Capure Nullable specific details into the MethodTable for better unboxing performance + FieldDesc* pFDValue = &pMT->GetApproxFieldDescListRaw()[1]; + _ASSERTE(strcmp(pFDValue->GetDebugName(), "value") == 0); + UINT32 offset = pFDValue->GetOffset(); + if (offset > 0xFF) + { + BuildMethodTableThrowException(IDS_CLASSLOAD_FIELDTOOLARGE); + } + + TypeHandle thValueFieldType = pFDValue->GetApproxFieldTypeHandleThrowing(); + if (!thValueFieldType.IsTypeDesc()) // Non-MethodTable cases can only happen when the size doesn't matter, such as for type variables. + { + pMT->SetNullableDetails((UINT8)offset, thValueFieldType.AsMethodTable()->GetNumInstanceFieldBytes()); + _ASSERTE(pMT->GetNullableNumInstanceFieldBytes() == pMT->GetNumInstanceFieldBytes()); + } + } + return; } } diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 4d61b22ae280a..105d0fc43f426 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -1606,7 +1606,8 @@ int32_t Nullable::GetValueAddrOffset(MethodTable* nullableMT) _ASSERTE(IsNullableType(nullableMT)); _ASSERTE(strcmp(nullableMT->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0); - return nullableMT->GetApproxFieldDescListRaw()[1].GetOffset(); + _ASSERTE(nullableMT->GetApproxFieldDescListRaw()[1].GetOffset() == nullableMT->GetNullableValueAddrOffset()); + return nullableMT->GetNullableValueAddrOffset(); } CLR_BOOL* Nullable::HasValueAddr(MethodTable* nullableMT) { @@ -1624,7 +1625,8 @@ void* Nullable::ValueAddr(MethodTable* nullableMT) { LIMITED_METHOD_CONTRACT; _ASSERTE(strcmp(nullableMT->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0); - return (((BYTE*) this) + nullableMT->GetApproxFieldDescListRaw()[1].GetOffset()); + _ASSERTE(nullableMT->GetApproxFieldDescListRaw()[1].GetOffset() == nullableMT->GetNullableValueAddrOffset()); + return (((BYTE*) this) + nullableMT->GetNullableValueAddrOffset()); } //=============================================================================== @@ -1765,21 +1767,6 @@ void Nullable::UnBoxNoCheck(void* destPtr, OBJECTREF boxedVal, MethodTable* dest } } -void Nullable::UnboxNullableValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - Nullable* dest = (Nullable*) destPtr; - - *dest->HasValueAddr(destMT) = true; - CopyValueClass(dest->ValueAddr(destMT), boxedVal->UnBox(), boxedVal->GetMethodTable()); -} - //=============================================================================== // a boxed Nullable should either be null or a boxed T, but sometimes it is // useful to have a 'true' boxed Nullable (that is it has two fields). This diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index a8f3d0374ca37..21a44dbaf875b 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -2551,7 +2551,6 @@ class Nullable { static BOOL UnBoxNoGC(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static void UnBoxNoCheck(void* dest, OBJECTREF boxedVal, MethodTable* destMT); static OBJECTREF BoxedNullableNull(TypeHandle nullableType) { return NULL; } - static void UnboxNullableValue(void* destPtr, OBJECTREF boxedVal, MethodTable* destMT); // if 'Obj' is a true boxed nullable, return the form we want (either null or a boxed T) static OBJECTREF NormalizeBox(OBJECTREF obj); From 7761ad831201712ed0001bc451079aaa3e6dd95c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 13 Nov 2024 13:34:52 -0800 Subject: [PATCH 15/17] Create GetNumInstanceFieldBytesIfContainsGCPointers helper and use it in appropriate places --- .../src/System/Array.CoreCLR.cs | 10 ++++++++-- .../Runtime/CompilerServices/CastHelpers.cs | 2 +- .../CompilerServices/RuntimeHelpers.CoreCLR.cs | 8 ++++++++ src/coreclr/vm/methodtable.h | 16 +++++++++++++--- src/coreclr/vm/object.cpp | 7 ++++--- 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs index 230355f9df99a..01743710316e7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs @@ -518,11 +518,16 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) if (pElementMethodTable->IsValueType) { ref byte offsetDataRef = ref Unsafe.Add(ref arrayDataRef, flattenedIndex * pMethodTable->ComponentSize); - nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytes(); if (pElementMethodTable->ContainsGCPointers) + { + nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytesIfContainsGCPointers(); SpanHelpers.ClearWithReferences(ref Unsafe.As(ref offsetDataRef), elementSize / (nuint)sizeof(IntPtr)); + } else + { + nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytes(); SpanHelpers.ClearWithoutReferences(ref offsetDataRef, elementSize); + } } else { @@ -550,13 +555,14 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex) } else { - nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytes(); if (pElementMethodTable->ContainsGCPointers) { + nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytesIfContainsGCPointers(); Buffer.BulkMoveWithWriteBarrier(ref offsetDataRef, ref value.GetRawData(), elementSize); } else { + nuint elementSize = pElementMethodTable->GetNumInstanceFieldBytes(); SpanHelpers.Memmove(ref offsetDataRef, ref value.GetRawData(), elementSize); } } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index ed65016794f72..b3be77f2093c7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -574,7 +574,7 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec else { // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass - SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); + SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destPtr), typeMT->GetNumInstanceFieldBytesIfContainsGCPointers() / (nuint)sizeof(IntPtr)); } } else diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index ad6e6039a61ee..f3caebeb00652 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -918,6 +918,14 @@ public uint GetNullableNumInstanceFieldBytes() Debug.Assert((NullableValueAddrOffset + NullableValueSize) == GetNumInstanceFieldBytes()); return NullableValueAddrOffset + NullableValueSize; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public uint GetNumInstanceFieldBytesIfContainsGCPointers() + { + Debug.Assert(ContainsGCPointers); + Debug.Assert((this->BaseSize - (nuint)(2 * sizeof(IntPtr)) == GetNumInstanceFieldBytes())); + return this->BaseSize - (nuint)(2 * sizeof(IntPtr)) + } } [StructLayout(LayoutKind.Sequential)] diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index eaa1b430cbcf3..abc1fb94eeaa1 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -1867,11 +1867,21 @@ class MethodTable // the information within MethodTable, and so less code manipulates EEClass // objects directly, because doing so can lead to bugs related to generics. // - // Use m_wBaseSize whenever this is identical to GetNumInstanceFieldBytes. - // We would need to reserve a flag for this. - // inline DWORD GetNumInstanceFieldBytes(); + // Returns the size of the instance fields for a value type, in bytes when + // the type is known to contain GC pointers. This takes advantage of the detail + // that if the type contains GC pointers, the size of the instance fields is aligned + // to pointer sized boundaries. This is only faster if we already have some reason + // to have checked for ContainsGCPointers. + inline DWORD GetNumInstanceFieldBytesIfContainsGCPointers() + { + LIMITED_METHOD_DAC_CONTRACT; + _ASSERTE(ContainsGCPointers()); + _ASSERTE(GetBaseSize() - (DWORD)(2 * sizeof(TADDR)) == GetNumInstanceFieldBytes()); + return GetBaseSize() - (DWORD)(2 * sizeof(TADDR)); + } + int GetFieldAlignmentRequirement(); inline WORD GetNumIntroducedInstanceFields(); diff --git a/src/coreclr/vm/object.cpp b/src/coreclr/vm/object.cpp index 105d0fc43f426..8955474a85cb9 100644 --- a/src/coreclr/vm/object.cpp +++ b/src/coreclr/vm/object.cpp @@ -366,11 +366,12 @@ void STDCALL CopyValueClassUnchecked(void* dest, void* src, MethodTable *pMT) if (pMT->ContainsGCPointers()) { - memmoveGCRefs(dest, src, pMT->GetNumInstanceFieldBytes()); + memmoveGCRefs(dest, src, pMT->GetNumInstanceFieldBytesIfContainsGCPointers()); } else { - switch (pMT->GetNumInstanceFieldBytes()) + DWORD numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes(); + switch (numInstanceFieldBytes) { case 1: *(UINT8*)dest = *(UINT8*)src; @@ -391,7 +392,7 @@ void STDCALL CopyValueClassUnchecked(void* dest, void* src, MethodTable *pMT) break; #endif // !ALIGN_ACCESS default: - memcpyNoGCRefs(dest, src, pMT->GetNumInstanceFieldBytes()); + memcpyNoGCRefs(dest, src, numInstanceFieldBytes); break; } } From 55e0df94ee52b7f2c92b1885717d0ad7e2598eb2 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 13 Nov 2024 15:52:47 -0800 Subject: [PATCH 16/17] Last minute changes --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 6 +++--- .../Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 4 ++-- src/coreclr/vm/generics.cpp | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index b3be77f2093c7..1e16845bdfb5a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -599,7 +599,7 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec [DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] - internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) + private static ref byte Unbox_Helper(MethodTable* pMT1, object obj) { // must be a value type Debug.Assert(pMT1->IsValueType); @@ -620,7 +620,7 @@ internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) [DebuggerHidden] [MethodImpl(MethodImplOptions.NoInlining)] - internal static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) + private static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) { if ((!pMT1->IsPrimitive || !pMT2->IsPrimitive || pMT1->GetPrimitiveCorElementType() != pMT2->GetPrimitiveCorElementType()) @@ -634,7 +634,7 @@ internal static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) } [DebuggerHidden] - internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) + private static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) { if (pMT1 == pMT2) return; diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index f3caebeb00652..662f187538179 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -923,8 +923,8 @@ public uint GetNullableNumInstanceFieldBytes() public uint GetNumInstanceFieldBytesIfContainsGCPointers() { Debug.Assert(ContainsGCPointers); - Debug.Assert((this->BaseSize - (nuint)(2 * sizeof(IntPtr)) == GetNumInstanceFieldBytes())); - return this->BaseSize - (nuint)(2 * sizeof(IntPtr)) + Debug.Assert((BaseSize - (nuint)(2 * sizeof(IntPtr)) == GetNumInstanceFieldBytes())); + return BaseSize - (uint)(2 * sizeof(IntPtr)); } } diff --git a/src/coreclr/vm/generics.cpp b/src/coreclr/vm/generics.cpp index 384b1007cf8bd..ad76316877fa7 100644 --- a/src/coreclr/vm/generics.cpp +++ b/src/coreclr/vm/generics.cpp @@ -401,6 +401,10 @@ ClassLoader::CreateTypeHandleForNonCanonicalGenericInstantiation( // Fill in interface map pointer pMT->SetInterfaceMap(wNumInterfaces, pInterfaceMap); + if (pMT->IsNullable()) + { + pMT->SetNullableDetails((UINT16)pOldMT->GetNullableValueAddrOffset(), (UINT16)pOldMT->GetNullableValueSize()); + } // Copy across extra flags for these interfaces as well. We may need additional memory for this. PVOID pExtraInterfaceInfo = NULL; From 6a8d1aeb90fd88fd10429588edf1d8e029e1eeff Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 19 Nov 2024 09:38:31 -0800 Subject: [PATCH 17/17] Update wording --- .../src/System/Runtime/CompilerServices/CastHelpers.cs | 1 - .../Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 10 ++++++---- src/coreclr/vm/methodtablebuilder.cpp | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs index 1e16845bdfb5a..202a339b68025 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CastHelpers.cs @@ -573,7 +573,6 @@ internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, objec } else { - // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass SpanHelpers.ClearWithReferences(ref Unsafe.As(ref destPtr), typeMT->GetNumInstanceFieldBytesIfContainsGCPointers() / (nuint)sizeof(IntPtr)); } } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 662f187538179..2d61aa3f0c700 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -705,10 +705,10 @@ internal unsafe struct MethodTable /// /// The PerInstInfo is used to describe the generic arguments and dictionary of this type. - /// It points as a PerInstInfo, which is an array of pointers to generic dictionaries, which then point - /// to the actual type arguments + the contents of the generic dictionary. The size of the PerInstInfo is - /// defined in the negative space of that structure, and the size of the generic dictionary is described - /// in the DictionaryLayout of the associated canonical MethodTable. + /// It points at a structure defined as PerInstInfo in C++, which is an array of pointers to generic + /// dictionaries, which then point to the actual type arguments + the contents of the generic dictionary. + /// The size of the PerInstInfo is defined in the negative space of that structure, and the size of the + /// generic dictionary is described in the DictionaryLayout of the associated canonical MethodTable. /// [FieldOffset(ElementTypeOffset)] public MethodTable*** PerInstInfo; @@ -922,6 +922,8 @@ public uint GetNullableNumInstanceFieldBytes() [MethodImpl(MethodImplOptions.AggressiveInlining)] public uint GetNumInstanceFieldBytesIfContainsGCPointers() { + // If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass + Debug.Assert(ContainsGCPointers); Debug.Assert((BaseSize - (nuint)(2 * sizeof(IntPtr)) == GetNumInstanceFieldBytes())); return BaseSize - (uint)(2 * sizeof(IntPtr)); diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 35ef3b08378af..573a3b2ca4f85 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1896,7 +1896,7 @@ MethodTableBuilder::BuildMethodTableThrowing( if (pMT->IsValueType()) { DWORD baseSizePadding = pMT->GetClass()->GetBaseSizePadding(); - _ASSERTE(baseSizePadding == (sizeof(TADDR) * 2)); // This is dependended on by the System.Runtime.CompilerServices.CastHelpers.IsNullableForType code + _ASSERTE(baseSizePadding == (sizeof(TADDR) * 2)); // This is dependended on by GetNumInstanceFieldBytesIfContainsGCPointers. } #endif // _DEBUG CGCDesc* gcDesc = CGCDesc::GetCGCDescFromMT(pMT);