From 5dbbc81f6cd1817fd82ea87da5f7b5648b710238 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 13 Feb 2025 15:27:06 -0800 Subject: [PATCH 1/3] JIT: enhance escape analysis to understand `Span` capture Implement a very simplistic "field sensitive" analysis for `Span` where we model the span as simply its byref field. If the span is only consumed locally, and the array does not otherwise escape, then the array does not escape. This is a subset of #112543 that does not try and reason interprocedurally. Contributes to #104936 / #108913 --- src/coreclr/jit/compiler.h | 7 + src/coreclr/jit/objectalloc.cpp | 123 ++++++++++++---- .../ObjectStackAllocationTests.cs | 137 ++++++++++++++++++ 3 files changed, 238 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 43b23d1372a84c..0f8cffd2ec3fec 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1048,6 +1048,13 @@ class LclVarDsc m_layout = layout; } + // Change the layout to one that may not be compatible. + void ChangeLayout(ClassLayout* layout) + { + assert(varTypeIsStruct(lvType)); + m_layout = layout; + } + // Grow the size of a block layout local. void GrowBlockLayout(ClassLayout* layout) { diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 751b11fc8898ee..1de499fffd7fe7 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -233,7 +233,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() lclEscapes = false; m_allocator->CheckForGuardedAllocationOrCopy(m_block, m_stmt, use, lclNum); } - else if (tree->OperIs(GT_LCL_VAR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL)) + else if (tree->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) { assert(tree == m_ancestors.Top()); if (!m_allocator->CanLclVarEscapeViaParentStack(&m_ancestors, lclNum, m_block)) @@ -265,7 +265,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() { var_types type = comp->lvaTable[lclNum].TypeGet(); - if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF) + if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF || type == TYP_STRUCT) { m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::MakeEmpty(&m_bitVecTraits); @@ -1023,10 +1023,13 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent assert(parentStack != nullptr); int parentIndex = 1; - bool keepChecking = true; - bool canLclVarEscapeViaParentStack = true; - bool isCopy = true; - bool isEnumeratorLocal = comp->lvaGetDesc(lclNum)->lvIsEnumerator; + LclVarDsc* const lclDsc = comp->lvaGetDesc(lclNum); + + bool keepChecking = true; + bool canLclVarEscapeViaParentStack = true; + bool isCopy = true; + bool const isEnumeratorLocal = lclDsc->lvIsEnumerator; + bool const isSpanLocal = lclDsc->IsSpan(); while (keepChecking) { @@ -1093,11 +1096,21 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_ADD: case GT_SUB: case GT_FIELD_ADDR: - // Check whether the local escapes via its grandparent. + // Check whether the local escapes higher up ++parentIndex; keepChecking = true; break; + case GT_LCL_ADDR: + if (isSpanLocal) + { + // Check whether the local escapes higher up + ++parentIndex; + keepChecking = true; + JITDUMP("... i'm good thanks\n"); + } + break; + case GT_BOX: isCopy = wasCopy; ++parentIndex; @@ -1119,11 +1132,43 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_STOREIND: case GT_STORE_BLK: case GT_BLK: - if (tree != parent->AsIndir()->Addr()) + { + GenTree* const addr = parent->AsIndir()->Addr(); + if (tree != addr) { - // TODO-ObjectStackAllocation: track stores to fields. + JITDUMP("... tree != addr\n"); + + // Is this an array element address store to (the pointer) field of a span? + // (note we can't yet handle cases where a span captures an object) + // + if (parent->OperIs(GT_STOREIND) && addr->OperIs(GT_FIELD_ADDR) && tree->OperIs(GT_INDEX_ADDR)) + { + // Todo: mark the span pointer field addr like we mark IsSpanLength? + // (for now we don't worry which field we store to) + // + GenTree* const base = addr->AsOp()->gtGetOp1(); + + if (base->OperIs(GT_LCL_ADDR)) + { + unsigned const dstLclNum = base->AsLclVarCommon()->GetLclNum(); + LclVarDsc* const dstLclDsc = comp->lvaGetDesc(dstLclNum); + + if (dstLclDsc->IsSpan()) + { + JITDUMP("... span ptr store\n"); + // Add an edge to the connection graph. + AddConnGraphEdge(dstLclNum, lclNum); + canLclVarEscapeViaParentStack = false; + } + } + } break; } + else + { + JITDUMP("... tree == addr\n"); + } + } FALLTHROUGH; case GT_IND: // Address of the field/ind is not taken so the local doesn't escape. @@ -1132,20 +1177,20 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_CALL: { - GenTreeCall* const asCall = parent->AsCall(); + GenTreeCall* const call = parent->AsCall(); - if (asCall->IsHelperCall()) + if (call->IsHelperCall()) { canLclVarEscapeViaParentStack = - !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(asCall->gtCallMethHnd)); + !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } - else if (asCall->IsSpecialIntrinsic()) + else if (call->IsSpecialIntrinsic()) { // Some known special intrinsics don't escape. At this moment, only the ones accepting byrefs // are supported. In order to support more intrinsics accepting objects, we need extra work // on the VM side which is not ready for that yet. // - switch (comp->lookupNamedIntrinsic(asCall->gtCallMethHnd)) + switch (comp->lookupNamedIntrinsic(call->gtCallMethHnd)) { case NI_System_SpanHelpers_ClearWithoutReferences: case NI_System_SpanHelpers_Fill: @@ -1246,6 +1291,7 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_SUB: case GT_FIELD_ADDR: case GT_INDEX_ADDR: + case GT_LCL_ADDR: if (parent->TypeGet() == TYP_REF) { parent->ChangeType(newType); @@ -1283,17 +1329,18 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_STOREIND: case GT_STORE_BLK: case GT_BLK: - assert(tree == parent->AsIndir()->Addr()); - - // The new target could be *not* on the heap. - parent->gtFlags &= ~GTF_IND_TGT_HEAP; - - if (newType != TYP_BYREF) + if (tree == parent->AsIndir()->Addr()) { - // This indicates that a write barrier is not needed when writing - // to this field/indirection since the address is not pointing to the heap. - // It's either null or points to inside a stack-allocated object. - parent->gtFlags |= GTF_IND_TGT_NOT_HEAP; + // The new target could be *not* on the heap. + parent->gtFlags &= ~GTF_IND_TGT_HEAP; + + if (newType != TYP_BYREF) + { + // This indicates that a write barrier is not needed when writing + // to this field/indirection since the address is not pointing to the heap. + // It's either null or points to inside a stack-allocated object. + parent->gtFlags |= GTF_IND_TGT_NOT_HEAP; + } } break; @@ -1354,10 +1401,7 @@ void ObjectAllocator::RewriteUses() if ((lclNum < BitVecTraits::GetSize(&m_allocator->m_bitVecTraits)) && m_allocator->MayLclVarPointToStack(lclNum)) { - // Analysis does not handle indirect access to pointer locals. - assert(tree->OperIsScalarLocal()); - - var_types newType; + var_types newType = TYP_UNDEF; if (m_allocator->m_HeapLocalToStackLocalMap.TryGetValue(lclNum, &newLclNum)) { assert(tree->OperIs(GT_LCL_VAR)); // Must be a use. @@ -1374,12 +1418,33 @@ void ObjectAllocator::RewriteUses() } } - if (lclVarDsc->lvType != newType) + // For local structs, retype the GC fields. + // + if (lclVarDsc->lvType == TYP_STRUCT) + { + // We should only see spans here. + // + assert(lclVarDsc->IsSpan()); + ClassLayout* const layout = lclVarDsc->GetLayout(); + ClassLayout* newLayout = nullptr; + + if (newType == TYP_I_IMPL) + { + // No GC refs remain, so now a block layout + newLayout = m_compiler->typGetBlkLayout(layout->GetSize()); + JITDUMP("Changing layout of span V%02u to block\n", lclNum); + lclVarDsc->ChangeLayout(newLayout); + } + } + // For locals, retype the local + // + else if (lclVarDsc->lvType != newType) { JITDUMP("Changing the type of V%02u from %s to %s\n", lclNum, varTypeName(lclVarDsc->lvType), varTypeName(newType)); lclVarDsc->lvType = newType; } + m_allocator->UpdateAncestorTypes(tree, &m_ancestors, newType); if (newLclNum != BAD_VAR_NUM) diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 6065b9cb3ae7a5..3b0e95acfa8d08 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -88,6 +88,13 @@ enum AllocationKind Undefined } + ref struct SpanKeeper + { + public int a; + public Span span; + public int b; + } + public class Tests { static volatile int f1 = 5; @@ -162,6 +169,12 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); + // Span captures + CallTestAndVerifyAllocation(SpanCaptureArray1, 41, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanCaptureArray2, 25, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanCaptureArrayT, 37, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanCaptureArrayT, 37, expectedAllocationKind); + // The remaining tests currently never allocate on the stack if (expectedAllocationKind == AllocationKind.Stack) { expectedAllocationKind = AllocationKind.Heap; @@ -176,6 +189,11 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsEscape, 42, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayWithGCElementsEscape, 42, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArrayArg, 42, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArrayArgCopy, 42, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArrayOutParam, 22, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArrayOutParam2, 22, expectedAllocationKind); + // This test calls CORINFO_HELP_OVERFLOW CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); @@ -403,6 +421,106 @@ static int AllocateArrayT() return array.Length + 42; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArray1() + { + Span span = new int[100]; + span[10] = 41; + return span[10] + span[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArray2() => SpanCaptureArray2Helper(null); + + static int SpanCaptureArray2Helper(int[]? x) + { + Span span = x ?? new int[100]; + span[10] = 25; + return span[10] + span[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArray3() + { + Span span = new int[128]; + span[10] = 100; + Span x = span; + return x[10] + span[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArrayT() + { + Span span = new T[37]; + return span.Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArrayArg() + { + Span y = new int[100]; + Use(y); + TrashStack(); + return y[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArrayArgCopy() + { + Span x = new int[100]; + Span y = x; + Use(y); + TrashStack(); + return y[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArrayReturn() => SpanEscapeArrayReturnHelper()[10]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static Span SpanEscapeArrayReturnHelper() + { + Span x = new int[44]; + Span y = x; + x[10] = 99; + return y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArrayOutParam() + { + Span x; + SpanEscapeArrayOutParamHelper(out x); + TrashStack(); + return x[10]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void SpanEscapeArrayOutParamHelper(out Span a) + { + a = new Span(new int[44]); + a[10] = 22; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArrayOutParam2() + { + SpanKeeper y; + SpanEscapeArrayOutParam2Helper(out y); + TrashStack(); + return y.span[10]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void SpanEscapeArrayOutParam2Helper(out SpanKeeper b) + { + int[] x = new int[44]; + x[10] = 22; + b.span = x; + b.a = 1; + b.b = 2; + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsEscape() { @@ -467,6 +585,15 @@ static void Use(ref string s) s = "42"; } + [MethodImpl(MethodImplOptions.NoInlining)] + static void Use(Span span) + { + span[42] = 42; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Span Identity(Span x) => x; + [MethodImpl(MethodImplOptions.NoInlining)] private static void ZeroAllocTest() { @@ -526,6 +653,16 @@ private static void Consume(T _) { } + [MethodImpl(MethodImplOptions.NoInlining)] + static void TrashStack() + { + Span span = stackalloc int[128]; + for (int i = 0; i < span.Length; i++) + { + span[i] = -1; + } + } + private record class MyRecord(int A, long B, Guid C); } } From 5fcfe24a33c4daa77b590f217534395738a5d79c Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 4 Mar 2025 18:46:57 -0800 Subject: [PATCH 2/3] Generalize to handling all GC structs. We conflate a struct with its fields, since we know the struct itself can't escape. We rely on accesses not being able to walk from one local to another. We try to be properly conservative around byrefs. --- src/coreclr/jit/compiler.h | 6 +- src/coreclr/jit/jitconfigvalues.h | 1 + src/coreclr/jit/layout.cpp | 137 ++++++++++-- src/coreclr/jit/layout.h | 5 +- src/coreclr/jit/objectalloc.cpp | 199 +++++++++++++---- src/coreclr/jit/objectalloc.h | 10 +- .../ObjectStackAllocationTests.cs | 200 ++++++++++++++++-- 7 files changed, 482 insertions(+), 76 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f8cffd2ec3fec..0e179667fb6ae8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -10967,8 +10967,10 @@ class Compiler unsigned typGetBlkLayoutNum(unsigned blockSize); // Get the layout for the specified array of known length ClassLayout* typGetArrayLayout(CORINFO_CLASS_HANDLE classHandle, unsigned length); - // Get the number of a layout for the specified array of known length - unsigned typGetArrayLayoutNum(CORINFO_CLASS_HANDLE classHandle, unsigned length); + // Get a layout like an existing layout, with all gc refs removed + ClassLayout* typGetNonGCLayout(ClassLayout* existingLayout); + // Get a layout like an existing layout, with all gc refs changed to byrefs + ClassLayout* typGetByrefLayout(ClassLayout* existingLayout); var_types TypeHandleToVarType(CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr); var_types TypeHandleToVarType(CorInfoType jitType, CORINFO_CLASS_HANDLE handle, ClassLayout** pLayout = nullptr); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 5fe7439d2bc658..cb24f0d2ed1409 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -675,6 +675,7 @@ RELEASE_CONFIG_INTEGER(JitObjectStackAllocationConditionalEscape, "JitObjectStac CONFIG_STRING(JitObjectStackAllocationConditionalEscapeRange, "JitObjectStackAllocationConditionalEscapeRange") RELEASE_CONFIG_INTEGER(JitObjectStackAllocationArray, "JitObjectStackAllocationArray", 1) RELEASE_CONFIG_INTEGER(JitObjectStackAllocationSize, "JitObjectStackAllocationSize", 528) +RELEASE_CONFIG_INTEGER(JitObjectStackAllocationTrackFields, "JitObjectStackAllocationTrackFields", 1) RELEASE_CONFIG_INTEGER(JitEECallTimingInfo, "JitEECallTimingInfo", 0) diff --git a/src/coreclr/jit/layout.cpp b/src/coreclr/jit/layout.cpp index 1e4613e5e2eb90..5e603a2be38345 100644 --- a/src/coreclr/jit/layout.cpp +++ b/src/coreclr/jit/layout.cpp @@ -414,18 +414,80 @@ ClassLayout* Compiler::typGetBlkLayout(unsigned blockSize) return typGetCustomLayout(ClassLayoutBuilder(this, blockSize)); } -unsigned Compiler::typGetArrayLayoutNum(CORINFO_CLASS_HANDLE classHandle, unsigned length) +ClassLayout* Compiler::typGetArrayLayout(CORINFO_CLASS_HANDLE classHandle, unsigned length) { ClassLayoutBuilder b = ClassLayoutBuilder::BuildArray(this, classHandle, length); - return typGetCustomLayoutNum(b); + return typGetCustomLayout(b); } -ClassLayout* Compiler::typGetArrayLayout(CORINFO_CLASS_HANDLE classHandle, unsigned length) +ClassLayout* Compiler::typGetNonGCLayout(ClassLayout* layout) { - ClassLayoutBuilder b = ClassLayoutBuilder::BuildArray(this, classHandle, length); + assert(layout->HasGCPtr()); + ClassLayoutBuilder b(this, layout->GetSize()); + b.CopyPaddingFrom(0, layout); + +#ifdef DEBUG + b.CopyNameFrom(layout, "[nongc] "); +#endif + return typGetCustomLayout(b); } +ClassLayout* Compiler::typGetByrefLayout(ClassLayout* layout) +{ + assert(layout->HasGCPtr()); + ClassLayoutBuilder b(this, layout->GetSize()); + b.CopyPaddingFrom(0, layout); + b.CopyGCInfoFromMakeByref(0, layout); + +#ifdef DEBUG + b.CopyNameFrom(layout, "[byref] "); +#endif + + return typGetCustomLayout(b); +} + +#ifdef DEBUG +//------------------------------------------------------------------------ +// CopyNameFrom: Copy layout names, with optional prefix. +// +// Parameters: +// layout - layout to copy from +// prefix - prefix to add (or nullptr) +// +void ClassLayoutBuilder::CopyNameFrom(ClassLayout* layout, const char* prefix) +{ + const char* layoutName = layout->GetClassName(); + const char* layoutShortName = layout->GetShortClassName(); + + if (prefix != nullptr) + { + char* newName = nullptr; + char* newShortName = nullptr; + + if (layoutName != nullptr) + { + size_t len = strlen(prefix) + strlen(layoutName) + 1; + newName = m_compiler->getAllocator(CMK_DebugOnly).allocate(len); + sprintf_s(newName, len, "%s%s", prefix, layoutShortName); + } + + if (layoutShortName != nullptr) + { + size_t len = strlen(prefix) + strlen(layoutName) + 1; + newShortName = m_compiler->getAllocator(CMK_DebugOnly).allocate(len); + sprintf_s(newShortName, len, "%s%s", prefix, layoutShortName); + } + + SetName(newName, newShortName); + } + else + { + SetName(layoutName, layoutShortName); + } +} +#endif // DEBUG + //------------------------------------------------------------------------ // Create: Create a ClassLayout from an EE side class handle. // @@ -646,8 +708,8 @@ const SegmentList& ClassLayout::GetNonPadding(Compiler* comp) // AreCompatible: check if 2 layouts are the same for copying. // // Arguments: -// layout1 - the first layout; -// layout2 - the second layout. +// layout1 - the first layout (copy destination) +// layout2 - the second layout (copy source) // // Return value: // true if compatible, false otherwise. @@ -706,8 +768,8 @@ bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* l return true; } - assert(clsHnd1 != NO_CLASS_HANDLE); - assert(clsHnd2 != NO_CLASS_HANDLE); + // assert(clsHnd1 != NO_CLASS_HANDLE); + // assert(clsHnd2 != NO_CLASS_HANDLE); assert(layout1->HasGCPtr() && layout2->HasGCPtr()); if (layout1->GetGCPtrCount() != layout2->GetGCPtrCount()) @@ -722,6 +784,13 @@ bool ClassLayout::AreCompatible(const ClassLayout* layout1, const ClassLayout* l { if (layout1->GetGCPtrType(i) != layout2->GetGCPtrType(i)) { + // Allow a source GC_REF to match a custom GC_BYREF + // + if ((layout2->GetGCPtrType(i) == TYP_REF) && (layout1->GetGCPtrType(i) == TYP_BYREF) && + layout1->IsCustomLayout()) + { + continue; + } return false; } } @@ -791,7 +860,7 @@ ClassLayoutBuilder ClassLayoutBuilder::BuildArray(Compiler* compiler, CORINFO_CL unsigned offset = OFFSETOF__CORINFO_Array__data; for (unsigned i = 0; i < length; i++) { - builder.CopyInfoFrom(offset, elementLayout, /* copy padding */ false); + builder.CopyGCInfoFrom(offset, elementLayout); offset += elementSize; } } @@ -896,14 +965,13 @@ void ClassLayoutBuilder::SetGCPtrType(unsigned slot, var_types type) } //------------------------------------------------------------------------ -// CopyInfoFrom: Copy GC pointers and padding information from another layout. +// CopyInfoGCFrom: Copy GC pointers from another layout. // // Arguments: // offset - Offset in this builder to start copy information into. // layout - Layout to get information from. -// copyPadding - Whether padding info should also be copied from the layout. // -void ClassLayoutBuilder::CopyInfoFrom(unsigned offset, ClassLayout* layout, bool copyPadding) +void ClassLayoutBuilder::CopyGCInfoFrom(unsigned offset, ClassLayout* layout) { assert(offset + layout->GetSize() <= m_size); @@ -916,18 +984,53 @@ void ClassLayoutBuilder::CopyInfoFrom(unsigned offset, ClassLayout* layout, bool SetGCPtr(startSlot + slot, layout->GetGCPtr(slot)); } } +} - if (copyPadding) - { - AddPadding(SegmentList::Segment(offset, offset + layout->GetSize())); +//------------------------------------------------------------------------ +// CopyInfoGCFromMakeByref: Copy GC pointers from another layout,and change +// all gc references to be TYP_BYREF (TYPE_GC_BYREF) +// +// Arguments: +// offset - Offset in this builder to start copy information into. +// layout - Layout to get information from. +// +void ClassLayoutBuilder::CopyGCInfoFromMakeByref(unsigned offset, ClassLayout* layout) +{ + assert(offset + layout->GetSize() <= m_size); - for (const SegmentList::Segment& nonPadding : layout->GetNonPadding(m_compiler)) + if (layout->GetGCPtrCount() > 0) + { + assert(offset % TARGET_POINTER_SIZE == 0); + unsigned startSlot = offset / TARGET_POINTER_SIZE; + for (unsigned slot = 0; slot < layout->GetSlotCount(); slot++) { - RemovePadding(SegmentList::Segment(offset + nonPadding.Start, offset + nonPadding.End)); + CorInfoGCType gcType = layout->GetGCPtr(slot); + if (gcType == TYPE_GC_REF) + { + gcType = TYPE_GC_BYREF; + } + SetGCPtr(startSlot + slot, gcType); } } } +//------------------------------------------------------------------------ +// CopyInfoPaddingFrom: Copy padding from another layout. +// +// Arguments: +// offset - Offset in this builder to start copy information into. +// layout - Layout to get information from. +// +void ClassLayoutBuilder::CopyPaddingFrom(unsigned offset, ClassLayout* layout) +{ + AddPadding(SegmentList::Segment(offset, offset + layout->GetSize())); + + for (const SegmentList::Segment& nonPadding : layout->GetNonPadding(m_compiler)) + { + RemovePadding(SegmentList::Segment(offset + nonPadding.Start, offset + nonPadding.End)); + } +} + //------------------------------------------------------------------------ // GetOrCreateNonPadding: Get the non padding segment list, or create it if it // does not exist. diff --git a/src/coreclr/jit/layout.h b/src/coreclr/jit/layout.h index 03708a007052d4..fff07717751477 100644 --- a/src/coreclr/jit/layout.h +++ b/src/coreclr/jit/layout.h @@ -34,12 +34,15 @@ class ClassLayoutBuilder ClassLayoutBuilder(Compiler* compiler, unsigned size); void SetGCPtrType(unsigned slot, var_types type); - void CopyInfoFrom(unsigned offset, ClassLayout* layout, bool copyPadding); + void CopyGCInfoFrom(unsigned offset, ClassLayout* layout); + void CopyGCInfoFromMakeByref(unsigned offset, ClassLayout* layout); + void CopyPaddingFrom(unsigned offset, ClassLayout* layout); void AddPadding(const SegmentList::Segment& padding); void RemovePadding(const SegmentList::Segment& nonPadding); #ifdef DEBUG void SetName(const char* name, const char* shortName); + void CopyNameFrom(ClassLayout* layout, const char* prefix); #endif static ClassLayoutBuilder BuildArray(Compiler* compiler, CORINFO_CLASS_HANDLE arrayType, unsigned length); diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 1de499fffd7fe7..3fefc3395b93f2 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -143,10 +143,43 @@ void ObjectAllocator::AddConnGraphEdge(unsigned int sourceLclNum, unsigned int t BitVecOps::AddElemD(&m_bitVecTraits, m_ConnGraphAdjacencyMatrix[sourceLclNum], targetLclNum); } +//------------------------------------------------------------------------ +// IsTrackedType: Check if this type is one we will track +// +// Arguments: +// type - type of interest +// +// Returns: +// true if so +// +bool ObjectAllocator::IsTrackedType(var_types type) +{ + const bool isTrackableScalar = (type == TYP_REF) || (genActualType(type) == TYP_I_IMPL) || (type == TYP_BYREF); + const bool isTrackableStruct = (type == TYP_STRUCT) && m_trackFields; + + return isTrackableScalar || isTrackableStruct; +} + +//------------------------------------------------------------------------ +// IsTrackedLocal: Check if this local is one we will track +// +// Arguments: +// lclNum - local of interest +// +// Returns: +// true if so +// +bool ObjectAllocator::IsTrackedLocal(unsigned lclNum) +{ + assert(lclNum < m_initialLocalCount); + var_types type = comp->lvaTable[lclNum].TypeGet(); + return IsTrackedType(type); +} + //------------------------------------------------------------------------ // DoAnalysis: Walk over basic blocks of the method and detect all local // variables that can be allocated on the stack. - +// void ObjectAllocator::DoAnalysis() { assert(m_IsObjectStackAllocationEnabled); @@ -233,7 +266,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() lclEscapes = false; m_allocator->CheckForGuardedAllocationOrCopy(m_block, m_stmt, use, lclNum); } - else if (tree->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) + else if (tree->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && m_allocator->IsTrackedLocal(lclNum)) { assert(tree == m_ancestors.Top()); if (!m_allocator->CanLclVarEscapeViaParentStack(&m_ancestors, lclNum, m_block)) @@ -263,17 +296,22 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() for (unsigned int lclNum = 0; lclNum < comp->lvaCount; ++lclNum) { - var_types type = comp->lvaTable[lclNum].TypeGet(); - - if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF || type == TYP_STRUCT) + if (IsTrackedLocal(lclNum)) { m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::MakeEmpty(&m_bitVecTraits); + // Pre-classify locals that we know lead to escape + // if (comp->lvaTable[lclNum].IsAddressExposed()) { JITDUMP(" V%02u is address exposed\n", lclNum); MarkLclVarAsEscaping(lclNum); } + else if (lclNum == comp->info.compRetBuffArg) + { + JITDUMP(" V%02u is retbuff\n", lclNum); + MarkLclVarAsEscaping(lclNum); + } } else { @@ -393,13 +431,15 @@ void ObjectAllocator::ComputeStackObjectPointers(BitVecTraits* bitVecTraits) while (changed) { changed = false; - for (unsigned int lclNum = 0; lclNum < comp->lvaCount; ++lclNum) - { - LclVarDsc* lclVarDsc = comp->lvaGetDesc(lclNum); - var_types type = lclVarDsc->TypeGet(); - if (type == TYP_REF || type == TYP_I_IMPL || type == TYP_BYREF) + // Exclude newly allocated locals + // + for (unsigned int lclNum = 0; lclNum < m_initialLocalCount; ++lclNum) + { + if (IsTrackedLocal(lclNum)) { + LclVarDsc* lclVarDsc = comp->lvaGetDesc(lclNum); + if (!MayLclVarPointToStack(lclNum) && !BitVecOps::IsEmptyIntersection(bitVecTraits, m_PossiblyStackPointingPointers, m_ConnGraphAdjacencyMatrix[lclNum])) @@ -414,7 +454,27 @@ void ObjectAllocator::ComputeStackObjectPointers(BitVecTraits* bitVecTraits) { // Check if we know what is assigned to this pointer. unsigned bitCount = BitVecOps::Count(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); - assert(bitCount <= 1); + +#ifdef DEBUG + if (bitCount > 1) + { + // We expect only one edge in the connection graph, from the place this + // local is assigned. But in some odd cases (eg subs of byrefs) we were + // improperly seeing two or more connected GC refs. Fix these by adjusting + // the connection building logic. + // + JITDUMP("Unexpected: single-def V%02u has %u connections:", lclNum, bitCount); + BitVecOps::Iter iter(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); + unsigned bitIndex = 0; + while (iter.NextElem(&bitIndex)) + { + JITDUMP(" V%02u", bitIndex); + } + JITDUMP("\n"); + assert(bitCount <= 1); + } +#endif + if (bitCount == 1) { BitVecOps::Iter iter(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); @@ -1029,7 +1089,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent bool canLclVarEscapeViaParentStack = true; bool isCopy = true; bool const isEnumeratorLocal = lclDsc->lvIsEnumerator; - bool const isSpanLocal = lclDsc->IsSpan(); + int numIndirs = 0; while (keepChecking) { @@ -1052,10 +1112,11 @@ 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. + // Add an edge to the connection graph, if destination is a local we're tracking + // (i.e. a local var or a field of a local var). const unsigned int dstLclNum = parent->AsLclVar()->GetLclNum(); const unsigned int srcLclNum = lclNum; @@ -1080,6 +1141,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_GE: case GT_NULLCHECK: case GT_ARR_LENGTH: + case GT_BOUNDS_CHECK: canLclVarEscapeViaParentStack = false; break; @@ -1094,21 +1156,24 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_COLON: case GT_QMARK: case GT_ADD: - case GT_SUB: case GT_FIELD_ADDR: + case GT_LCL_ADDR: // Check whether the local escapes higher up ++parentIndex; keepChecking = true; break; - case GT_LCL_ADDR: - if (isSpanLocal) + case GT_SUB: + // Sub of two GC refs is no longer a GC ref. + if (!parent->TypeIs(TYP_BYREF, TYP_REF)) { - // Check whether the local escapes higher up - ++parentIndex; - keepChecking = true; - JITDUMP("... i'm good thanks\n"); + canLclVarEscapeViaParentStack = false; + break; } + + // Check whether the local escapes higher up + ++parentIndex; + keepChecking = true; break; case GT_BOX: @@ -1136,44 +1201,85 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent GenTree* const addr = parent->AsIndir()->Addr(); if (tree != addr) { - JITDUMP("... tree != addr\n"); + JITDUMP("... store value\n"); - // Is this an array element address store to (the pointer) field of a span? - // (note we can't yet handle cases where a span captures an object) + // Is this a store to a field of a local struct...? // - if (parent->OperIs(GT_STOREIND) && addr->OperIs(GT_FIELD_ADDR) && tree->OperIs(GT_INDEX_ADDR)) + if (parent->OperIs(GT_STOREIND) && addr->OperIs(GT_FIELD_ADDR, GT_LCL_ADDR)) { - // Todo: mark the span pointer field addr like we mark IsSpanLength? - // (for now we don't worry which field we store to) + // Do we know which local? // - GenTree* const base = addr->AsOp()->gtGetOp1(); + GenTree* const base = addr->OperIs(GT_FIELD_ADDR) ? addr->AsOp()->gtGetOp1() : addr; if (base->OperIs(GT_LCL_ADDR)) { - unsigned const dstLclNum = base->AsLclVarCommon()->GetLclNum(); - LclVarDsc* const dstLclDsc = comp->lvaGetDesc(dstLclNum); + unsigned const dstLclNum = base->AsLclVarCommon()->GetLclNum(); - if (dstLclDsc->IsSpan()) + if (IsTrackedLocal(dstLclNum)) { - JITDUMP("... span ptr store\n"); + JITDUMP("... local [struct] store\n"); // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, lclNum); canLclVarEscapeViaParentStack = false; } + else + { + // Store to untracked local. Assume escape. + } + } + else + { + // Store destination unknown. Assume escape. + // + // TODO: handle more general trees here. + // Since we visit the address subtree first, perhaps we can annotate somehow. } } - break; + else + { + // Likely heap store. Assume escape. + } } else { - JITDUMP("... tree == addr\n"); + canLclVarEscapeViaParentStack = false; + JITDUMP("... store address\n"); } + break; } - FALLTHROUGH; + case GT_IND: - // Address of the field/ind is not taken so the local doesn't escape. + { + GenTree* const addr = parent->AsIndir()->Addr(); + + // For loads from structs we may be tracking the underlying fields. + // + // We don't handle TYP_REF locals (yet), and allowing that requires separating out the object from + // its fields in our tracking. + // + // We treat TYP_BYREF like TYP_STRUCT, though possibly this needs more scrutiny, as byrefs may alias. + // Ditto for TYP_I_IMPL. + // + // We can assume that the local being read is lclNum, since we have walked up to this node from a leaf + // local. + // + // We only track through the first indir. + // + if (m_trackFields && (numIndirs == 0) && varTypeIsGC(parent->TypeGet()) && + (lclDsc->TypeGet() != TYP_REF)) + { + JITDUMP("... local [struct] load\n"); + ++parentIndex; + ++numIndirs; + keepChecking = true; + break; + } + + // Address doesn't refer to anything we track + // canLclVarEscapeViaParentStack = false; break; + } case GT_CALL: { @@ -1349,6 +1455,8 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p break; default: + JITDUMP("UpdateAncestorTypes: unexpected op %s in [%06u]\n", GenTree::OpName(parent->OperGet()), + comp->dspTreeID(parent)); unreached(); } @@ -1422,17 +1530,22 @@ void ObjectAllocator::RewriteUses() // if (lclVarDsc->lvType == TYP_STRUCT) { - // We should only see spans here. - // - assert(lclVarDsc->IsSpan()); ClassLayout* const layout = lclVarDsc->GetLayout(); ClassLayout* newLayout = nullptr; - if (newType == TYP_I_IMPL) + if ((newType == TYP_I_IMPL) && !layout->IsBlockLayout()) + { + // New layout with no gc refs + padding + newLayout = m_compiler->typGetNonGCLayout(layout); + JITDUMP("Changing layout of struct V%02u to block\n", lclNum); + lclVarDsc->ChangeLayout(newLayout); + } + else if (!layout->IsCustomLayout()) // hacky... want to know if there are any TYP_GC { - // No GC refs remain, so now a block layout - newLayout = m_compiler->typGetBlkLayout(layout->GetSize()); - JITDUMP("Changing layout of span V%02u to block\n", lclNum); + // New layout with all gc refs as byrefs + padding + // (todo, perhaps: see if old layout was already all byrefs) + newLayout = m_compiler->typGetByrefLayout(layout); + JITDUMP("Changing layout of struct V%02u to byref\n", lclNum); lclVarDsc->ChangeLayout(newLayout); } } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 99dd8965924669..51033f086ca0e1 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -125,6 +125,7 @@ class ObjectAllocator final : public Phase // Data members bool m_IsObjectStackAllocationEnabled; bool m_AnalysisDone; + unsigned m_initialLocalCount; BitVecTraits m_bitVecTraits; BitVec m_EscapingPointers; // We keep the set of possibly-stack-pointing pointers as a superset of the set of @@ -142,6 +143,9 @@ class ObjectAllocator final : public Phase unsigned m_numPseudoLocals; unsigned m_regionsToClone; + // Struct fields + bool m_trackFields; + //=============================================================================== // Methods public: @@ -160,6 +164,8 @@ class ObjectAllocator final : public Phase virtual PhaseStatus DoPhase() override; private: + bool IsTrackedType(var_types type); + bool IsTrackedLocal(unsigned lclNum); bool CanLclVarEscape(unsigned int lclNum); void MarkLclVarAsPossiblyStackPointing(unsigned int lclNum); void MarkLclVarAsDefinitelyStackPointing(unsigned int lclNum); @@ -220,6 +226,7 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) : Phase(comp, PHASE_ALLOCATE_OBJECTS) , m_IsObjectStackAllocationEnabled(false) , m_AnalysisDone(false) + , m_initialLocalCount(comp->lvaCount) , m_bitVecTraits(BitVecTraits(comp->lvaCount, comp)) , m_HeapLocalToStackLocalMap(comp->getAllocator(CMK_ObjectAllocator)) , m_EnumeratorLocalToPseudoLocalMap(comp->getAllocator(CMK_ObjectAllocator)) @@ -227,7 +234,7 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) , m_maxPseudoLocals(0) , m_numPseudoLocals(0) , m_regionsToClone(0) - + , m_trackFields(false) { // If we are going to do any conditional escape analysis, allocate // extra BV space for the "pseudo" locals we'll need. @@ -284,6 +291,7 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) m_ConnGraphAdjacencyMatrix = nullptr; m_StackAllocMaxSize = (unsigned)JitConfig.JitObjectStackAllocationSize(); + m_trackFields = JitConfig.JitObjectStackAllocationTrackFields() > 0; } //------------------------------------------------------------------------ diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 3b0e95acfa8d08..2bdb6c1d407035 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -81,6 +81,19 @@ struct NestedStruct public SimpleStruct s; } + struct GCStruct + { + public int i; + public int[] o1; + public int[] o2; + } + + struct GCStruct2 + { + public string[] a; + public string[] b; + } + enum AllocationKind { Heap, @@ -115,7 +128,8 @@ public class Tests public static int TestEntryPoint() { AllocationKind expectedAllocationKind = AllocationKind.Stack; - if (GCStressEnabled()) { + if (GCStressEnabled()) + { Console.WriteLine("GCStress is enabled"); expectedAllocationKind = AllocationKind.Undefined; } @@ -169,14 +183,18 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); - // Span captures + // Spans CallTestAndVerifyAllocation(SpanCaptureArray1, 41, expectedAllocationKind); CallTestAndVerifyAllocation(SpanCaptureArray2, 25, expectedAllocationKind); CallTestAndVerifyAllocation(SpanCaptureArrayT, 37, expectedAllocationKind); CallTestAndVerifyAllocation(SpanCaptureArrayT, 37, expectedAllocationKind); + // Other structs with GC fields. + CallTestAndVerifyAllocation(StructReferredObjects, 25, expectedAllocationKind); + // The remaining tests currently never allocate on the stack - if (expectedAllocationKind == AllocationKind.Stack) { + if (expectedAllocationKind == AllocationKind.Stack) + { expectedAllocationKind = AllocationKind.Heap; } @@ -193,6 +211,14 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(SpanEscapeArrayArgCopy, 42, expectedAllocationKind); CallTestAndVerifyAllocation(SpanEscapeArrayOutParam, 22, expectedAllocationKind); CallTestAndVerifyAllocation(SpanEscapeArrayOutParam2, 22, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeRef, 55, expectedAllocationKind); + + // Structs + CallTestAndVerifyAllocation(StructReferredObjectEscape1, 33, expectedAllocationKind); + CallTestAndVerifyAllocation(StructReferredObjectEscape2, 33, expectedAllocationKind); + CallTestAndVerifyAllocation(StructReferredObjectEscape3, 41, expectedAllocationKind); + CallTestAndVerifyAllocation(StructReferredObjectEscape4, 41, expectedAllocationKind); + CallTestAndVerifyAllocation(StructReferredObjectEscape5, 5, expectedAllocationKind); // This test calls CORINFO_HELP_OVERFLOW CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); @@ -223,27 +249,34 @@ static void CallTestAndVerifyAllocation(Test test, int expectedResult, Allocatio int testResult = test(); long allocatedBytesAfter = GC.GetAllocatedBytesForCurrentThread(); - if (testResult != expectedResult) { + if (testResult != expectedResult) + { Console.WriteLine($"FAILURE ({methodName}): expected {expectedResult}, got {testResult}"); methodResult = -1; } - else if ((expectedAllocationsKind == AllocationKind.Stack) && (allocatedBytesBefore != allocatedBytesAfter)) { + else if ((expectedAllocationsKind == AllocationKind.Stack) && (allocatedBytesBefore != allocatedBytesAfter)) + { Console.WriteLine($"FAILURE ({methodName}): unexpected allocation of {allocatedBytesAfter - allocatedBytesBefore} bytes"); methodResult = -1; } - else if ((expectedAllocationsKind == AllocationKind.Heap) && (allocatedBytesBefore == allocatedBytesAfter)) { + else if ((expectedAllocationsKind == AllocationKind.Heap) && (allocatedBytesBefore == allocatedBytesAfter)) + { Console.WriteLine($"FAILURE ({methodName}): unexpected stack allocation"); methodResult = -1; } - else { + else + { Console.WriteLine($"SUCCESS ({methodName})"); } } - catch { - if (throws) { + catch + { + if (throws) + { Console.WriteLine($"SUCCESS ({methodName})"); } - else { + else + { throw; } } @@ -452,6 +485,7 @@ static int SpanCaptureArray3() static int SpanCaptureArrayT() { Span span = new T[37]; + Use(span[0]); return span.Length; } @@ -464,7 +498,7 @@ static int SpanEscapeArrayArg() return y[42]; } - [MethodImpl(MethodImplOptions.NoInlining)] + [MethodImpl(MethodImplOptions.NoInlining)] static int SpanEscapeArrayArgCopy() { Span x = new int[100]; @@ -501,7 +535,7 @@ static void SpanEscapeArrayOutParamHelper(out Span a) a = new Span(new int[44]); a[10] = 22; } - + [MethodImpl(MethodImplOptions.NoInlining)] static int SpanEscapeArrayOutParam2() { @@ -521,6 +555,22 @@ static void SpanEscapeArrayOutParam2Helper(out SpanKeeper b) b.b = 2; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeRef() + { + ref int q = ref SpanEscapeRef(55); + TrashStack(); + return q; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static ref int SpanEscapeRef(int n) + { + Span x = new int[100]; + x[99] = n; + return ref x[99]; + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsEscape() { @@ -573,6 +623,126 @@ static int AllocateLongLengthArrayWithNonGCElements() return 1; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int StructReferredObjects() + { + int[] a1 = new int[10]; + int[] a2 = new int[10]; + + a1[3] = 7; + a2[4] = 8; + + GCStruct s = new GCStruct() { i = 10, o1 = a1, o2 = a2 }; + + return s.i + s.o1[3] + s.o2[4]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int StructReferredObjectEscape1() => StructReferredObjectEscape1Helper()[3]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int[] StructReferredObjectEscape1Helper() + { + int[] a1 = new int[10]; + int[] a2 = a1; + + a1[3] = 33; + a2[4] = 8; + + GCStruct s = new GCStruct() { i = 10, o1 = a1, o2 = a2 }; + + return s.o2; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int StructReferredObjectEscape2() + { + ref int a = ref StructReferredObjectEscape2Helper(); + TrashStack(); + return a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static ref int StructReferredObjectEscape2Helper() + { + int[] a1 = new int[10]; + int[] a2 = a1; + + a1[3] = 33; + a2[4] = 8; + + GCStruct s = new GCStruct() { i = 10, o1 = a1, o2 = a2 }; + return ref s.o2[3]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int StructReferredObjectEscape3() + { + GCStruct s = StructReferredObjectEscape3Helper(); + return s.o1[3] + s.o2[4]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static GCStruct StructReferredObjectEscape3Helper() + { + int[] a1 = new int[10]; + int[] a2 = a1; + + a1[3] = 33; + a2[4] = 8; + + GCStruct s = new GCStruct() { i = 10, o1 = a1, o2 = a2 }; + return s; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int StructReferredObjectEscape4() + { + GCStruct s; + StructReferredObjectEscape4Helper(out s); + return s.o1[3] + s.o2[4]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void StructReferredObjectEscape4Helper(out GCStruct s) + { + int[] a1 = new int[10]; + int[] a2 = a1; + + a1[3] = 33; + a2[4] = 8; + + s = new GCStruct() { i = 10, o1 = a1, o2 = a2 }; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int StructReferredObjectEscape5() + { + string[] s = StructReferredObjectEscape5Helper(0); + TrashStack(); + return s[0].Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static string[] StructReferredObjectEscape5Helper(int n) + { + GCStruct2 g = new GCStruct2(); + g.a = new string[10]; + g.b = new string[10]; + + g.a[0] = "Goodbye"; + g.b[0] = "Hello"; + + ref string[] rs = ref g.b; + + if (n > 0) + { + rs = ref g.a; + } + + return rs; + } + [MethodImpl(MethodImplOptions.NoInlining)] static void Use(ref int v) { @@ -591,6 +761,12 @@ static void Use(Span span) span[42] = 42; } + [MethodImpl(MethodImplOptions.NoInlining)] + static void Use(T t) + { + + } + [MethodImpl(MethodImplOptions.NoInlining)] static Span Identity(Span x) => x; From 7b0172ad8e5a19ca8a95564bc162f311ae0132d5 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 7 Mar 2025 19:25:32 -0800 Subject: [PATCH 3/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 3fefc3395b93f2..024ece2aece07a 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1394,7 +1394,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_LCL_ADDR: @@ -1406,6 +1405,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();