Skip to content

Commit 0ae29ed

Browse files
EgorBojkotas
authored andcommitted
JIT: Skip more covariance checks (dotnet#107116)
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 75b693a commit 0ae29ed

File tree

20 files changed

+210
-191
lines changed

20 files changed

+210
-191
lines changed

src/coreclr/inc/corinfo.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,12 +2750,12 @@ class ICorStaticInfo
27502750
// the field's value class (if 'structType' == 0, then don't bother
27512751
// the structure info).
27522752
//
2753-
// 'memberParent' is typically only set when verifying. It should be the
2754-
// result of calling getMemberParent.
2753+
// 'fieldOwnerHint' is, potentially, a more exact owner of the field.
2754+
// it's fine for it to be non-precise, it's just a hint.
27552755
virtual CorInfoType getFieldType(
27562756
CORINFO_FIELD_HANDLE field,
27572757
CORINFO_CLASS_HANDLE * structType = NULL,
2758-
CORINFO_CLASS_HANDLE memberParent = NULL /* IN */
2758+
CORINFO_CLASS_HANDLE fieldOwnerHint = NULL /* IN */
27592759
) = 0;
27602760

27612761
// return the data member's instance offset

src/coreclr/inc/icorjitinfoimpl_generated.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ CORINFO_CLASS_HANDLE getFieldClass(
395395
CorInfoType getFieldType(
396396
CORINFO_FIELD_HANDLE field,
397397
CORINFO_CLASS_HANDLE* structType,
398-
CORINFO_CLASS_HANDLE memberParent) override;
398+
CORINFO_CLASS_HANDLE fieldOwnerHint) override;
399399

400400
unsigned getFieldOffset(
401401
CORINFO_FIELD_HANDLE field) override;

src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -939,10 +939,10 @@ CORINFO_CLASS_HANDLE WrapICorJitInfo::getFieldClass(
939939
CorInfoType WrapICorJitInfo::getFieldType(
940940
CORINFO_FIELD_HANDLE field,
941941
CORINFO_CLASS_HANDLE* structType,
942-
CORINFO_CLASS_HANDLE memberParent)
942+
CORINFO_CLASS_HANDLE fieldOwnerHint)
943943
{
944944
API_ENTER(getFieldType);
945-
CorInfoType temp = wrapHnd->getFieldType(field, structType, memberParent);
945+
CorInfoType temp = wrapHnd->getFieldType(field, structType, fieldOwnerHint);
946946
API_LEAVE(getFieldType);
947947
return temp;
948948
}

src/coreclr/jit/compiler.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3695,6 +3695,8 @@ class Compiler
36953695
return callMethodHandle == info.compMethodHnd;
36963696
}
36973697

3698+
bool gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array);
3699+
36983700
//-------------------------------------------------------------------------
36993701

37003702
GenTree* gtFoldExpr(GenTree* tree);
@@ -5153,8 +5155,6 @@ class Compiler
51535155
bool impIsImplicitTailCallCandidate(
51545156
OPCODE curOpcode, const BYTE* codeAddrOfNextOpcode, const BYTE* codeEnd, int prefixFlags, bool isRecursive);
51555157

5156-
bool impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array);
5157-
51585158
methodPointerInfo* impAllocateMethodPointerInfo(const CORINFO_RESOLVED_TOKEN& token, mdToken tokenConstrained);
51595159

