From 605ec9838a8fe6ce1b2bbf1b1f6295e889e45151 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 00:43:07 +0200 Subject: [PATCH 01/19] Skip more covariance checks --- src/coreclr/jit/gentree.cpp | 18 ++++++++++++++++++ src/coreclr/jit/morph.cpp | 13 +++++-------- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d4d29595d2b033..28b5382e41a68b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19174,6 +19174,24 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b if (fieldType == TYP_REF) { objClass = fieldClass; + if (!info.compCompHnd->isFieldStatic(fieldHnd)) + { + // See if we can make objClass more specific, e.g. objClass is _Canon[] + // but we can extract the real field type from field's owner: + bool objIsExact, objIsNonNull; + CORINFO_CLASS_HANDLE fieldOwnerObj = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); + if (fieldOwnerObj != NO_CLASS_HANDLE) + { + CORINFO_CLASS_HANDLE fieldExactCls = NO_CLASS_HANDLE; + info.compCompHnd->getFieldType(fieldHnd, &fieldExactCls, fieldOwnerObj); + + if ((fieldExactCls != NO_CLASS_HANDLE) && ((objClass == NO_CLASS_HANDLE) || + info.compCompHnd->isMoreSpecificType(objClass, fieldExactCls))) + { + objClass = fieldExactCls; + } + } + } } } } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index f8989a40f405ff..3b15582b300bf2 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 (impCanSkipCovariantStoreCheck(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; From e36bfb9c5aa81aa5afef0997c8caa9cf511bdda0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 01:09:59 +0200 Subject: [PATCH 02/19] formatting --- src/coreclr/jit/gentree.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 28b5382e41a68b..07e0beac0d6ac4 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19185,8 +19185,9 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b CORINFO_CLASS_HANDLE fieldExactCls = NO_CLASS_HANDLE; info.compCompHnd->getFieldType(fieldHnd, &fieldExactCls, fieldOwnerObj); - if ((fieldExactCls != NO_CLASS_HANDLE) && ((objClass == NO_CLASS_HANDLE) || - info.compCompHnd->isMoreSpecificType(objClass, fieldExactCls))) + if ((fieldExactCls != NO_CLASS_HANDLE) && + ((objClass == NO_CLASS_HANDLE) || + info.compCompHnd->isMoreSpecificType(objClass, fieldExactCls))) { objClass = fieldExactCls; } From 4d5967968f7d4c043033a140921d293ec3e5fcf3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 01:27:13 +0200 Subject: [PATCH 03/19] formatting --- src/coreclr/jit/gentree.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 07e0beac0d6ac4..d6cddea1efb51c 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19174,10 +19174,13 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b if (fieldType == TYP_REF) { objClass = fieldClass; - if (!info.compCompHnd->isFieldStatic(fieldHnd)) + + // See if we can make objClass more specific, e.g. objClass is _Canon[] + // but we can extract the real field type from field's owner: + if (op1->TypeIs(TYP_REF)) { - // See if we can make objClass more specific, e.g. objClass is _Canon[] - // but we can extract the real field type from field's owner: + assert(!info.compCompHnd->isFieldStatic(fieldHnd)); + bool objIsExact, objIsNonNull; CORINFO_CLASS_HANDLE fieldOwnerObj = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); if (fieldOwnerObj != NO_CLASS_HANDLE) From ca57fc954966cb2151fda469336f63462383a677 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 02:06:42 +0200 Subject: [PATCH 04/19] Clean up --- src/coreclr/jit/compiler.h | 4 +++- src/coreclr/jit/ee_il_dll.hpp | 6 ++++-- src/coreclr/jit/gentree.cpp | 34 ++++++++++------------------------ 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8730693773f664..0fb5d040f5129e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8295,7 +8295,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 5ee1c7510d2755..356a16107bb0a0 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 memberParent) { - return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd)); + return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); } FORCEINLINE diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d6cddea1efb51c..131b81b20c93e2 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19168,34 +19168,20 @@ 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 (op1->TypeIs(TYP_REF) && !eeIsFieldStatic(fieldHnd)) { - objClass = fieldClass; - - // See if we can make objClass more specific, e.g. objClass is _Canon[] - // but we can extract the real field type from field's owner: - if (op1->TypeIs(TYP_REF)) - { - assert(!info.compCompHnd->isFieldStatic(fieldHnd)); + bool objIsExact, objIsNonNull; + fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); + } - bool objIsExact, objIsNonNull; - CORINFO_CLASS_HANDLE fieldOwnerObj = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); - if (fieldOwnerObj != NO_CLASS_HANDLE) - { - CORINFO_CLASS_HANDLE fieldExactCls = NO_CLASS_HANDLE; - info.compCompHnd->getFieldType(fieldHnd, &fieldExactCls, fieldOwnerObj); - - if ((fieldExactCls != NO_CLASS_HANDLE) && - ((objClass == NO_CLASS_HANDLE) || - info.compCompHnd->isMoreSpecificType(objClass, fieldExactCls))) - { - objClass = fieldExactCls; - } - } - } + if (eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) + { + objClass = fieldClass; } } } From 7775ae9717cbe3457cf5bf8856c64a396e26dc7d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 14:11:40 +0200 Subject: [PATCH 05/19] Address feedback + fix CI --- src/coreclr/jit/compiler.h | 4 +- src/coreclr/jit/ee_il_dll.hpp | 9 +++ src/coreclr/jit/gentree.cpp | 118 +++++++++++++++++++++++++++++++++- src/coreclr/jit/importer.cpp | 117 +-------------------------------- src/coreclr/jit/morph.cpp | 2 +- 5 files changed, 129 insertions(+), 121 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0fb5d040f5129e..aa3e89da4745ac 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3692,6 +3692,8 @@ class Compiler return callMethodHandle == info.compMethodHnd; } + bool gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array); + //------------------------------------------------------------------------- GenTree* gtFoldExpr(GenTree* tree); @@ -5150,8 +5152,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); /* diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 356a16107bb0a0..66278e47f9fb31 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -82,6 +82,15 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd, CORINFO_CLASS_HANDLE memberParent) { + + // memberParent is an opportunistic hint to get a more exact field type + if ((memberParent != NO_CLASS_HANDLE) && + !info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) + { + // Ignore it if it doesn't help (e.g. the input hint was just System.Object) + memberParent = NO_CLASS_HANDLE; + } + return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 131b81b20c93e2..c60bff8c10202b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19171,9 +19171,8 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b CORINFO_CLASS_HANDLE fieldOwner = NO_CLASS_HANDLE; CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE; - // fieldOwner helps us to get a more exact field class - // for instance fields - if (op1->TypeIs(TYP_REF) && !eeIsFieldStatic(fieldHnd)) + // fieldOwner helps us to get a more exact field class for instance fields + if (!fieldSeq->IsStaticField()) { bool objIsExact, objIsNonNull; fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); @@ -31848,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 793bed41dd2e20..d1bf541169c159 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -7079,7 +7079,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)) { @@ -7218,7 +7218,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; @@ -13908,116 +13908,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/morph.cpp b/src/coreclr/jit/morph.cpp index 3b15582b300bf2..c4f1c04f10ef95 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -7095,7 +7095,7 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call) GenTree* index = call->gtArgs.GetArgByIndex(1)->GetNode(); GenTree* value = call->gtArgs.GetArgByIndex(2)->GetNode(); - if (impCanSkipCovariantStoreCheck(value, arr)) + 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. From 40839fa325963298b360a7e7f0510515d2823cf7 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 16:59:21 +0200 Subject: [PATCH 06/19] Test change --- src/coreclr/jit/ee_il_dll.hpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 66278e47f9fb31..b37e01395d6c96 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -84,11 +84,25 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, { // memberParent is an opportunistic hint to get a more exact field type - if ((memberParent != NO_CLASS_HANDLE) && - !info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) + if (memberParent != NO_CLASS_HANDLE) { + bool hasTheField = false; + if (info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) + { + // Make sure the hinted class actually has our field + unsigned fields = info.compCompHnd->getClassNumInstanceFields(memberParent); + for (unsigned i = 0; i < fields; i++) + { + if (info.compCompHnd->getFieldInClass(memberParent, i) == fldHnd) + { + hasTheField = true; + break; + } + } + } + // Ignore it if it doesn't help (e.g. the input hint was just System.Object) - memberParent = NO_CLASS_HANDLE; + memberParent = hasTheField ? memberParent : NO_CLASS_HANDLE; } return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); From c154a55e48ccfeadc11be29d2cc9573ca45f13e0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 29 Aug 2024 23:17:03 +0200 Subject: [PATCH 07/19] Test change --- src/coreclr/jit/ee_il_dll.hpp | 22 +++------------------- src/coreclr/vm/jitinterface.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index b37e01395d6c96..6fca76dd89260d 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -82,29 +82,13 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd, CORINFO_CLASS_HANDLE memberParent) { - // memberParent is an opportunistic hint to get a more exact field type - if (memberParent != NO_CLASS_HANDLE) + if ((memberParent != NO_CLASS_HANDLE) && + !info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) { - bool hasTheField = false; - if (info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) - { - // Make sure the hinted class actually has our field - unsigned fields = info.compCompHnd->getClassNumInstanceFields(memberParent); - for (unsigned i = 0; i < fields; i++) - { - if (info.compCompHnd->getFieldInClass(memberParent, i) == fldHnd) - { - hasTheField = true; - break; - } - } - } - // Ignore it if it doesn't help (e.g. the input hint was just System.Object) - memberParent = hasTheField ? memberParent : NO_CLASS_HANDLE; + memberParent = NO_CLASS_HANDLE; } - return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e7572863016f08..eebe762e42cf2a 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -4363,15 +4363,15 @@ 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 isHnd1CanonSubtype && !isHnd2CanonSubtype; } // Otherwise both types are either shared or not shared. From eb5295e208c355b0c554e91fa05be544da524f6c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 30 Aug 2024 01:09:38 +0200 Subject: [PATCH 08/19] fix r2r/naot --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 06141640ef53cb..97bdc32934f877 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2906,11 +2906,10 @@ private bool isMoreSpecificType(CORINFO_CLASS_STRUCT_* cls1, CORINFO_CLASS_STRUC // consider the unshared type as more specific. bool isType1CanonSubtype = type1.IsCanonicalSubtype(CanonicalFormKind.Any); bool isType2CanonSubtype = type2.IsCanonicalSubtype(CanonicalFormKind.Any); - if (isType1CanonSubtype != isType2CanonSubtype) + + 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 isType1CanonSubtype && !isType2CanonSubtype; } // Otherwise both types are either shared or not shared. From 015a1347214a17ff7ab353b32106f17a51368fdf Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 30 Aug 2024 01:13:25 +0200 Subject: [PATCH 09/19] add comments --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 97bdc32934f877..ddf7d2b6093b10 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2907,6 +2907,8 @@ private bool isMoreSpecificType(CORINFO_CLASS_STRUCT_* cls1, CORINFO_CLASS_STRUC bool isType1CanonSubtype = type1.IsCanonicalSubtype(CanonicalFormKind.Any); bool isType2CanonSubtype = type2.IsCanonicalSubtype(CanonicalFormKind.Any); + // If both types have the same type definition while + // hnd1 is shared and hnd2 is not - consider hnd2 more specific. if (type1.HasSameTypeDefinition(type2)) { return isType1CanonSubtype && !isType2CanonSubtype; From 727aac472009fc2bdb9c69eab2b0c5a5b51b5e88 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 30 Aug 2024 16:34:56 +0200 Subject: [PATCH 10/19] Address feedback --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 8 +------- src/coreclr/vm/jitinterface.cpp | 6 +----- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index ddf7d2b6093b10..8185f1f0441f3e 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -2902,19 +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 both types have the same type definition while // hnd1 is shared and hnd2 is not - consider hnd2 more specific. if (type1.HasSameTypeDefinition(type2)) { - return isType1CanonSubtype && !isType2CanonSubtype; + 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); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index eebe762e42cf2a..e0c8ab0255523d 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; } - BOOL isHnd1CanonSubtype = hnd1.IsCanonicalSubtype(); - BOOL isHnd2CanonSubtype = hnd2.IsCanonicalSubtype(); - // 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())) { - return isHnd1CanonSubtype && !isHnd2CanonSubtype; + 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); From a63f4cfd8494badbd1342b6e47e69fd0a6f4550e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 1 Sep 2024 20:53:26 +0200 Subject: [PATCH 11/19] add validation --- src/coreclr/jit/ee_il_dll.hpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 6fca76dd89260d..60804b9f59c6c9 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -83,11 +83,26 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE memberParent) { // memberParent is an opportunistic hint to get a more exact field type - if ((memberParent != NO_CLASS_HANDLE) && - !info.compCompHnd->isMoreSpecificType(info.compCompHnd->getFieldClass(fldHnd), memberParent)) + if (memberParent != NO_CLASS_HANDLE) { + bool useHint = false; + CORINFO_FIELD_HANDLE fld = info.compCompHnd->getFieldClass(fldHnd); + if (info.compCompHnd->isMoreSpecificType(fld, memberParent)) + { + // Now validate that the more specific class actually contains the field: + unsigned fields = info.compCompHnd->getClassNumInstanceFields(memberParent); + for (unsigned i = 0; i < fields; i++) + { + if (info.compCompHnd->getFieldInClass(memberParent, i) == fld) + { + useHint = true; + break; + } + } + } + // Ignore it if it doesn't help (e.g. the input hint was just System.Object) - memberParent = NO_CLASS_HANDLE; + memberParent = useHint ? memberParent : NO_CLASS_HANDLE; } return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); } From dd0c21703f20c03a10fc3470ed29f0dbd0114a69 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 1 Sep 2024 21:54:32 +0200 Subject: [PATCH 12/19] fix build --- src/coreclr/jit/ee_il_dll.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 60804b9f59c6c9..cb2b91e613ae4e 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -85,15 +85,15 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, // memberParent is an opportunistic hint to get a more exact field type if (memberParent != NO_CLASS_HANDLE) { - bool useHint = false; - CORINFO_FIELD_HANDLE fld = info.compCompHnd->getFieldClass(fldHnd); - if (info.compCompHnd->isMoreSpecificType(fld, memberParent)) + bool useHint = false; + CORINFO_CLASS_HANDLE fieldOwner = info.compCompHnd->getFieldClass(fldHnd); + if (info.compCompHnd->isMoreSpecificType(fieldOwner, memberParent)) { // Now validate that the more specific class actually contains the field: unsigned fields = info.compCompHnd->getClassNumInstanceFields(memberParent); for (unsigned i = 0; i < fields; i++) { - if (info.compCompHnd->getFieldInClass(memberParent, i) == fld) + if (info.compCompHnd->getFieldInClass(memberParent, i) == fldHnd) { useHint = true; break; From 6315c71b20f6712a708f09ff2317bffa7bd4f657 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 2 Sep 2024 19:46:44 +0200 Subject: [PATCH 13/19] Address feedback --- src/coreclr/jit/ee_il_dll.hpp | 22 ---------------- src/coreclr/jit/importercalls.cpp | 4 +-- src/coreclr/vm/jitinterface.cpp | 44 ++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index cb2b91e613ae4e..356a16107bb0a0 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -82,28 +82,6 @@ var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd, CORINFO_CLASS_HANDLE memberParent) { - // memberParent is an opportunistic hint to get a more exact field type - if (memberParent != NO_CLASS_HANDLE) - { - bool useHint = false; - CORINFO_CLASS_HANDLE fieldOwner = info.compCompHnd->getFieldClass(fldHnd); - if (info.compCompHnd->isMoreSpecificType(fieldOwner, memberParent)) - { - // Now validate that the more specific class actually contains the field: - unsigned fields = info.compCompHnd->getClassNumInstanceFields(memberParent); - for (unsigned i = 0; i < fields; i++) - { - if (info.compCompHnd->getFieldInClass(memberParent, i) == fldHnd) - { - useHint = true; - break; - } - } - } - - // Ignore it if it doesn't help (e.g. the input hint was just System.Object) - memberParent = useHint ? memberParent : NO_CLASS_HANDLE; - } return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index e0fc0e0dbacc7e..891e264863fef4 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2908,11 +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)); + 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 diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e0c8ab0255523d..45b16355ab8e82 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9179,10 +9179,52 @@ 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(owner); + + // Validate the hint: + if (!hintedFieldOwner.IsNull()) + { + bool isValidParent = false; + if (!hintedFieldOwner.IsTypeDesc()) + { + // Now we need to check whether hinted ownerTh is an actual + // subclass of fieldsOwner. + + MethodTable* hintedFieldOwnerParent = hintedFieldOwner.AsMethodTable(); + while (hintedFieldOwnerParent != nullptr) + { + if (hintedFieldOwnerParent->HasSameTypeDefAs(actualFieldsOwner)) + { + // If the hinted owner has the same definition as the actualFieldsOwner + // we'll use the hinted handle only in case if it's more specific: + if (hintedFieldOwnerParent == hintedFieldOwner.AsMethodTable()) + { + // actual is shared and hinted is not -> hinted is more specific + isValidParent = + TypeHandle(actualFieldsOwner).IsCanonicalSubtype() && + !hintedFieldOwner.IsCanonicalSubtype(); + break; + } + + // It's a valid subclass! + isValidParent = true; + break; + } + hintedFieldOwnerParent = hintedFieldOwnerParent->GetParentMethodTable(); + } + } + // Ignore invalid hints + hintedFieldOwner = isValidParent ? hintedFieldOwner : TypeHandle(); + } + // 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, hintedFieldOwner); clsHnd = ptr.GetTypeHandleThrowing(field->GetModule(), &typeContext); _ASSERTE(!clsHnd.IsNull()); From b1dc478c64f8de28571de5978dff1be744a31ee0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 2 Sep 2024 19:47:56 +0200 Subject: [PATCH 14/19] fix build --- src/coreclr/jit/gentree.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index c60bff8c10202b..8d5f0ddb69e429 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19178,7 +19178,8 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); } - if (eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) + // TODO: implement for NAOT in this PR.. + if (!opts.IsReadyToRun() && eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) { objClass = fieldClass; } From 463e540f710c000f88809c6002b54c99fc009dc9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 2 Sep 2024 19:49:07 +0200 Subject: [PATCH 15/19] fix formatting --- src/coreclr/jit/importercalls.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 891e264863fef4..c1f01983993f2e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2909,9 +2909,8 @@ GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig) } CORINFO_CLASS_HANDLE fieldClsHnd; - var_types fieldElementType = - JITtype2varType(info.compCompHnd->getFieldType(fieldToken, &fieldClsHnd)); - 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 From 6b746a69efc55b27f217ed7c1816d2db79b3b263 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 2 Sep 2024 22:57:04 +0200 Subject: [PATCH 16/19] Address feedback --- src/coreclr/inc/corinfo.h | 6 +- src/coreclr/inc/icorjitinfoimpl_generated.h | 2 +- .../jit/ICorJitInfo_wrapper_generated.hpp | 4 +- src/coreclr/jit/ee_il_dll.hpp | 4 +- src/coreclr/jit/gentree.cpp | 3 +- .../tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../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 | 55 +++++++------------ 16 files changed, 56 insertions(+), 72 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 30c7b0ab95d8f1..566d1ecb769a4f 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 96068326c34704..bbaf43a6ee0ce0 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 0ee637a957cf39..ff3270192dc5f7 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/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index 356a16107bb0a0..32289aff62dac2 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -80,9 +80,9 @@ bool Compiler::eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd) FORCEINLINE var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd, - CORINFO_CLASS_HANDLE memberParent) + CORINFO_CLASS_HANDLE fieldOwnerHint) { - return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, memberParent)); + return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, fieldOwnerHint)); } FORCEINLINE diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 8d5f0ddb69e429..c60bff8c10202b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19178,8 +19178,7 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull); } - // TODO: implement for NAOT in this PR.. - if (!opts.IsReadyToRun() && eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) + if (eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF) { objClass = fieldClass; } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 8185f1f0441f3e..987bbc6508f132 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3101,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 76b08b42458b38..9213d42aba3272 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 e776aa2796259f..76d3ea9d3864af 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 c56fe139318630..0ba43d2a45885f 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 d1078fe8d356ca..ddfc401aff1f26 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 4625ad44aab37d..ffe0e7aaa5bdc7 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 b3fab2e939dae8..5cf11bf9420dfb 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 638f6649613fa5..9d5eb18dfc4d9f 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 c285ef2c3c6ca9..8e3bff4eb70674 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 64c52f61705ae5..da9cc9f24ecb20 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 45b16355ab8e82..01fbca044b3d30 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9128,11 +9128,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; @@ -9144,7 +9144,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(); @@ -9154,7 +9154,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; @@ -9183,43 +9183,28 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, MethodTable* actualFieldsOwner = field->GetApproxEnclosingMethodTable(); // Potentially, a more specific field's owner (hint) - TypeHandle hintedFieldOwner = TypeHandle(owner); + TypeHandle hintedFieldOwner = TypeHandle(fieldOwnerHint); // Validate the hint: - if (!hintedFieldOwner.IsNull()) + bool isHintValid = false; + if (!hintedFieldOwner.IsNull() && !hintedFieldOwner.IsTypeDesc()) { - bool isValidParent = false; - if (!hintedFieldOwner.IsTypeDesc()) + MethodTable* hintedFieldOwnerParent = hintedFieldOwner.AsMethodTable(); + if (hintedFieldOwnerParent->HasSameTypeDefAs(actualFieldsOwner)) { - // Now we need to check whether hinted ownerTh is an actual - // subclass of fieldsOwner. - - MethodTable* hintedFieldOwnerParent = hintedFieldOwner.AsMethodTable(); - while (hintedFieldOwnerParent != nullptr) - { - if (hintedFieldOwnerParent->HasSameTypeDefAs(actualFieldsOwner)) - { - // If the hinted owner has the same definition as the actualFieldsOwner - // we'll use the hinted handle only in case if it's more specific: - if (hintedFieldOwnerParent == hintedFieldOwner.AsMethodTable()) - { - // actual is shared and hinted is not -> hinted is more specific - isValidParent = - TypeHandle(actualFieldsOwner).IsCanonicalSubtype() && - !hintedFieldOwner.IsCanonicalSubtype(); - break; - } - - // It's a valid subclass! - isValidParent = true; - break; - } - hintedFieldOwnerParent = hintedFieldOwnerParent->GetParentMethodTable(); - } + // if actualFieldsOwner has the same definition as hintedFieldOwner, we + // take hintedFieldOwner only if actualFieldsOwner is shared and hintedFieldOwner + // is not, hence, it's definitely more exact. + isHintValid = TypeHandle(actualFieldsOwner).IsCanonicalSubtype() && + !hintedFieldOwner.IsCanonicalSubtype(); + } + else if (hintedFieldOwnerParent->GetMethodTableMatchingParentClass(actualFieldsOwner) != nullptr) + { + // Otherwise, use hintedFieldOwner only if it's a subclass of actualFieldsOwner + isHintValid = true; } - // Ignore invalid hints - hintedFieldOwner = isValidParent ? hintedFieldOwner : TypeHandle(); } + hintedFieldOwner = isHintValid ? hintedFieldOwner : TypeHandle(); // For verifying code involving generics, use the class instantiation // of the optional owner (to provide exact, not representative, From a1b5ba7c5a28e7310ac014bf627a4884cbf8789b Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 4 Sep 2024 15:54:02 +0200 Subject: [PATCH 17/19] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 01fbca044b3d30..f7156e11868152 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9186,25 +9186,21 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, TypeHandle hintedFieldOwner = TypeHandle(fieldOwnerHint); // Validate the hint: - bool isHintValid = false; + TypeHandle moreExactFieldOwner = TypeHandle(); if (!hintedFieldOwner.IsNull() && !hintedFieldOwner.IsTypeDesc()) { - MethodTable* hintedFieldOwnerParent = hintedFieldOwner.AsMethodTable(); - if (hintedFieldOwnerParent->HasSameTypeDefAs(actualFieldsOwner)) + MethodTable* matchingHintedFieldOwner = hintedFieldOwner.AsMethodTable()->GetMethodTableMatchingParentClass(actualFieldsOwner); + if (matchingHintedFieldOwner != NULL) { - // if actualFieldsOwner has the same definition as hintedFieldOwner, we - // take hintedFieldOwner only if actualFieldsOwner is shared and hintedFieldOwner + // we take matchingHintedFieldOwner only if actualFieldsOwner is shared and matchingHintedFieldOwner // is not, hence, it's definitely more exact. - isHintValid = TypeHandle(actualFieldsOwner).IsCanonicalSubtype() && - !hintedFieldOwner.IsCanonicalSubtype(); - } - else if (hintedFieldOwnerParent->GetMethodTableMatchingParentClass(actualFieldsOwner) != nullptr) - { - // Otherwise, use hintedFieldOwner only if it's a subclass of actualFieldsOwner - isHintValid = true; + if (actualFieldsOwner->IsSharedByGenericInstantiations() && + !matchingHintedFieldOwner->IsSharedByGenericInstantiations()) + { + moreExactFieldOwner = matchingHintedFieldOwner; + } } } - hintedFieldOwner = isHintValid ? hintedFieldOwner : TypeHandle(); // For verifying code involving generics, use the class instantiation // of the optional owner (to provide exact, not representative, From b95ffbb98e2cfa99947c2628e9cbc84a240530e0 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 4 Sep 2024 15:54:09 +0200 Subject: [PATCH 18/19] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index f7156e11868152..2e8a5ab8c63f32 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9205,7 +9205,7 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, // For verifying code involving generics, use the class instantiation // of the optional owner (to provide exact, not representative, // type information) - SigTypeContext typeContext(field, hintedFieldOwner); + SigTypeContext typeContext(field, moreExactFieldOwner); clsHnd = ptr.GetTypeHandleThrowing(field->GetModule(), &typeContext); _ASSERTE(!clsHnd.IsNull()); From 931bc0f9c1561a421eba3f3b3f20ff8e12511b39 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 4 Sep 2024 11:54:19 -0700 Subject: [PATCH 19/19] Update src/coreclr/vm/jitinterface.cpp --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 2e8a5ab8c63f32..7a35633ce9f1bc 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -9197,7 +9197,7 @@ CorInfoType CEEInfo::getFieldTypeInternal (CORINFO_FIELD_HANDLE fieldHnd, if (actualFieldsOwner->IsSharedByGenericInstantiations() && !matchingHintedFieldOwner->IsSharedByGenericInstantiations()) { - moreExactFieldOwner = matchingHintedFieldOwner; + moreExactFieldOwner = matchingHintedFieldOwner; } } }