Skip to content

Commit cb001f3

Browse files
authored
JIT: fix retyping of some storeinds during object stack allocation (#118251)
This PR fixes an issue that comes up in the LogicArray benchmark where the JIT misses cloning a key loop. In that benchmark there is a struct local `Workspace` that contains an array reference, and the JIT is now able to prove the array does not escape. In the mainline code the field is retyped to TYP_BYREF but the store of the array value is typed as TYP_I_IMPL. This mixed typing of this field hinders physical promotion, which in turn induces morph to create a comma-defined temp for the array references, which in turn creates an in-loop assignment that blocks loop cloning. In general we may sometimes store a non-GC type in a TYP_BYREF field of a local struct, if say that field can hold both GC refs and stack allocated object refs, or if (as here) we are in an OSR method where we have to assume that object allocations might come from the Tier0 code. If so we should still try and consistently type the store as TYP_BYREF, so as to not confuse physical promotion. During the escape analysis phase we built up a store map to track how each store is modeled, so we can consult this during retyping to figure out which type to use for the store. Fixes #114999.
1 parent adeda17 commit cb001f3

File tree

1 file changed

+22
-3
lines changed

1 file changed

+22
-3
lines changed

src/coreclr/jit/objectalloc.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2427,11 +2427,30 @@ void ObjectAllocator::UpdateAncestorTypes(
24272427
{
24282428
assert(tree == parent->AsIndir()->Data());
24292429

2430-
// If we are storing to a GC struct field, we may need to retype the store
2431-
//
24322430
if (varTypeIsGC(parent->TypeGet()))
24332431
{
2434-
parent->ChangeType(newType);
2432+
// We are storing a non-struct value, possibly to a struct field.
2433+
//
2434+
// See if we can figure out the field type from the address.
2435+
// It might be TYP_BYREF even if the value we are storing is TYP_I_IMPL.
2436+
//
2437+
StoreInfo* const info = m_StoreAddressToIndexMap.LookupPointer(parent);
2438+
bool wasRetyped = false;
2439+
2440+
if (info != nullptr)
2441+
{
2442+
unsigned const addressIndex = info->m_index;
2443+
if ((addressIndex != BAD_VAR_NUM) && !DoesIndexPointToStack(addressIndex))
2444+
{
2445+
parent->ChangeType(TYP_BYREF);
2446+
wasRetyped = true;
2447+
}
2448+
}
2449+
2450+
if (!wasRetyped)
2451+
{
2452+
parent->ChangeType(newType);
2453+
}
24352454
}
24362455
else if (retypeFields && parent->OperIs(GT_STORE_BLK))
24372456
{

0 commit comments

Comments
 (0)