From 1729df43d29ce7cd0e1a4824d2aea3c1b9413b6d Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 28 May 2025 09:05:13 -0700 Subject: [PATCH 1/2] JIT: revisions to struct field escape analysis Peel off some changes from the more general field-wise escape analysis: * make sure we have modelled all relevant stores. Change local store tracking to use the new model. * keep track of which temps will represent stack allocated arrays and do suitable retyping * reject "known escaping" allocations earlier (eg finalizable objects) * handle local fields (rare but can exist in importer IR) * add the field test cases; all of these stack allocate for now This generally is able to classify more locals as non-GC. --- src/coreclr/jit/gentree.h | 1 - src/coreclr/jit/objectalloc.cpp | 250 +++++++-- src/coreclr/jit/objectalloc.h | 17 +- .../JIT/opt/ObjectStackAllocation/Fields.cs | 531 ++++++++++++++++++ .../opt/ObjectStackAllocation/Fields.csproj | 12 + 5 files changed, 775 insertions(+), 36 deletions(-) create mode 100644 src/tests/JIT/opt/ObjectStackAllocation/Fields.cs create mode 100644 src/tests/JIT/opt/ObjectStackAllocation/Fields.csproj diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 17f46648c2fba9..fab42a9c4385ee 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -450,7 +450,6 @@ enum GenTreeFlags : unsigned int GTF_VAR_MOREUSES = 0x00800000, // GT_LCL_VAR -- this node has additional uses, for example due to cloning GTF_VAR_CONTEXT = 0x00400000, // GT_LCL_VAR -- this node is part of a runtime lookup GTF_VAR_EXPLICIT_INIT = 0x00200000, // GT_LCL_VAR -- this node is an "explicit init" store. Valid until rationalization. - GTF_VAR_CONNECTED = 0x00100000, // GT_STORE_LCL_VAR -- this store was modelled in the connection graph during escape analysis // For additional flags for GT_CALL node see GTF_CALL_M_* diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 426a3f54f0cdd2..0c72717b6cdc39 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -42,7 +42,8 @@ ObjectAllocator::ObjectAllocator(Compiler* comp) , m_bvCount(0) , m_bitVecTraits(BitVecTraits(comp->lvaCount, comp)) , m_unknownSourceIndex(BAD_VAR_NUM) - , m_HeapLocalToStackLocalMap(comp->getAllocator(CMK_ObjectAllocator)) + , m_HeapLocalToStackObjLocalMap(comp->getAllocator(CMK_ObjectAllocator)) + , m_HeapLocalToStackArrLocalMap(comp->getAllocator(CMK_ObjectAllocator)) , m_ConnGraphAdjacencyMatrix(nullptr) , m_StackAllocMaxSize(0) , m_stackAllocationCount(0) @@ -703,13 +704,18 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() Compiler::fgWalkResult PostOrderVisit(GenTree** use, GenTree* user) { GenTree* const tree = *use; + if (tree->OperIsLocalStore()) { GenTreeLclVarCommon* const lclTree = tree->AsLclVarCommon(); unsigned const lclNum = lclTree->GetLclNum(); if (m_allocator->IsTrackedLocal(lclNum) && !m_allocator->CanLclVarEscape(lclNum)) { - if ((lclTree->gtFlags & GTF_VAR_CONNECTED) == 0) + // See if we connected it to a source. + // + StoreInfo* const info = m_allocator->m_StoreAddressToIndexMap.LookupPointer(tree); + + if ((info == nullptr) || !info->m_connected) { // This store was not modelled in the connection graph. // @@ -717,8 +723,12 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() // add an edge to unknown source. This will ensure this local does // not get retyped as TYP_I_IMPL. // - GenTree* const data = lclTree->Data(); - if (!data->IsIntegralConst(0) && (m_allocator->AllocationKind(data) == OAT_NONE)) + GenTree* const data = lclTree->Data(); + ObjectAllocationType const oat = m_allocator->AllocationKind(data); + bool const valueIsUnknown = + (oat == OAT_NEWOBJ_HEAP) || ((oat == OAT_NONE) && !data->IsIntegralConst(0)); + + if (valueIsUnknown) { // Add a connection to the unknown source. // @@ -727,8 +737,82 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() m_allocator->m_unknownSourceIndex); } } + else + { + JITDUMP(" ... Already connected at [%06u]\n", m_compiler->dspTreeID(tree)); + } + } + else + { + JITDUMP(" ... Not a GC store at [%06u]\n", m_compiler->dspTreeID(tree)); + } + } + else if (tree->OperIs(GT_STOREIND, GT_STORE_BLK)) + { + // Is this a GC store? + // + bool isGCStore = true; + + if (!m_allocator->IsTrackedType(tree->TypeGet())) + { + isGCStore = false; + } + else if (tree->OperIs(GT_STORE_BLK)) + { + isGCStore = tree->AsBlk()->GetLayout()->HasGCPtr(); + } + + // If so, did we model it yet? + // + if (isGCStore) + { + // See if we have an index for the destination, and if we connected it to a source. + // + StoreInfo* const info = m_allocator->m_StoreAddressToIndexMap.LookupPointer(tree); + + // Note here, unlike the local case above, we do not implicitly know the destination + // of the store. So if we have no info, we assume the store is to some place we don't track. + // + if ((info != nullptr) && !info->m_connected) + { + assert(info->m_index != BAD_VAR_NUM); + const unsigned dstIndex = info->m_index; + + JITDUMP(" ... Unmodelled GC store to"); + JITDUMPEXEC(m_allocator->DumpIndex(dstIndex)); + JITDUMP(" at [%06u]\n", m_compiler->dspTreeID(tree)); + + // Look for stores of nullptrs; these do not need to create a connection. + // + GenTree* const data = tree->AsIndir()->Data(); + bool const valueIsUnknown = !data->IsIntegralConst(0); + + if (valueIsUnknown) + { + m_allocator->AddConnGraphEdgeIndex(dstIndex, m_allocator->m_unknownSourceIndex); + JITDUMPEXEC(m_allocator->DumpIndex(dstIndex)) + JITDUMP(" ... value unknown at [%06u]\n", m_compiler->dspTreeID(tree)); + } + else + { + JITDUMP(" ... Store of nullptr(s) at [%06u]\n", m_compiler->dspTreeID(tree)); + } + + info->m_connected = true; + } + else if (info == nullptr) + { + JITDUMP(" ... No store info for [%06u]\n", m_compiler->dspTreeID(tree)); + } + else + { + JITDUMP(" ... Already connected at [%06u]\n", m_compiler->dspTreeID(tree)); + } + } + else + { + JITDUMP(" ... Not a GC store at [%06u]\n", m_compiler->dspTreeID(tree)); } - tree->gtFlags &= ~GTF_VAR_CONNECTED; } return Compiler::fgWalkResult::WALK_CONTINUE; } @@ -1003,6 +1087,11 @@ bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, unsigned classSize = 0; + if (allocType == OAT_NEWOBJ_HEAP) + { + *reason = "[runtime disallows]"; + return false; + } if (allocType == OAT_NEWARR) { if (!enableArrays) @@ -1040,12 +1129,7 @@ bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, return false; } - if (!comp->info.compCompHnd->canAllocateOnStack(clsHnd)) - { - *reason = "[runtime disallows]"; - return false; - } - + assert(comp->info.compCompHnd->canAllocateOnStack(clsHnd)); classSize = comp->info.compCompHnd->getHeapClassSize(clsHnd); } } @@ -1096,7 +1180,12 @@ ObjectAllocator::ObjectAllocationType ObjectAllocator::AllocationKind(GenTree* t ObjectAllocationType allocType = OAT_NONE; if (tree->OperGet() == GT_ALLOCOBJ) { - allocType = OAT_NEWOBJ; + GenTreeAllocObj* const allocObj = tree->AsAllocObj(); + CORINFO_CLASS_HANDLE clsHnd = allocObj->gtAllocObjClsHnd; + assert(clsHnd != NO_CLASS_HANDLE); + const bool isValueClass = comp->info.compCompHnd->isValueClass(clsHnd); + bool const canBeOnStack = isValueClass || comp->info.compCompHnd->canAllocateOnStack(clsHnd); + allocType = canBeOnStack ? OAT_NEWOBJ : OAT_NEWOBJ_HEAP; } else if (!m_isR2R && tree->IsHelperCall()) { @@ -1236,7 +1325,7 @@ void ObjectAllocator::MorphAllocObjNode(AllocationCandidate& candidate) { assert(candidate.m_onHeapReason != nullptr); JITDUMP("Allocating V%02u on the heap: %s\n", lclNum, candidate.m_onHeapReason); - if (candidate.m_allocType == OAT_NEWOBJ) + if ((candidate.m_allocType == OAT_NEWOBJ) || (candidate.m_allocType == OAT_NEWOBJ_HEAP)) { GenTree* const stmtExpr = candidate.m_tree; GenTree* const oldData = stmtExpr->AsLclVar()->Data(); @@ -1284,6 +1373,9 @@ bool ObjectAllocator::MorphAllocObjNodeHelper(AllocationCandidate& candidate) return MorphAllocObjNodeHelperArr(candidate); case OAT_NEWOBJ: return MorphAllocObjNodeHelperObj(candidate); + case OAT_NEWOBJ_HEAP: + candidate.m_onHeapReason = "[runtime disallows]"; + return false; default: unreached(); } @@ -1362,9 +1454,9 @@ bool ObjectAllocator::MorphAllocObjNodeHelperArr(AllocationCandidate& candidate) MorphNewArrNodeIntoStackAlloc(data->AsCall(), clsHnd, (unsigned int)len->AsIntCon()->IconValue(), blockSize, candidate.m_block, candidate.m_statement); - // Note we do not want to rewrite uses of the array temp, so we - // do not update m_HeapLocalToStackLocalMap. + // Keep track of this new local for later type updates. // + m_HeapLocalToStackArrLocalMap.AddOrUpdate(candidate.m_lclNum, stackLclNum); comp->Metrics.StackAllocatedArrays++; return true; @@ -1432,7 +1524,7 @@ bool ObjectAllocator::MorphAllocObjNodeHelperObj(AllocationCandidate& candidate) const unsigned int stackLclNum = MorphAllocObjNodeIntoStackAlloc(data->AsAllocObj(), layout, candidate.m_block, candidate.m_statement); - m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); + m_HeapLocalToStackObjLocalMap.AddOrUpdate(lclNum, stackLclNum); candidate.m_bashCall = true; @@ -1787,7 +1879,7 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi // Note that we modelled this store in the connection graph // - parent->gtFlags |= GTF_VAR_CONNECTED; + m_StoreAddressToIndexMap.Set(parent, StoreInfo(dstIndex, /* connected */ true)); } break; @@ -1855,6 +1947,24 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi case GT_STOREIND: case GT_STORE_BLK: { + // Is this a GC store? + // + if (!IsTrackedType(parent->TypeGet())) + { + canLclVarEscapeViaParentStack = false; + break; + } + + if (tree->OperIs(GT_STORE_BLK)) + { + ClassLayout* const layout = parent->AsBlk()->GetLayout(); + + if (!layout->HasGCPtr()) + { + canLclVarEscapeViaParentStack = false; + break; + } + } GenTree* const addr = parent->AsIndir()->Addr(); if (tree == addr) { @@ -1862,8 +1972,8 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi { // Remember the resource being stored to. // - JITDUMP("... store address\n"); - m_StoreAddressToIndexMap.Set(tree, lclIndex); + JITDUMP("... store address is local\n"); + m_StoreAddressToIndexMap.Set(parent, StoreInfo(lclIndex)); } // The address does not escape @@ -1872,22 +1982,32 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi break; } - // Is this a store to a tracked resource? + // If we're walking the value tree, model the store. // - unsigned dstIndex; - if (m_trackFields && m_StoreAddressToIndexMap.Lookup(addr, &dstIndex) && (dstIndex != BAD_VAR_NUM)) + StoreInfo* const dstInfo = m_StoreAddressToIndexMap.LookupPointer(parent); + if (dstInfo != nullptr) { + assert(dstInfo->m_index != BAD_VAR_NUM); + assert(!dstInfo->m_connected); JITDUMP("... local.field store\n"); - if (!isAddress) + // Note that we will model this store + // + dstInfo->m_connected = true; + + JITDUMP(" ... Modelled GC store to"); + JITDUMPEXEC(DumpIndex(dstInfo->m_index)); + JITDUMP(" at [%06u]\n", comp->dspTreeID(parent)); + + if (isAddress) { - AddConnGraphEdgeIndex(dstIndex, lclIndex); - canLclVarEscapeViaParentStack = false; - break; + AddConnGraphEdgeIndex(dstInfo->m_index, m_unknownSourceIndex); } else { - AddConnGraphEdgeIndex(dstIndex, m_unknownSourceIndex); + AddConnGraphEdgeIndex(dstInfo->m_index, lclIndex); + canLclVarEscapeViaParentStack = false; + break; } } @@ -1895,6 +2015,35 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi break; } + case GT_STORE_LCL_FLD: + { + // Does this store a type we're tracking? + // + if (!IsTrackedType(tree->TypeGet())) + { + canLclVarEscapeViaParentStack = false; + break; + } + + unsigned const dstLclNum = parent->AsLclVarCommon()->GetLclNum(); + + if (IsTrackedLocal(dstLclNum)) + { + JITDUMP("... local V%02u.f store\n", dstLclNum); + const unsigned dstIndex = LocalToIndex(dstLclNum); + AddConnGraphEdgeIndex(dstIndex, lclIndex); + canLclVarEscapeViaParentStack = false; + + // Note that we modelled this store in the connection graph + // + m_StoreAddressToIndexMap.Set(parent, StoreInfo(dstIndex, /* connected */ true)); + } + + // Else we're storing the value somewhere unknown. + // Assume the worst. + break; + } + case GT_IND: case GT_BLK: { @@ -1943,6 +2092,33 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi break; } + case GT_LCL_FLD: + { + // Does this load a type we're tracking? + // + if (!IsTrackedType(parent->TypeGet())) + { + canLclVarEscapeViaParentStack = false; + break; + } + + // For loads from local structs we may be tracking the underlying fields. + // + if (m_trackFields && (lclDsc->TypeGet() == TYP_STRUCT)) + { + JITDUMP("... load local.field\n"); + ++parentIndex; + isAddress = false; + keepChecking = true; + break; + } + + // Load from some untracked local's fields. + // + canLclVarEscapeViaParentStack = false; + break; + } + case GT_CALL: { GenTreeCall* const call = parent->AsCall(); @@ -2004,9 +2180,10 @@ void ObjectAllocator::AnalyzeParentStack(ArrayStack* parentStack, unsi } } - if (canLclVarEscapeViaParentStack) + if (canLclVarEscapeViaParentStack && !CanIndexEscape(lclIndex)) { - JITDUMP("V%02u first escapes via [%06u]...[%06u]\n", lclNum, comp->dspTreeID(parentStack->Top()), + JITDUMPEXEC(DumpIndex(lclIndex)); + JITDUMP(" first escapes via [%06u]...[%06u]\n", comp->dspTreeID(parentStack->Top()), comp->dspTreeID(parentStack->Top(parentIndex))); MarkLclVarAsEscaping(lclNum); } @@ -2359,7 +2536,7 @@ void ObjectAllocator::RewriteUses() var_types newType = lclVarDsc->TypeGet(); ClassLayout* newLayout = nullptr; - if (m_allocator->m_HeapLocalToStackLocalMap.TryGetValue(lclNum, &newLclNum)) + if (m_allocator->m_HeapLocalToStackObjLocalMap.TryGetValue(lclNum, &newLclNum)) { assert(tree->OperIs(GT_LCL_VAR)); // Must be a use. newType = TYP_I_IMPL; @@ -2552,9 +2729,15 @@ void ObjectAllocator::RewriteUses() } var_types newType = TYP_UNDEF; - if (m_HeapLocalToStackLocalMap.Contains(lclNum)) + if (m_HeapLocalToStackObjLocalMap.Contains(lclNum)) + { + // Appearances of lclNum will be replaced. We need to retype. + // + newType = TYP_I_IMPL; + } + else if (m_HeapLocalToStackArrLocalMap.Contains(lclNum)) { - // Appearances of lclNum will be replaced. We'll retype anyways. + // Appearances of lclNum will be NOT be replaced. We need to retype. // newType = TYP_I_IMPL; } @@ -3146,7 +3329,8 @@ void ObjectAllocator::CheckForGuardedAllocationOrCopy(BasicBlock* block, const char* reason = nullptr; unsigned size = 0; unsigned length = TARGET_POINTER_SIZE; - if (CanAllocateLclVarOnStack(enumeratorLocal, clsHnd, OAT_NEWOBJ, length, &size, &reason, + ObjectAllocationType oat = AllocationKind(data); + if (CanAllocateLclVarOnStack(enumeratorLocal, clsHnd, oat, length, &size, &reason, /* preliminaryCheck */ true)) { // We are going to conditionally track accesses to the enumerator local via a pseudo. diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 5d32911ad347ec..b06d39c7eb533c 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -109,8 +109,19 @@ struct CloneInfo : public GuardInfo bool m_willClone = false; }; +struct StoreInfo +{ + StoreInfo(unsigned index, bool connected = false) + : m_index(index) + , m_connected(connected) + { + } + unsigned m_index; + bool m_connected; +}; + typedef JitHashTable, CloneInfo*> CloneMap; -typedef JitHashTable, unsigned> NodeToIndexMap; +typedef JitHashTable, StoreInfo> NodeToIndexMap; class ObjectAllocator final : public Phase { @@ -118,6 +129,7 @@ class ObjectAllocator final : public Phase { OAT_NONE, OAT_NEWOBJ, + OAT_NEWOBJ_HEAP, OAT_NEWARR }; @@ -159,7 +171,8 @@ class ObjectAllocator final : public Phase // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. BitVec m_PossiblyStackPointingPointers; BitVec m_DefinitelyStackPointingPointers; - LocalToLocalMap m_HeapLocalToStackLocalMap; + LocalToLocalMap m_HeapLocalToStackObjLocalMap; + LocalToLocalMap m_HeapLocalToStackArrLocalMap; BitSetShortLongRep* m_ConnGraphAdjacencyMatrix; unsigned int m_StackAllocMaxSize; unsigned m_stackAllocationCount; diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Fields.cs b/src/tests/JIT/opt/ObjectStackAllocation/Fields.cs new file mode 100644 index 00000000000000..8caab8d4afb549 --- /dev/null +++ b/src/tests/JIT/opt/ObjectStackAllocation/Fields.cs @@ -0,0 +1,531 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using Xunit; + +enum AllocationKind +{ + Heap, + Stack, + Undefined +} + +delegate int Test(); + +public class X +{ + public X() { } + public X? a; + public X? b; + public int y; +} + +public class F +{ + public X x; + ~F() + { + if (x != null) + { + Console.WriteLine($"F destroyed: {x.y}"); + x = null; + } + } +} + +public class Fields +{ + static bool GCStressEnabled() + { + return Environment.GetEnvironmentVariable("DOTNET_GCStress") != null; + } + + static AllocationKind StackAllocation() + { + AllocationKind expectedAllocationKind = AllocationKind.Stack; + if (GCStressEnabled()) + { + Console.WriteLine("GCStress is enabled"); + expectedAllocationKind = AllocationKind.Undefined; + } + return expectedAllocationKind; + } + + static AllocationKind HeapAllocation() + { + AllocationKind expectedAllocationKind = AllocationKind.Heap; + if (GCStressEnabled()) + { + Console.WriteLine("GCStress is enabled"); + expectedAllocationKind = AllocationKind.Undefined; + } + return expectedAllocationKind; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Consume(X x) + { + // Do nothing + } + + static int CallTestAndVerifyAllocation(Test test, int expectedResult, AllocationKind expectedAllocationsKind, bool throws = false) + { + string methodName = test.Method.Name; + try + { + long allocatedBytesBefore = GC.GetAllocatedBytesForCurrentThread(); + int testResult = test(); + long allocatedBytesAfter = GC.GetAllocatedBytesForCurrentThread(); + + if (testResult != expectedResult) + { + Console.WriteLine($"FAILURE ({methodName}): expected {expectedResult}, got {testResult}"); + return -1; + } + + if ((expectedAllocationsKind == AllocationKind.Stack) && (allocatedBytesBefore != allocatedBytesAfter)) + { + Console.WriteLine($"FAILURE ({methodName}): unexpected allocation of {allocatedBytesAfter - allocatedBytesBefore} bytes"); + return -1; + } + + if ((expectedAllocationsKind == AllocationKind.Heap) && (allocatedBytesBefore == allocatedBytesAfter)) + { + Console.WriteLine($"FAILURE ({methodName}): unexpected stack allocation"); + return -1; + } + else + { + Console.WriteLine($"SUCCESS ({methodName})"); + return 100; + } + } + catch + { + if (throws) + { + Console.WriteLine($"SUCCESS ({methodName})"); + return 100; + } + else + { + return -1; + } + } + } + + // --------------- NO ESCAPE TESTS (once fields are fully enabled) ------------------ + + // Local objects that refer to one another + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack0() + { + X x1 = new X(); + x1.y = 3; + x1.a = new X(); + x1.a.y = 4; + x1.b = x1.a; + + return x1.y + x1.a.y + x1.b.y; + } + + [Fact] + public static int Stack0() + { + RunStack0(); + return CallTestAndVerifyAllocation(RunStack0, 11, HeapAllocation()); + } + + // Field refers to both non-escaping and other objects + + static X s_x = new X(); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack2() + { + X x = new X(); + x.y = 3; + x.a = new X(); + x.b = s_x; + + return x.y + x.a.y + x.b.y; + } + + [Fact] + public static int Stack2() + { + RunStack2(); + return CallTestAndVerifyAllocation(RunStack2, 3, HeapAllocation()); + } + + // Array refers to both non-escaping and other objects + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack3() + { + X[] x = new X[2]; + + x[0] = new X(); + x[1] = s_x; + x[0].y = 77; + + return x[0].y + x[1].y; + } + + [Fact] + public static int Stack3() + { + return CallTestAndVerifyAllocation(RunStack3, 77, HeapAllocation()); + } + + // Local objects that refer to one another (including array) + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack4() + { + X[] a = new X[10]; + a[0] = new X(); + a[1] = new X(); + + a[0].y = 3; + a[1].y = 4; + + GC.Collect(); + + return a[1].y + a[0].y; + } + + [Fact] + public static int Stack4() + { + return CallTestAndVerifyAllocation(RunStack4, 7, HeapAllocation()); + } + + // Local objects that refer to one another (including array) + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack5() + { + X[] a = new X[10]; + a[0] = new X(); + + a[1] = a[0]; + + a[0].y = 3; + a[1].y = 4; + + GC.Collect(); + + return a[1].y + a[0].y; + } + + [Fact] + public static int Stack5() + { + return CallTestAndVerifyAllocation(RunStack5, 8, HeapAllocation()); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunStack6() + { + // Array of structs of arrays + strings. + + var fullInput = new[] + { + new { utf8Bytes = new byte[] { 0x40 }, output = "@" }, + new { utf8Bytes = new byte[] { 0xC3, 0x85 }, output = "[00C5]" }, + }; + + int result = 0; + + foreach (var f in fullInput) + { + if (result == 0) + { + GC.Collect(); + } + result += f.utf8Bytes.Length + f.output.Length; + } + + return result; + } + + [Fact] + public static int Stack6() + { + return CallTestAndVerifyAllocation(RunStack6, 10, HeapAllocation()); + } + + // --------------- ESCAPE TESTS ------------------ + + // Field escapes via return + + [MethodImpl(MethodImplOptions.NoInlining)] + static X DoHeap0() + { + X x1 = new X(); + x1.y = 3; + x1.a = new X(); + x1.a.y = 4; + x1.b = x1.a; + + return x1.b; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap0() + { + X x = DoHeap0(); + + return x.y; + } + + [Fact] + public static int Heap0() + { + return CallTestAndVerifyAllocation(RunHeap0, 4, HeapAllocation()); + } + + // Field escapes via param + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap1(ref X xe) + { + X x1 = new X(); + x1.y = 3; + x1.a = new X(); + x1.a.y = 4; + x1.b = x1.a; + + xe = x1.a; + + return x1.y + x1.a.y + x1.b.y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap1() + { + X x = null; + return DoHeap1(ref x); + } + + [Fact] + public static int Heap1() + { + return CallTestAndVerifyAllocation(RunHeap1, 11, HeapAllocation()); + } + + // Field escapes via assignment to param field + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap2(X x) + { + X x1 = new X(); + x1.y = 3; + x1.a = new X(); + x1.a.y = 8; + x1.b = x1.a; + + x.b = x1.b; + + return x1.y + x1.a.y + x1.b.y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap2() + { + return DoHeap2(new X()); + } + + [Fact] + public static int Heap2() + { + return CallTestAndVerifyAllocation(RunHeap2, 19, HeapAllocation()); + } + + // Object escapes via assignment to param field + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap3(X x) + { + X x1 = new X(); + x.b = x1; + x1.y = 11; + + return x1.y + x.b.y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap3() + { + return DoHeap3(new X()); + } + + [Fact] + public static int Heap3() + { + return CallTestAndVerifyAllocation(RunHeap3, 22, HeapAllocation()); + } + + // Field escapes via call + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap4() + { + X x1 = new X(); + x1.y = 3; + x1.a = new X(); + x1.a.y = 4; + x1.b = x1.a; + + Consume(x1.b); + + return x1.y + x1.a.y + x1.b.y; + } + + [Fact] + public static int Heap4() + { + return CallTestAndVerifyAllocation(RunHeap4, 11, HeapAllocation()); + } + + // Array field escapes via return + + [MethodImpl(MethodImplOptions.NoInlining)] + static X DoHeap5() + { + X[] x = new X[2]; + x[0] = new X(); + x[1] = s_x; + + x[0].y = 33; + + return x[0]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap5() + { + return DoHeap5().y; + } + + [Fact] + public static int Heap5() + { + RunHeap5(); + return CallTestAndVerifyAllocation(RunHeap5, 33, HeapAllocation()); + } + + // Array field escapes via param + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap6(ref X xe) + { + X[] x = new X[2]; + x[0] = new X(); + x[1] = s_x; + + x[0].y = 44; + + xe = x[1]; + + return x[0].y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap6() + { + X x = null; + return DoHeap6(ref x); + } + + [Fact] + public static int Heap6() + { + return CallTestAndVerifyAllocation(RunHeap6, 44, HeapAllocation()); + } + + // Array field escapes via param field + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap7(X xe) + { + X[] x = new X[2]; + x[0] = new X(); + x[1] = s_x; + + x[0].y = 45; + + xe.a = x[1]; + + return x[0].y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap7() + { + X x = new X(); + return DoHeap7(x); + } + + [Fact] + public static int Heap7() + { + return CallTestAndVerifyAllocation(RunHeap7, 45, HeapAllocation()); + } + + // Delegate and closure + // Delegate doesn't escape, but closure does + + [MethodImpl(MethodImplOptions.NoInlining)] + static int DoHeap8(int[] a) + { + int y = 1; + var f = (int x) => x + y; + int sum = 0; + + foreach (int i in a) + { + sum += f(i); + } + + return sum; + } + + static int[] s_a = new int[100]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap8() => DoHeap8(s_a); + + [Fact] + public static int Heap8() + { + RunHeap8(); + return CallTestAndVerifyAllocation(RunHeap8, 100, HeapAllocation()); + } + + // "non-escaping" finalizable object with GC fields + // + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunHeap9() + { + F f = new F(); + f.x = new X(); + f.x.y = 1; + return 100; + } + + [Fact] + public static int Heap9() + { + int result = CallTestAndVerifyAllocation(RunHeap9, 100, HeapAllocation()); + GC.Collect(); + GC.WaitForPendingFinalizers(); + return result; + } +} diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Fields.csproj b/src/tests/JIT/opt/ObjectStackAllocation/Fields.csproj new file mode 100644 index 00000000000000..8a86f57ba24f36 --- /dev/null +++ b/src/tests/JIT/opt/ObjectStackAllocation/Fields.csproj @@ -0,0 +1,12 @@ + + + + true + None + True + true + + + + + From 642ad8fbe46bad2974a965f9d070b29418c363a2 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 30 May 2025 08:27:32 -0700 Subject: [PATCH 2/2] keep retyping layouts after first indir; add test --- src/coreclr/jit/objectalloc.cpp | 5 +- .../ObjectStackAllocation/Runtime_116214.cs | 59 +++++++++++++++++++ .../Runtime_116214.csproj | 9 +++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.cs create mode 100644 src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.csproj diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 0c72717b6cdc39..5b043583c70ef1 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -2213,6 +2213,7 @@ void ObjectAllocator::UpdateAncestorTypes( assert(parentStack != nullptr); int parentIndex = 1; bool keepChecking = true; + bool sawIndir = false; while (keepChecking && (parentStack->Height() > parentIndex)) { @@ -2416,7 +2417,7 @@ void ObjectAllocator::UpdateAncestorTypes( { // If we are loading from a GC struct field, we may need to retype the load // - if (retypeFields) + if (retypeFields && !sawIndir) { bool didRetype = false; @@ -2460,7 +2461,7 @@ void ObjectAllocator::UpdateAncestorTypes( { ++parentIndex; keepChecking = true; - retypeFields = false; + sawIndir = true; } } diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.cs b/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.cs new file mode 100644 index 00000000000000..182a2a2b10c370 --- /dev/null +++ b/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.Intrinsics; +using Xunit; + +// Generated by Fuzzlyn v3.1 on 2025-05-30 09:51:10 +// Run on Arm64 MacOS +// Seed: 1654487800225229357-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256 +// Reduced from 20.8 KiB to 0.7 KiB in 00:00:16 +// Hits JIT assert for Release: +// Assertion failed 'm_blockLayout->CanAssignFrom(m_src->GetLayout(m_comp))' in 'Program:M1():ubyte' during 'Morph - Global' (IL size 62; hash 0x3f34c1f3; FullOpts) +// +// File: /Users/runner/work/1/s/src/coreclr/jit/morphblock.cpp Line: 668 +// + +public class C0 +{ +} + +public struct S0 +{ + public C0 F0; + public bool F2; + public C0 F4; + public byte F6; + public S0(C0 f4) : this() + { + F4 = f4; + } +} + +public struct S1 +{ + public S0 F2; + public Vector64 F4; + public S1(S0 f2) + { + F2 = f2; + } +} + +public class Runtime_116124 +{ + [Fact] + public static void Test() + { + bool vr1 = M1(); + } + + static bool M1() + { + S1 var0 = new S1(new S0(new C0())); + var0.F2 = var0.F2; + byte var5 = var0.F2.F6++; + return var0.F2.F2; + } +} diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.csproj b/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.csproj new file mode 100644 index 00000000000000..501217e4d86892 --- /dev/null +++ b/src/tests/JIT/opt/ObjectStackAllocation/Runtime_116214.csproj @@ -0,0 +1,9 @@ + + + None + True + + + + +