From baff212ff8f4d5336f0533cbcede891f44208287 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 15:27:06 -0700 Subject: [PATCH 1/3] JIT: stop tracking TYP_I_IMPL locals in escape analysis Abstractly a non-GC local can't cause escape. On 32 bit platforms tracking connections to `TYP_I_IMPL` leads to confusion once we start struct field tracking, as we logically "overlay" GC and non-GC fields. Contributes to #104936 --- src/coreclr/jit/objectalloc.cpp | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index a86197994e377d..91b9d17d7ea369 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -71,7 +71,7 @@ ObjectAllocator::ObjectAllocator(Compiler* comp) // bool ObjectAllocator::IsTrackedType(var_types type) { - const bool isTrackableScalar = (type == TYP_REF) || (genActualType(type) == TYP_I_IMPL) || (type == TYP_BYREF); + const bool isTrackableScalar = (type == TYP_REF) || (type == TYP_BYREF); return isTrackableScalar; } @@ -1579,12 +1579,19 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent switch (parent->OperGet()) { - // Update the connection graph if we are storing to a local. - // For all other stores we mark the local as escaping. case GT_STORE_LCL_VAR: { - // Add an edge to the connection graph. const unsigned int dstLclNum = parent->AsLclVar()->GetLclNum(); + + // If we're not tracking stores to this local, the value + // does not escape. + if (!IsTrackedLocal(dstLclNum)) + { + canLclVarEscapeViaParentStack = false; + break; + } + + // Add an edge to the connection graph. const unsigned int srcLclNum = lclNum; AddConnGraphEdge(dstLclNum, srcLclNum); @@ -1626,13 +1633,25 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_COLON: case GT_QMARK: case GT_ADD: - case GT_SUB: case GT_FIELD_ADDR: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; break; + case GT_SUB: + // Sub of two GC refs is no longer a GC ref. + if (!parent->TypeIs(TYP_BYREF, TYP_REF)) + { + canLclVarEscapeViaParentStack = false; + break; + } + + // Check whether the local escapes higher up + ++parentIndex; + keepChecking = true; + break; + case GT_BOX: isCopy = wasCopy; ++parentIndex; From 6f462944a0d4441001f408da8958f35682eff3fc Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 18:54:29 -0700 Subject: [PATCH 2/3] fix GT_SUB type propagation --- src/coreclr/jit/objectalloc.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 91b9d17d7ea369..7e912972425a5b 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1806,7 +1806,6 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p FALLTHROUGH; case GT_QMARK: case GT_ADD: - case GT_SUB: case GT_FIELD_ADDR: case GT_INDEX_ADDR: case GT_BOX: @@ -1818,6 +1817,15 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p keepChecking = true; break; + case GT_SUB: + if (parent->TypeGet() != newType) + { + parent->ChangeType(newType); + ++parentIndex; + keepChecking = true; + } + break; + case GT_COLON: { GenTree* const lhs = parent->AsOp()->gtGetOp1(); From 0ff879e134681380e74798b47a73a117cf0314fe Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 2 Apr 2025 09:58:24 -0700 Subject: [PATCH 3/3] add comments/asserts to sub retyping --- src/coreclr/jit/objectalloc.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 7e912972425a5b..987a81146667c8 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1818,13 +1818,31 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p break; case GT_SUB: - if (parent->TypeGet() != newType) + { + // Parent type can be TYP_I_IMPL, TYP_BYREF. + // But not TYP_REF. + // + var_types parentType = parent->TypeGet(); + assert(parentType != TYP_REF); + + // New type can be TYP_I_IMPL, TYP_BYREF. + // But TYP_BYREF only if parent is also + // + if (parentType != newType) { + // We must be retyping TYP_BYREF to TYP_I_IMPL. + // + assert(newType == TYP_I_IMPL); + assert(parentType == TYP_BYREF); parent->ChangeType(newType); + + // Propgate that upwards. + // ++parentIndex; keepChecking = true; } break; + } case GT_COLON: {