51605160
/*
@@ -8294,7 +8294,9 @@ class Compiler
82948294
bool eeIsIntrinsic(CORINFO_METHOD_HANDLE ftn);
82958295
bool eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd);
82968296

8297-
var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd = nullptr);
8297+
var_types eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd,
8298+
CORINFO_CLASS_HANDLE* pStructHnd = nullptr,
8299+
CORINFO_CLASS_HANDLE memberParent = NO_CLASS_HANDLE);
82988300

82998301
template <typename TPrint>
83008302
void eeAppendPrint(class StringPrinter* printer, TPrint print);

src/coreclr/jit/ee_il_dll.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,11 @@ bool Compiler::eeIsFieldStatic(CORINFO_FIELD_HANDLE fldHnd)
7878
}
7979

8080
FORCEINLINE
81-
var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE* pStructHnd)
81+
var_types Compiler::eeGetFieldType(CORINFO_FIELD_HANDLE fldHnd,
82+
CORINFO_CLASS_HANDLE* pStructHnd,
83+
CORINFO_CLASS_HANDLE fieldOwnerHint)
8284
{
83-
return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd));
85+
return JITtype2varType(info.compCompHnd->getFieldType(fldHnd, pStructHnd, fieldOwnerHint));
8486
}
8587

8688
FORCEINLINE

src/coreclr/jit/gentree.cpp

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19168,10 +19168,17 @@ CORINFO_CLASS_HANDLE Compiler::gtGetClassHandle(GenTree* tree, bool* pIsExact, b
1916819168
// No benefit to calling gtGetFieldClassHandle here, as
1916919169
// the exact field being accessed can vary.
1917019170
CORINFO_FIELD_HANDLE fieldHnd = fieldSeq->GetFieldHandle();
19171+
CORINFO_CLASS_HANDLE fieldOwner = NO_CLASS_HANDLE;
1917119172
CORINFO_CLASS_HANDLE fieldClass = NO_CLASS_HANDLE;
19172-
var_types fieldType = eeGetFieldType(fieldHnd, &fieldClass);
1917319173

19174-
if (fieldType == TYP_REF)
19174+
// fieldOwner helps us to get a more exact field class for instance fields
19175+
if (!fieldSeq->IsStaticField())
19176+
{
19177+
bool objIsExact, objIsNonNull;
19178+
fieldOwner = gtGetClassHandle(op1, &objIsExact, &objIsNonNull);
19179+
}
19180+
19181+
if (eeGetFieldType(fieldHnd, &fieldClass, fieldOwner) == TYP_REF)
1917519182
{
1917619183
objClass = fieldClass;
1917719184
}
@@ -31840,3 +31847,116 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
3184031847
return resultNode;
3184131848
}
3184231849
#endif // FEATURE_HW_INTRINSICS
31850+
31851+
//------------------------------------------------------------------------
31852+
// gtCanSkipCovariantStoreCheck: see if storing a ref type value to an array
31853+
// can skip the array store covariance check.
31854+
//
31855+
// Arguments:
31856+
// value -- tree producing the value to store
31857+
// array -- tree representing the array to store to
31858+
//
31859+
// Returns:
31860+
// true if the store does not require a covariance check.
31861+
//
31862+
bool Compiler::gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array)
31863+
{
31864+
// We should only call this when optimizing.
31865+
assert(opts.OptimizationEnabled());
31866+
31867+
// Check for store to same array, ie. arrLcl[i] = arrLcl[j]
31868+
if (value->OperIs(GT_IND) && value->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && array->OperIs(GT_LCL_VAR))
31869+
{
31870+
GenTree* valueArray = value->AsIndir()->Addr()->AsIndexAddr()->Arr();
31871+
if (valueArray->OperIs(GT_LCL_VAR))
31872+
{
31873+
unsigned valueArrayLcl = valueArray->AsLclVar()->GetLclNum();
31874+
unsigned arrayLcl = array->AsLclVar()->GetLclNum();
31875+
if ((valueArrayLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed())
31876+
{
31877+
JITDUMP("\nstelem of ref from same array: skipping covariant store check\n");
31878+
return true;
31879+
}
31880+
}
31881+
}
31882+
31883+
// Check for store of NULL.
31884+
if (value->OperIs(GT_CNS_INT))
31885+
{
31886+
assert(value->gtType == TYP_REF);
31887+
if (value->AsIntCon()->gtIconVal == 0)
31888+
{
31889+
JITDUMP("\nstelem of null: skipping covariant store check\n");
31890+
return true;
31891+
}
31892+
// Non-0 const refs can only occur with frozen objects
31893+
assert(value->IsIconHandle(GTF_ICON_OBJ_HDL));
31894+
assert(doesMethodHaveFrozenObjects() ||
31895+
(compIsForInlining() && impInlineInfo->InlinerCompiler->doesMethodHaveFrozenObjects()));
31896+
}
31897+
31898+
// Try and get a class handle for the array
31899+
if (!value->TypeIs(TYP_REF))
31900+
{
31901+
return false;
31902+
}
31903+
31904+
bool arrayIsExact = false;
31905+
bool arrayIsNonNull = false;
31906+
CORINFO_CLASS_HANDLE arrayHandle = gtGetClassHandle(array, &arrayIsExact, &arrayIsNonNull);
31907+
31908+
if (arrayHandle == NO_CLASS_HANDLE)
31909+
{
31910+
return false;
31911+
}
31912+
31913+
// There are some methods in corelib where we're storing to an array but the IL
31914+
// doesn't reflect this (see SZArrayHelper). Avoid.
31915+
DWORD attribs = info.compCompHnd->getClassAttribs(arrayHandle);
31916+
if ((attribs & CORINFO_FLG_ARRAY) == 0)
31917+
{
31918+
return false;
31919+
}
31920+
31921+
CORINFO_CLASS_HANDLE arrayElementHandle = nullptr;
31922+
CorInfoType arrayElemType = info.compCompHnd->getChildType(arrayHandle, &arrayElementHandle);
31923+
31924+
// Verify array type handle is really an array of ref type
31925+
assert(arrayElemType == CORINFO_TYPE_CLASS);
31926+
31927+
// Check for exactly object[]
31928+
if (arrayIsExact && (arrayElementHandle == impGetObjectClass()))
31929+
{
31930+
JITDUMP("\nstelem to (exact) object[]: skipping covariant store check\n");
31931+
return true;
31932+
}
31933+
31934+
const bool arrayTypeIsSealed = info.compCompHnd->isExactType(arrayElementHandle);
31935+
31936+
if ((!arrayIsExact && !arrayTypeIsSealed) || (arrayElementHandle == NO_CLASS_HANDLE))
31937+
{
31938+
// Bail out if we don't know array's exact type
31939+
return false;
31940+
}
31941+
31942+
bool valueIsExact = false;
31943+
bool valueIsNonNull = false;
31944+
CORINFO_CLASS_HANDLE valueHandle = gtGetClassHandle(value, &valueIsExact, &valueIsNonNull);
31945+
31946+
// Array's type is sealed and equals to value's type
31947+
if (arrayTypeIsSealed && (valueHandle == arrayElementHandle))
31948+
{
31949+
JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n");
31950+
return true;
31951+
}
31952+
31953+
// Array's type is not sealed but we know its exact type
31954+
if (arrayIsExact && (valueHandle != NO_CLASS_HANDLE) &&
31955+
(info.compCompHnd->compareTypesForCast(valueHandle, arrayElementHandle) == TypeCompareState::Must))
31956+
{
31957+
JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n");
31958+
return true;
31959+
}
31960+
31961+
return false;
31962+
}

src/coreclr/jit/importer.cpp

Lines changed: 2 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -7069,7 +7069,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
70697069
lclTyp = JITtype2varType(info.compCompHnd->asCorInfoType(ldelemClsHnd));
70707070

70717071
// If it's a value class / pointer array, or a readonly access, we don't need a type check.
7072-
// TODO-CQ: adapt "impCanSkipCovariantStoreCheck" to handle "ldelema"s and call it here to
7072+
// TODO-CQ: adapt "gtCanSkipCovariantStoreCheck" to handle "ldelema"s and call it here to
70737073
// skip using the helper in more cases.
70747074
if ((lclTyp != TYP_REF) || ((prefixFlags & PREFIX_READONLY) != 0))
70757075
{
@@ -7208,7 +7208,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
72087208
if (opts.OptimizationEnabled())
72097209
{
72107210
// Is this a case where we can skip the covariant store check?
7211-
if (impCanSkipCovariantStoreCheck(value, array))
7211+
if (gtCanSkipCovariantStoreCheck(value, array))
72127212
{
72137213
lclTyp = TYP_REF;
72147214
goto ARR_ST;
@@ -13898,116 +13898,3 @@ methodPointerInfo* Compiler::impAllocateMethodPointerInfo(const CORINFO_RESOLVED
1389813898
memory->m_tokenConstraint = tokenConstrained;
1389913899
return memory;
1390013900
}
13901-
13902-
//------------------------------------------------------------------------
13903-
// impCanSkipCovariantStoreCheck: see if storing a ref type value to an array
13904-
// can skip the array store covariance check.
13905-
//
13906-
// Arguments:
13907-
// value -- tree producing the value to store
13908-
// array -- tree representing the array to store to
13909-
//
13910-
// Returns:
13911-
// true if the store does not require a covariance check.
13912-
//
13913-
bool Compiler::impCanSkipCovariantStoreCheck(GenTree* value, GenTree* array)
13914-
{
13915-
// We should only call this when optimizing.
13916-
assert(opts.OptimizationEnabled());
13917-
13918-
// Check for store to same array, ie. arrLcl[i] = arrLcl[j]
13919-
if (value->OperIs(GT_IND) && value->AsIndir()->Addr()->OperIs(GT_INDEX_ADDR) && array->OperIs(GT_LCL_VAR))
13920-
{
13921-
GenTree* valueArray = value->AsIndir()->Addr()->AsIndexAddr()->Arr();
13922-
if (valueArray->OperIs(GT_LCL_VAR))
13923-
{
13924-
unsigned valueArrayLcl = valueArray->AsLclVar()->GetLclNum();
13925-
unsigned arrayLcl = array->AsLclVar()->GetLclNum();
13926-
if ((valueArrayLcl == arrayLcl) && !lvaGetDesc(arrayLcl)->IsAddressExposed())
13927-
{
13928-
JITDUMP("\nstelem of ref from same array: skipping covariant store check\n");
13929-
return true;
13930-
}
13931-
}
13932-
}
13933-
13934-
// Check for store of NULL.
13935-
if (value->OperIs(GT_CNS_INT))
13936-
{
13937-
assert(value->gtType == TYP_REF);
13938-
if (value->AsIntCon()->gtIconVal == 0)
13939-
{
13940-
JITDUMP("\nstelem of null: skipping covariant store check\n");
13941-
return true;
13942-
}
13943-
// Non-0 const refs can only occur with frozen objects
13944-
assert(value->IsIconHandle(GTF_ICON_OBJ_HDL));
13945-
assert(doesMethodHaveFrozenObjects() ||
13946-
(compIsForInlining() && impInlineInfo->InlinerCompiler->doesMethodHaveFrozenObjects()));
13947-
}
13948-
13949-
// Try and get a class handle for the array
13950-
if (value->gtType != TYP_REF)
13951-
{
13952-
return false;
13953-
}
13954-
13955-
bool arrayIsExact = false;
13956-
bool arrayIsNonNull = false;
13957-
CORINFO_CLASS_HANDLE arrayHandle = gtGetClassHandle(array, &arrayIsExact, &arrayIsNonNull);
13958-
13959-
if (arrayHandle == NO_CLASS_HANDLE)
13960-
{
13961-
return false;
13962-
}
13963-
13964-
// There are some methods in corelib where we're storing to an array but the IL
13965-
// doesn't reflect this (see SZArrayHelper). Avoid.
13966-
DWORD attribs = info.compCompHnd->getClassAttribs(arrayHandle);
13967-
if ((attribs & CORINFO_FLG_ARRAY) == 0)
13968-
{
13969-
return false;
13970-
}
13971-
13972-
CORINFO_CLASS_HANDLE arrayElementHandle = nullptr;
13973-
CorInfoType arrayElemType = info.compCompHnd->getChildType(arrayHandle, &arrayElementHandle);
13974-
13975-
// Verify array type handle is really an array of ref type
13976-
assert(arrayElemType == CORINFO_TYPE_CLASS);
13977-
13978-
// Check for exactly object[]
13979-
if (arrayIsExact && (arrayElementHandle == impGetObjectClass()))
13980-
{
13981-
JITDUMP("\nstelem to (exact) object[]: skipping covariant store check\n");
13982-
return true;
13983-
}
13984-
13985-
const bool arrayTypeIsSealed = info.compCompHnd->isExactType(arrayElementHandle);
13986-
13987-
if ((!arrayIsExact && !arrayTypeIsSealed) || (arrayElementHandle == NO_CLASS_HANDLE))
13988-
{
13989-
// Bail out if we don't know array's exact type
13990-
return false;
13991-
}
13992-
13993-
bool valueIsExact = false;
13994-
bool valueIsNonNull = false;
13995-
CORINFO_CLASS_HANDLE valueHandle = gtGetClassHandle(value, &valueIsExact, &valueIsNonNull);
13996-
13997-
// Array's type is sealed and equals to value's type
13998-
if (arrayTypeIsSealed && (valueHandle == arrayElementHandle))
13999-
{
14000-
JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n");
14001-
return true;
14002-
}
14003-
14004-
// Array's type is not sealed but we know its exact type
14005-
if (arrayIsExact && (valueHandle != NO_CLASS_HANDLE) &&
14006-
(info.compCompHnd->compareTypesForCast(valueHandle, arrayElementHandle) == TypeCompareState::Must))
14007-
{
14008-
JITDUMP("\nstelem to T[] with T exact: skipping covariant store check\n");
14009-
return true;
14010-
}
14011-
14012-
return false;
14013-
}

src/coreclr/jit/importercalls.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2908,12 +2908,9 @@ GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig)
29082908
return nullptr;
29092909
}
29102910

2911-
CORINFO_CLASS_HANDLE fieldOwnerHnd = info.compCompHnd->getFieldClass(fieldToken);
2912-
29132911
CORINFO_CLASS_HANDLE fieldClsHnd;
2914-
var_types fieldElementType =
2915-
JITtype2varType(info.compCompHnd->getFieldType(fieldToken, &fieldClsHnd, fieldOwnerHnd));
2916-
unsigned totalFieldSize;
2912+
var_types fieldElementType = JITtype2varType(info.compCompHnd->getFieldType(fieldToken, &fieldClsHnd));
2913+
unsigned totalFieldSize;
29172914

29182915
// Most static initialization data fields are of some structure, but it is possible for them to be of various
29192916
// primitive types as well

src/coreclr/jit/morph.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7088,18 +7088,15 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
70887088

70897089
// Morph stelem.ref helper call to store a null value, into a store into an array without the helper.
70907090
// This needs to be done after the arguments are morphed to ensure constant propagation has already taken place.
7091-
if (opts.OptimizationEnabled() && call->IsHelperCall() &&
7092-
(call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST)))
7091+
if (opts.OptimizationEnabled() && call->IsHelperCall(this, CORINFO_HELP_ARRADDR_ST))
70937092
{
70947093
assert(call->gtArgs.CountArgs() == 3);
7094+
GenTree* arr = call->gtArgs.GetArgByIndex(0)->GetNode();
7095+
GenTree* index = call->gtArgs.GetArgByIndex(1)->GetNode();
70957096
GenTree* value = call->gtArgs.GetArgByIndex(2)->GetNode();
7096-
if (value->IsIntegralConst(0))
7097-
{
7098-
assert(value->OperGet() == GT_CNS_INT);
7099-
7100-
GenTree* arr = call->gtArgs.GetArgByIndex(0)->GetNode();
7101-
GenTree* index = call->gtArgs.GetArgByIndex(1)->GetNode();
71027097

7098+
if (gtCanSkipCovariantStoreCheck(value, arr))
7099+
{
71037100
// Either or both of the array and index arguments may have been spilled to temps by `fgMorphArgs`. Copy
71047101
// the spill trees as well if necessary.
71057102
GenTree* argSetup = nullptr;

0 commit comments

Comments
 (0)