From f8e9dd1f839bada8d35ca5594c5ec7cf58314a58 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 5 Sep 2024 01:03:47 +0200 Subject: [PATCH] JIT: Skip more covariance checks (#107116) Co-authored-by: Jan Kotas --- src/coreclr/inc/corinfo.h | 6 +- src/coreclr/inc/icorjitinfoimpl_generated.h | 2 +- .../jit/ICorJitInfo_wrapper_generated.hpp | 4 +- src/coreclr/jit/compiler.h | 8 +- src/coreclr/jit/ee_il_dll.hpp | 6 +- src/coreclr/jit/gentree.cpp | 124 +++++++++++++++++- src/coreclr/jit/importer.cpp | 117 +---------------- src/coreclr/jit/importercalls.cpp | 7 +- src/coreclr/jit/morph.cpp | 13 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 15 +-- .../JitInterface/CorInfoImpl_generated.cs | 4 +- .../ThunkGenerator/ThunkInput.txt | 2 +- .../aot/jitinterface/jitinterface_generated.h | 6 +- .../superpmi-shared/methodcontext.cpp | 10 +- .../superpmi/superpmi-shared/methodcontext.h | 4 +- .../superpmi-shim-collector/icorjitinfo.cpp | 10 +- .../icorjitinfo_generated.cpp | 4 +- .../icorjitinfo_generated.cpp | 4 +- .../tools/superpmi/superpmi/icorjitinfo.cpp | 8 +- src/coreclr/vm/jitinterface.cpp | 47 +++++-- 20 files changed, 210 insertions(+), 191 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 30c7b0ab95d8f..566d1ecb769a4 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -2750,12 +2750,12 @@ class ICorStaticInfo // the field's value class (if 'structType' == 0, then don't bother // the structure info). // - // 'memberParent' is typically only set when verifying. It should be the - // result of calling getMemberParent. + // 'fieldOwnerHint' is, potentially, a more exact owner of the field. + // it's fine for it to be non-precise, it's just a hint. virtual CorInfoType getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE * structType = NULL, - CORINFO_CLASS_HANDLE memberParent = NULL /* IN */ + CORINFO_CLASS_HANDLE fieldOwnerHint = NULL /* IN */ ) = 0; // return the data member's instance offset diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h index 96068326c3470..bbaf43a6ee0ce 100644 --- a/src/coreclr/inc/icorjitinfoimpl_generated.h +++ b/src/coreclr/inc/icorjitinfoimpl_generated.h @@ -395,7 +395,7 @@ CORINFO_CLASS_HANDLE getFieldClass( CorInfoType getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) override; + CORINFO_CLASS_HANDLE fieldOwnerHint) override; unsigned getFieldOffset( CORINFO_FIELD_HANDLE field) override; diff --git a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp index 0ee637a957cf3..ff3270192dc5f 100644 --- a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp +++ b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp @@ -939,10 +939,10 @@ CORINFO_CLASS_HANDLE WrapICorJitInfo::getFieldClass( CorInfoType WrapICorJitInfo::getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { API_ENTER(getFieldType); - CorInfoType temp = wrapHnd->getFieldType(field, structType, memberParent); + CorInfoType temp = wrapHnd->getFieldType(field, structType, fieldOwnerHint); API_LEAVE(getFieldType); return temp; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 3689cfaa72b28..2febde4d7365d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3695,6 +3695,8 @@ class Compiler return callMethodHandle == info.compMethodHnd; } + bool gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array); + //------------------------------------------------------------------------- GenTree* gtFoldExpr(GenTree* tree); @@ -5153,8 +5155,6 @@ class Compiler bool impIsImplicitTailCallCandidate( OPCODE curOpcode, const BYTE* codeAddrOfNextOpcode, const BYTE* codeEnd, int prefixFlags, bool isRecursive); - bool impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array); - methodPointerInfo* impAllocateMethodPointerInfo(const CORINFO_RESOLVED_TOKEN& token, mdToken tokenConstrained); /* @@ -8294,7 +8294,9 @@ class Compiler bool eeIsIntrinsic(CORINFO_METHOD_HANDLE ftn); bool eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd); - var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd = nullptr); + var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE* pStructHnd = nullptr, + CORINFO_CLASS_HANDLE memberParent = NO_CLASS_HANDLE); template void eeAppendPrint(class StringPrinter* printer, TPrint print); diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 5ee1c7510d275..32289aff62dac 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -78,9 +78,11 @@ bool Compiler::eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd) } FORCEINLINE -var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd) +var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, + CORINFO_CLASS_HANDLE* pStructHnd, + CORINFO_CLASS_HANDLE fieldOwnerHint) { - return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd)); + return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, fieldOwnerHint)); } FORCEINLINE diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d4d29595d2b03..c60bff8c10202 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19168,10 +19168,17 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b // No benefit to calling gtGetFieldClassHandle here, as // the exact field being accessed can vary. CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->GetFieldHandle(); + CORINFO_CLASS_HANDLE fieldOwner = NO_CLASS_HANDLE; CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE; - var_types fieldType = eeGetFieldType(fieldHnd, &fieldClass); - if (fieldType == TYP_REF) + // fieldOwner helps us to get a more exact field class for instance fields + if (!fieldSeq->IsStaticField()) + { + bool objIsExact, objIsNonNull; + fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); + } + + if (eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) { objClass = fieldClass; } @@ -31840,3 +31847,116 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree) return resultNode; } #endif // FEATURE_HW_INTRINSICS + +//------------------------------------------------------------------------ +// gtCanSkipCovariantStoreCheck: see if storing a ref type value to an array +// can skip the array store covariance check. +// +// Arguments: +// value -- tree producing the value to store +// array -- tree representing the array to store to +// +// Returns: +// true if the store does not require a covariance check. +// +bool Compiler::gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array) +{ + // We should only call this when optimizing. + assert(opts.OptimizationEnabled()); + + // Check for store to same array, ie. arrLcl[i] = arrLcl[j] + if (value->OperIs(GT_IND) && value->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && array->OperIs(GT_LCL_VAR)) + { + GenTree* valueArray = value->AsIndir()->Addr()->AsIndexAddr()->Arr(); + if (valueArray->OperIs(GT_LCL_VAR)) + { + unsigned valueArrayLcl = valueArray->AsLclVar()->GetLclNum(); + unsigned arrayLcl = array->AsLclVar()->GetLclNum(); + if ((valueArrayLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed()) + { + JITDUMP("\nstelem of ref from same array: skipping covariant store check\n"); + return true; + } + } + } + + // Check for store of NULL. + if (value->OperIs(GT_CNS_INT)) + { + assert(value->gtType == TYP_REF); + if (value->AsIntCon()->gtIconVal == 0) + { + JITDUMP("\nstelem of null: skipping covariant store check\n"); + return true; + } + // Non-0 const refs can only occur with frozen objects + assert(value->IsIconHandle(GTF_ICON_OBJ_HDL)); + assert(doesMethodHaveFrozenObjects() || + (compIsForInlining() && impInlineInfo->InlinerCompiler->doesMethodHaveFrozenObjects())); + } + + // Try and get a class handle for the array + if (!value->TypeIs(TYP_REF)) + { + return false; + } + + bool arrayIsExact = false; + bool arrayIsNonNull = false; + CORINFO_CLASS_HANDLE arrayHandle = gtGetClassHandle(array, &arrayIsExact, &arrayIsNonNull); + + if (arrayHandle == NO_CLASS_HANDLE) + { + return false; + } + + // There are some methods in corelib where we're storing to an array but the IL + // doesn't reflect this (see SZArrayHelper). Avoid. + DWORD attribs = info.compCompHnd->getClassAttribs(arrayHandle); + if ((attribs & CORINFO_FLG_ARRAY) == 0) + { + return false; + } + + CORINFO_CLASS_HANDLE arrayElementHandle = nullptr; + CorInfoType arrayElemType = info.compCompHnd->getChildType(arrayHandle, &arrayElementHandle); + + // Verify array type handle is really an array of ref type + assert(arrayElemType == CORINFO_TYPE_CLASS); + + // Check for exactly object[] + if (arrayIsExact && (arrayElementHandle == impGetObjectClass())) + { + JITDUMP("\nstelem to (exact) object[]: skipping covariant store check\n"); + return true; + } + + const bool arrayTypeIsSealed = info.compCompHnd->isExactType(arrayElementHandle); + + if ((!arrayIsExact && !arrayTypeIsSealed) || (arrayElementHandle == NO_CLASS_HANDLE)) + { + // Bail out if we don't know array's exact type + return false; + } + + bool valueIsExact = false; + bool valueIsNonNull = false; + CORINFO_CLASS_HANDLE valueHandle = gtGetClassHandle(value, &valueIsExact, &valueIsNonNull); + + // Array's type is sealed and equals to value's type + if (arrayTypeIsSealed && (valueHandle == arrayElementHandle)) + { + JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n"); + return true; + } + + // Array's type is not sealed but we know its exact type + if (arrayIsExact && (valueHandle != NO_CLASS_HANDLE) && + (info.compCompHnd->compareTypesForCast(valueHandle, arrayElementHandle) == TypeCompareState::Must)) + { + JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n"); + return true; + } + + return false; +} diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 528c082629fcf..ea1d4f6d09370 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7069,7 +7069,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(ldelemClsHnd)); // If it's a value class / pointer array, or a readonly access, we don't need a type check. - // TODO-CQ: adapt "impCanSkipCovariantStoreCheck" to handle "ldelema"s and call it here to + // TODO-CQ: adapt "gtCanSkipCovariantStoreCheck" to handle "ldelema"s and call it here to // skip using the helper in more cases. if ((lclTyp != TYP_REF) || ((prefixFlags & PREFIX_READONLY) != 0)) { @@ -7208,7 +7208,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) if (opts.OptimizationEnabled()) { // Is this a case where we can skip the covariant store check? - if (impCanSkipCovariantStoreCheck(value, array)) + if (gtCanSkipCovariantStoreCheck(value, array)) { lclTyp = TYP_REF; goto ARR_ST; @@ -13898,116 +13898,3 @@ methodPointerInfo* Compiler::impAllocateMethodPointerInfo(const CORINFO_RESOLVED memory->m_tokenConstraint = tokenConstrained; return memory; } - -//------------------------------------------------------------------------ -// impCanSkipCovariantStoreCheck: see if storing a ref type value to an array -// can skip the array store covariance check. -// -// Arguments: -// value -- tree producing the value to store -// array -- tree representing the array to store to -// -// Returns: -// true if the store does not require a covariance check. -// -bool Compiler::impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array) -{ - // We should only call this when optimizing. - assert(opts.OptimizationEnabled()); - - // Check for store to same array, ie. arrLcl[i] = arrLcl[j] - if (value->OperIs(GT_IND) && value->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && array->OperIs(GT_LCL_VAR)) - { - GenTree* valueArray = value->AsIndir()->Addr()->AsIndexAddr()->Arr(); - if (valueArray->OperIs(GT_LCL_VAR)) - { - unsigned valueArrayLcl = valueArray->AsLclVar()->GetLclNum(); - unsigned arrayLcl = array->AsLclVar()->GetLclNum(); - if ((valueArrayLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed()) - { - JITDUMP("\nstelem of ref from same array: skipping covariant store check\n"); - return true; - } - } - } - - // Check for store of NULL. - if (value->OperIs(GT_CNS_INT)) - { - assert(value->gtType == TYP_REF); - if (value->AsIntCon()->gtIconVal == 0) - { - JITDUMP("\nstelem of null: skipping covariant store check\n"); - return true; - } - // Non-0 const refs can only occur with frozen objects - assert(value->IsIconHandle(GTF_ICON_OBJ_HDL)); - assert(doesMethodHaveFrozenObjects() || - (compIsForInlining() && impInlineInfo->InlinerCompiler->doesMethodHaveFrozenObjects())); - } - - // Try and get a class handle for the array - if (value->gtType != TYP_REF) - { - return false; - } - - bool arrayIsExact = false; - bool arrayIsNonNull = false; - CORINFO_CLASS_HANDLE arrayHandle = gtGetClassHandle(array, &arrayIsExact, &arrayIsNonNull); - - if (arrayHandle == NO_CLASS_HANDLE) - { - return false; - } - - // There are some methods in corelib where we're storing to an array but the IL - // doesn't reflect this (see SZArrayHelper). Avoid. - DWORD attribs = info.compCompHnd->getClassAttribs(arrayHandle); - if ((attribs & CORINFO_FLG_ARRAY) == 0) - { - return false; - } - - CORINFO_CLASS_HANDLE arrayElementHandle = nullptr; - CorInfoType arrayElemType = info.compCompHnd->getChildType(arrayHandle, &arrayElementHandle); - - // Verify array type handle is really an array of ref type - assert(arrayElemType == CORINFO_TYPE_CLASS); - - // Check for exactly object[] - if (arrayIsExact && (arrayElementHandle == impGetObjectClass())) - { - JITDUMP("\nstelem to (exact) object[]: skipping covariant store check\n"); - return true; - } - - const bool arrayTypeIsSealed = info.compCompHnd->isExactType(arrayElementHandle); - - if ((!arrayIsExact && !arrayTypeIsSealed) || (arrayElementHandle == NO_CLASS_HANDLE)) - { - // Bail out if we don't know array's exact type - return false; - } - - bool valueIsExact = false; - bool valueIsNonNull = false; - CORINFO_CLASS_HANDLE valueHandle = gtGetClassHandle(value, &valueIsExact, &valueIsNonNull); - - // Array's type is sealed and equals to value's type - if (arrayTypeIsSealed && (valueHandle == arrayElementHandle)) - { - JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n"); - return true; - } - - // Array's type is not sealed but we know its exact type - if (arrayIsExact && (valueHandle != NO_CLASS_HANDLE) && - (info.compCompHnd->compareTypesForCast(valueHandle, arrayElementHandle) == TypeCompareState::Must)) - { - JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n"); - return true; - } - - return false; -} diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index e0fc0e0dbacc7..c1f01983993f2 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2908,12 +2908,9 @@ GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig) return nullptr; } - CORINFO_CLASS_HANDLE fieldOwnerHnd = info.compCompHnd->getFieldClass(fieldToken); - CORINFO_CLASS_HANDLE fieldClsHnd; - var_types fieldElementType = - JITtype2varType(info.compCompHnd->getFieldType(fieldToken, &fieldClsHnd, fieldOwnerHnd)); - unsigned totalFieldSize; + var_types fieldElementType = JITtype2varType(info.compCompHnd->getFieldType(fieldToken, &fieldClsHnd)); + unsigned totalFieldSize; // Most static initialization data fields are of some structure, but it is possible for them to be of various // primitive types as well diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 6c788fa243e67..32b382d888bf8 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -7088,18 +7088,15 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) // Morph stelem.ref helper call to store a null value, into a store into an array without the helper. // This needs to be done after the arguments are morphed to ensure constant propagation has already taken place. - if (opts.OptimizationEnabled() && call->IsHelperCall() && - (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST))) + if (opts.OptimizationEnabled() && call->IsHelperCall(this, CORINFO_HELP_ARRADDR_ST)) { assert(call->gtArgs.CountArgs() == 3); + GenTree* arr = call->gtArgs.GetArgByIndex(0)->GetNode(); + GenTree* index = call->gtArgs.GetArgByIndex(1)->GetNode(); GenTree* value = call->gtArgs.GetArgByIndex(2)->GetNode(); - if (value->IsIntegralConst(0)) - { - assert(value->OperGet() == GT_CNS_INT); - - GenTree* arr = call->gtArgs.GetArgByIndex(0)->GetNode(); - GenTree* index = call->gtArgs.GetArgByIndex(1)->GetNode(); + if (gtCanSkipCovariantStoreCheck(value, arr)) + { // Either or both of the array and index arguments may have been spilled to temps by `fgMorphArgs`. Copy // the spill trees as well if necessary. GenTree* argSetup = nullptr; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 06141640ef53c..987bbc6508f13 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2902,18 +2902,13 @@ private bool isMoreSpecificType(CORINFO_CLASS_STRUCT_* cls1, CORINFO_CLASS_STRUC TypeDesc type1 = HandleToObject(cls1); TypeDesc type2 = HandleToObject(cls2); - // If we have a mixture of shared and unshared types, - // consider the unshared type as more specific. - bool isType1CanonSubtype = type1.IsCanonicalSubtype(CanonicalFormKind.Any); - bool isType2CanonSubtype = type2.IsCanonicalSubtype(CanonicalFormKind.Any); - if (isType1CanonSubtype != isType2CanonSubtype) + // If both types have the same type definition while + // hnd1 is shared and hnd2 is not - consider hnd2 more specific. + if (type1.HasSameTypeDefinition(type2)) { - // Only one of type1 and type2 is shared. - // type2 is more specific if type1 is the shared type. - return isType1CanonSubtype; + return type1.IsCanonicalSubtype(CanonicalFormKind.Any) && !type2.IsCanonicalSubtype(CanonicalFormKind.Any); } - // Otherwise both types are either shared or not shared. // Look for a common parent type. TypeDesc merged = TypeExtensions.MergeTypesToCommonParent(type1, type2); @@ -3106,7 +3101,7 @@ private void getThreadLocalStaticBlocksInfo(CORINFO_THREAD_STATIC_BLOCKS_INFO* p return ObjectToHandle(fieldDesc.OwningType); } - private CorInfoType getFieldType(CORINFO_FIELD_STRUCT_* field, CORINFO_CLASS_STRUCT_** structType, CORINFO_CLASS_STRUCT_* memberParent) + private CorInfoType getFieldType(CORINFO_FIELD_STRUCT_* field, CORINFO_CLASS_STRUCT_** structType, CORINFO_CLASS_STRUCT_* fieldOwnerHint) { FieldDesc fieldDesc = HandleToObject(field); TypeDesc fieldType = fieldDesc.FieldType; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs index 76b08b42458b3..9213d42aba327 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs @@ -1421,12 +1421,12 @@ private static UIntPtr _printFieldName(IntPtr thisHandle, IntPtr* ppException, C } [UnmanagedCallersOnly] - private static CorInfoType _getFieldType(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, CORINFO_CLASS_STRUCT_** structType, CORINFO_CLASS_STRUCT_* memberParent) + private static CorInfoType _getFieldType(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, CORINFO_CLASS_STRUCT_** structType, CORINFO_CLASS_STRUCT_* fieldOwnerHint) { var _this = GetThis(thisHandle); try { - return _this.getFieldType(field, structType, memberParent); + return _this.getFieldType(field, structType, fieldOwnerHint); } catch (Exception ex) { diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt index e776aa2796259..76d3ea9d3864a 100644 --- a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt +++ b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt @@ -259,7 +259,7 @@ FUNCTIONS CorInfoIsAccessAllowedResult canAccessClass(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, CORINFO_HELPER_DESC* pAccessHelper) size_t printFieldName(CORINFO_FIELD_HANDLE field, char* buffer, size_t bufferSize, size_t* pRequiredBufferSize) CORINFO_CLASS_HANDLE getFieldClass(CORINFO_FIELD_HANDLE field) - CorInfoType getFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, CORINFO_CLASS_HANDLE memberParent) + CorInfoType getFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, CORINFO_CLASS_HANDLE fieldOwnerHint) unsigned getFieldOffset(CORINFO_FIELD_HANDLE field) void getFieldInfo(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, CORINFO_ACCESS_FLAGS flags, CORINFO_FIELD_INFO* pResult) uint32_t getThreadLocalFieldInfo (CORINFO_FIELD_HANDLE field, bool isGCtype) diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h index c56fe13931863..0ba43d2a45885 100644 --- a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h +++ b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h @@ -106,7 +106,7 @@ struct JitInterfaceCallbacks CorInfoIsAccessAllowedResult (* canAccessClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, CORINFO_HELPER_DESC* pAccessHelper); size_t (* printFieldName)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, char* buffer, size_t bufferSize, size_t* pRequiredBufferSize); CORINFO_CLASS_HANDLE (* getFieldClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field); - CorInfoType (* getFieldType)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, CORINFO_CLASS_HANDLE memberParent); + CorInfoType (* getFieldType)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, CORINFO_CLASS_HANDLE fieldOwnerHint); unsigned (* getFieldOffset)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field); void (* getFieldInfo)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, CORINFO_ACCESS_FLAGS flags, CORINFO_FIELD_INFO* pResult); uint32_t (* getThreadLocalFieldInfo)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, bool isGCtype); @@ -1130,10 +1130,10 @@ class JitInterfaceWrapper : public ICorJitInfo virtual CorInfoType getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { CorInfoExceptionClass* pException = nullptr; - CorInfoType temp = _callbacks->getFieldType(_thisHandle, &pException, field, structType, memberParent); + CorInfoType temp = _callbacks->getFieldType(_thisHandle, &pException, field, structType, fieldOwnerHint); if (pException != nullptr) throw pException; return temp; } diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index d1078fe8d356c..ddfc401aff1f2 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -5029,7 +5029,7 @@ GetTypeLayoutResult MethodContext::repGetTypeLayout(CORINFO_CLASS_HANDLE typeHnd void MethodContext::recGetFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent, + CORINFO_CLASS_HANDLE fieldOwnerHint, CorInfoType result) { if (GetFieldType == nullptr) @@ -5038,7 +5038,7 @@ void MethodContext::recGetFieldType(CORINFO_FIELD_HANDLE field, DLDL key; ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding key.A = CastHandle(field); - key.B = CastHandle(memberParent); + key.B = CastHandle(fieldOwnerHint); DLD value; value.B = (DWORD)result; @@ -5051,7 +5051,7 @@ void MethodContext::recGetFieldType(CORINFO_FIELD_HANDLE field, value.A = CastHandle(*structType); // If we had a previous call with null 'structType', we will not have captured the - // class handle (we use only 'field' and 'memberParent' as keys). + // class handle (we use only 'field' and 'fieldOwnerHint' as keys). // Update the value in that case. unsigned index = GetFieldType->GetIndex(key); if ((index != (unsigned)-1) && (GetFieldType->GetItem(index).A == 0)) @@ -5071,12 +5071,12 @@ void MethodContext::dmpGetFieldType(DLDL key, DLD value) } CorInfoType MethodContext::repGetFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { DLDL key; ZeroMemory(&key, sizeof(key)); // Zero key including any struct padding key.A = CastHandle(field); - key.B = CastHandle(memberParent); + key.B = CastHandle(fieldOwnerHint); DLD value = LookupByKeyOrMiss(GetFieldType, key, ": key %016" PRIX64 "", key.A); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index 4625ad44aab37..ffe0e7aaa5bdc 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -640,12 +640,12 @@ class MethodContext void recGetFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent, + CORINFO_CLASS_HANDLE fieldOwnerHint, CorInfoType result); void dmpGetFieldType(DLDL key, DLD value); CorInfoType repGetFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent); + CORINFO_CLASS_HANDLE fieldOwnerHint); void recSatisfiesMethodConstraints(CORINFO_CLASS_HANDLE parent, CORINFO_METHOD_HANDLE method, bool result); void dmpSatisfiesMethodConstraints(DLDL key, DWORD value); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp index b3fab2e939dae..5cf11bf9420df 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -1062,16 +1062,16 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getFieldClass(CORINFO_FIELD_HANDLE field) // the field's value class (if 'structType' == 0, then don't bother // the structure info). // -// 'memberParent' is typically only set when verifying. It should be the -// result of calling getMemberParent. +// 'fieldOwnerHint' is, potentially, a more exact owner of the field. +// it's fine for it to be non-precise, it's just a hint. CorInfoType interceptor_ICJI::getFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent /* IN */ + CORINFO_CLASS_HANDLE fieldOwnerHint /* IN */ ) { mc->cr->AddCall("getFieldType"); - CorInfoType temp = original_ICorJitInfo->getFieldType(field, structType, memberParent); - mc->recGetFieldType(field, structType, memberParent, temp); + CorInfoType temp = original_ICorJitInfo->getFieldType(field, structType, fieldOwnerHint); + mc->recGetFieldType(field, structType, fieldOwnerHint, temp); return temp; } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp index 638f6649613fa..9d5eb18dfc4d9 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp @@ -766,10 +766,10 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getFieldClass( CorInfoType interceptor_ICJI::getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { mcs->AddCall("getFieldType"); - return original_ICorJitInfo->getFieldType(field, structType, memberParent); + return original_ICorJitInfo->getFieldType(field, structType, fieldOwnerHint); } unsigned interceptor_ICJI::getFieldOffset( diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp index c285ef2c3c6ca..8e3bff4eb7067 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp @@ -671,9 +671,9 @@ CORINFO_CLASS_HANDLE interceptor_ICJI::getFieldClass( CorInfoType interceptor_ICJI::getFieldType( CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { - return original_ICorJitInfo->getFieldType(field, structType, memberParent); + return original_ICorJitInfo->getFieldType(field, structType, fieldOwnerHint); } unsigned interceptor_ICJI::getFieldOffset( diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp index 3b9c00aba9568..c3c4fd276490e 100644 --- a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp @@ -883,15 +883,15 @@ CORINFO_CLASS_HANDLE MyICJI::getFieldClass(CORINFO_FIELD_HANDLE field) // the field's value class (if 'structType' == 0, then don't bother // the structure info). // -// 'memberParent' is typically only set when verifying. It should be the -// result of calling getMemberParent. +// 'fieldOwnerHint' is, potentially, a more exact owner of the field. +// it's fine for it to be non-precise, it's just a hint. CorInfoType MyICJI::getFieldType(CORINFO_FIELD_HANDLE field, CORINFO_CLASS_HANDLE* structType, - CORINFO_CLASS_HANDLE memberParent /* IN */ + CORINFO_CLASS_HANDLE fieldOwnerHint /* IN */ ) { jitInstance->mc->cr->AddCall("getFieldType"); - return jitInstance->mc->repGetFieldType(field, structType, memberParent); + return jitInstance->mc->repGetFieldType(field, structType, fieldOwnerHint); } // return the data member's instance offset diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2c470710f0b4c..b9d260cfe7830 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -4363,18 +4363,14 @@ static BOOL isMoreSpecificTypeHelper(TypeHandle hnd1, TypeHandle hnd2) return FALSE; } - // If we have a mixture of shared and unshared types, - // consider the unshared type as more specific. - BOOL isHnd1CanonSubtype = hnd1.IsCanonicalSubtype(); - BOOL isHnd2CanonSubtype = hnd2.IsCanonicalSubtype(); - if (isHnd1CanonSubtype != isHnd2CanonSubtype) + // If both types have the same type definition while + // hnd1 is shared and hnd2 is not - consider hnd2 more specific. + if (!hnd1.IsTypeDesc() && !hnd2.IsTypeDesc() && + hnd1.AsMethodTable()->HasSameTypeDefAs(hnd2.AsMethodTable())) { - // Only one of hnd1 and hnd2 is shared. - // hdn2 is more specific if hnd1 is the shared type. - return isHnd1CanonSubtype; + return hnd1.IsCanonicalSubtype() && !hnd2.IsCanonicalSubtype(); } - // Otherwise both types are either shared or not shared. // Look for a common parent type. TypeHandle merged = TypeHandle::MergeTypeHandlesToCommonParent(hnd1, hnd2); @@ -9138,11 +9134,11 @@ CORINFO_CLASS_HANDLE CEEInfo::getFieldClass (CORINFO_FIELD_HANDLE fieldHnd) // // pTypeHnd - Optional. If not null then on return, for reference and value types, // *pTypeHnd will contain the normalized type of the field. -// owner - Optional. For resolving in a generic context +// fieldOwnerHint - Optional. For resolving in a generic context CorInfoType CEEInfo::getFieldType (CORINFO_FIELD_HANDLE fieldHnd, CORINFO_CLASS_HANDLE* pTypeHnd, - CORINFO_CLASS_HANDLE owner) + CORINFO_CLASS_HANDLE fieldOwnerHint) { CONTRACTL { THROWS; @@ -9154,7 +9150,7 @@ CorInfoType CEEInfo::getFieldType (CORINFO_FIELD_HANDLE fieldHnd, JIT_TO_EE_TRANSITION(); - result = getFieldTypeInternal(fieldHnd, pTypeHnd, owner); + result = getFieldTypeInternal(fieldHnd, pTypeHnd, fieldOwnerHint); EE_TO_JIT_TRANSITION(); @@ -9164,7 +9160,7 @@ CorInfoType CEEInfo::getFieldType (CORINFO_FIELD_HANDLE fieldHnd, /*********************************************************************/ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, CORINFO_CLASS_HANDLE* pTypeHnd, - CORINFO_CLASS_HANDLE owner) + CORINFO_CLASS_HANDLE fieldOwnerHint) { STANDARD_VM_CONTRACT; @@ -9189,10 +9185,33 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, SigPointer ptr(sig, sigCount); + // Actual field's owner + MethodTable* actualFieldsOwner = field->GetApproxEnclosingMethodTable(); + + // Potentially, a more specific field's owner (hint) + TypeHandle hintedFieldOwner = TypeHandle(fieldOwnerHint); + + // Validate the hint: + TypeHandle moreExactFieldOwner = TypeHandle(); + if (!hintedFieldOwner.IsNull() && !hintedFieldOwner.IsTypeDesc()) + { + MethodTable* matchingHintedFieldOwner = hintedFieldOwner.AsMethodTable()->GetMethodTableMatchingParentClass(actualFieldsOwner); + if (matchingHintedFieldOwner != NULL) + { + // we take matchingHintedFieldOwner only if actualFieldsOwner is shared and matchingHintedFieldOwner + // is not, hence, it's definitely more exact. + if (actualFieldsOwner->IsSharedByGenericInstantiations() && + !matchingHintedFieldOwner->IsSharedByGenericInstantiations()) + { + moreExactFieldOwner = matchingHintedFieldOwner; + } + } + } + // For verifying code involving generics, use the class instantiation // of the optional owner (to provide exact, not representative, // type information) - SigTypeContext typeContext(field, (TypeHandle)owner); + SigTypeContext typeContext(field, moreExactFieldOwner); clsHnd = ptr.GetTypeHandleThrowing(field->GetModule(), &typeContext); _ASSERTE(!clsHnd.IsNull());