From 2a5c22000f524ebd4ccc423bf13f081ab8e4c00a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 13 Feb 2025 15:27:06 -0800 Subject: [PATCH 1/8] JIT: enhance escape analysis to understand `Span` capture `Span` captured arrays can be passed to callees without escaping. Implement a very simplistic "field sensitive" analysis for `Span` where we pretend the span is simply its byref field. --- src/coreclr/jit/compiler.h | 7 ++ src/coreclr/jit/objectalloc.cpp | 114 +++++++++++++++++++++++++------- 2 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index dd967f33cbed9e..c9b2d311846b28 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1207,6 +1207,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 fce4152d0a0620..8b536153969aef 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,40 @@ 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 a store to the x field of a span? + // Todo: mark like IsSpanLength? + // (more generally, a store to a ref class -- though we'd also need to handle the load case) + // + if (addr->OperIs(GT_FIELD_ADDR)) + { + 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. @@ -1156,6 +1198,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent JITDUMP("Enumerator V%02u passed to call...\n", lclNum); canLclVarEscapeViaParentStack = !CheckForGuardedUse(block, parent, lclNum); } + else if (isSpanLocal) + { + canLclVarEscapeViaParentStack = false; + } break; } @@ -1227,6 +1273,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); @@ -1264,17 +1311,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; @@ -1335,10 +1383,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. @@ -1355,12 +1400,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) From 0b4a754e8b2ad7346b051ac1ace0ac617c9a4566 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 13 Feb 2025 18:28:28 -0800 Subject: [PATCH 2/8] model calls to methods that can return span arg fields --- src/coreclr/jit/objectalloc.cpp | 78 +++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 8b536153969aef..690a800ab8bc92 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1174,12 +1174,12 @@ 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)); } // Note there is nothing special here about the parent being a call. We could move all this processing @@ -1200,7 +1200,77 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent } else if (isSpanLocal) { - canLclVarEscapeViaParentStack = false; + // A span argument can't escape, but the callee can pass its fields back via + // return value and the return value may escape. (Todo: model this more precisely + // -- see if we're assigning to a byref gc field) + // + JITDUMP("Span V%02u parent is call...\n", lclNum); + + CallArg* const retBufArg = call->gtArgs.GetRetBufferArg(); + if (retBufArg != nullptr) + { + GenTree* const retBuf = retBufArg->GetNode(); + + if (retBuf->OperIs(GT_LCL_ADDR)) + { + GenTreeLclVarCommon* const dstLocal = retBuf->AsLclVarCommon(); + + if (dstLocal == tree) + { + JITDUMP(" ... span is local retbuf\n"); + } + else + { + const unsigned dstLclNum = dstLocal->GetLclNum(); + LclVarDsc* const dstLclDsc = comp->lvaGetDesc(dstLclNum); + + if (dstLclDsc->HasGCPtr()) + { + // Model as assignment of the argument to the return value local + // + JITDUMP(" ... (possible) byref return buffer. Modelling as V%02u <- V%02u\n", + dstLclNum, lclNum); + AddConnGraphEdge(dstLclNum, lclNum); + } + else + { + JITDUMP(" ... non-byref return buffer\n"); + } + } + canLclVarEscapeViaParentStack = false; + } + else + { + JITDUMP(" ... retbuf is not a local\n"); + } + } + else if (call->TypeGet() == TYP_STRUCT) + { + CORINFO_CLASS_HANDLE retClsHnd = call->gtRetClsHnd; + assert(retClsHnd != NO_CLASS_HANDLE); + DWORD attrs = comp->info.compCompHnd->getClassAttribs(retClsHnd); + + if ((attrs & CORINFO_FLG_BYREF_LIKE) != 0) + { + JITDUMP(" ... byref return\n"); + // defer to parent. + ++parentIndex; + keepChecking = true; + } + else + { + JITDUMP(" ... non-byref return\n"); + canLclVarEscapeViaParentStack = false; + } + } + else + { + JITDUMP(" ... non-struct return\n"); + canLclVarEscapeViaParentStack = false; + } + + // Todo: the callee may also assign to any ref or out params... + // } break; } From cf45f6b5434bcae0f1c292ac7d41bc996499d709 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 14 Feb 2025 14:26:36 -0800 Subject: [PATCH 3/8] restrict to span captured arrays --- src/coreclr/jit/objectalloc.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 690a800ab8bc92..4b557670b72149 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -84,6 +84,8 @@ PhaseStatus ObjectAllocator::DoPhase() assert(enabled); ComputeStackObjectPointers(&m_bitVecTraits); RewriteUses(); + + printf("\n**** Stack allocation in 0x%08x %s\n", comp->info.compMethodHash(), comp->info.compFullName); } // This phase always changes the IR. It may also modify the flow graph. @@ -1137,12 +1139,15 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent if (tree != addr) { JITDUMP("... tree != addr\n"); - // Is this a store to the x field of a span? - // Todo: mark like IsSpanLength? - // (more generally, a store to a ref class -- though we'd also need to handle the load case) + + // 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 (addr->OperIs(GT_FIELD_ADDR)) + 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)) From 7f332225dbf84b873bdc1953f7a4537ca2073db1 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Sat, 15 Feb 2025 08:33:44 -0800 Subject: [PATCH 4/8] remove printf --- src/coreclr/jit/objectalloc.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 4b557670b72149..d9d17a92b1532e 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -84,8 +84,6 @@ PhaseStatus ObjectAllocator::DoPhase() assert(enabled); ComputeStackObjectPointers(&m_bitVecTraits); RewriteUses(); - - printf("\n**** Stack allocation in 0x%08x %s\n", comp->info.compMethodHash(), comp->info.compFullName); } // This phase always changes the IR. It may also modify the flow graph. From 2e4e81d00e937bb176861b2618f6df0d83c7c911 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 17 Feb 2025 06:49:45 -0800 Subject: [PATCH 5/8] align up array layouts; avoid tail calls --- src/coreclr/jit/layout.cpp | 1 + src/coreclr/jit/objectalloc.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/layout.cpp b/src/coreclr/jit/layout.cpp index e84f55781ce17e..1e4613e5e2eb90 100644 --- a/src/coreclr/jit/layout.cpp +++ b/src/coreclr/jit/layout.cpp @@ -778,6 +778,7 @@ ClassLayoutBuilder ClassLayoutBuilder::BuildArray(Compiler* compiler, CORINFO_CL ClrSafeInt totalSize(elementSize); totalSize *= static_cast(length); + totalSize.AlignUp(TARGET_POINTER_SIZE); totalSize += static_cast(OFFSETOF__CORINFO_Array__data); assert(!totalSize.IsOverflow()); diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index d9d17a92b1532e..c039b40dd16928 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1201,7 +1201,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent JITDUMP("Enumerator V%02u passed to call...\n", lclNum); canLclVarEscapeViaParentStack = !CheckForGuardedUse(block, parent, lclNum); } - else if (isSpanLocal) + else if (isSpanLocal && !call->CanTailCall()) { // A span argument can't escape, but the callee can pass its fields back via // return value and the return value may escape. (Todo: model this more precisely From 6f81a235c5ed464dcb4a2ffd401baa85b8294742 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 18 Feb 2025 18:31:09 -0800 Subject: [PATCH 6/8] JIT: array allocation fixes Fix some issues with array stack allocation that are currently hard to observe: * use correct method table for shared array types * pad array layout out to a multiple of TARGET_POINTER_SIZE It's currently not easy to observe the type of a stack allocated array as accessing the type causes escape. But this becomes possible when supporting span capture or once we have a version of the covariant store check that can handle stack allocated arrays. Non-padded layouts may mean elements just off the end of an odd-length short or byte array won't be zeroed, which some unsafe uses may expect. Added some test cases. --- src/coreclr/jit/helperexpansion.cpp | 10 ++--- .../ObjectStackAllocationTests.cs | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index 731dbd477ba67d..6ecbc71cedf292 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -2822,6 +2822,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt, const CorInfoHelpFunc helper = eeGetHelperNum(call->gtCallMethHnd); int lengthArgIndex = -1; + int typeArgIndex = -1; switch (helper) { @@ -2830,10 +2831,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt, case CORINFO_HELP_NEWARR_1_OBJ: case CORINFO_HELP_NEWARR_1_ALIGN8: lengthArgIndex = 1; - break; - - case CORINFO_HELP_READYTORUN_NEWARR_1: - lengthArgIndex = 0; + typeArgIndex = 0; break; default: @@ -2871,9 +2869,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt, // Initialize the array method table pointer. // - CORINFO_CLASS_HANDLE arrayHnd = (CORINFO_CLASS_HANDLE)call->compileTimeHelperArgumentHandle; - - GenTree* const mt = gtNewIconEmbClsHndNode(arrayHnd); + GenTree* const mt = call->gtArgs.GetArgByIndex(typeArgIndex)->GetNode(); GenTree* const mtStore = gtNewStoreValueNode(TYP_I_IMPL, stackLocalAddress, mt); Statement* const mtStmt = fgNewStmtFromTree(mtStore); diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 07114d7ebd9b7c..6065b9cb3ae7a5 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -156,7 +156,11 @@ public static int TestEntryPoint() // Stack allocation of boxed structs is now enabled CallTestAndVerifyAllocation(BoxSimpleStructAndAddFields, 12, expectedAllocationKind); + // Fixed-sized stack array cases CallTestAndVerifyAllocation(AllocateArrayWithNonGCElements, 84, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayWithGCElements, 84, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); // The remaining tests currently never allocate on the stack if (expectedAllocationKind == AllocationKind.Stack) { @@ -170,6 +174,7 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsEscape, 42, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayWithGCElementsEscape, 42, expectedAllocationKind); // This test calls CORINFO_HELP_OVERFLOW CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); @@ -375,6 +380,29 @@ static int AllocateArrayWithNonGCElements() return array[24] + array.Length; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithGCElements() + { + string[] array = new string[42]; + array[24] = "42"; + GC.Collect(); + return array[24].Length * 21 + array.Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayT() + { + T[] array = new T[42]; + T t = array[24]; + GC.Collect(); + + // Todo -- validate array type (currently causes escape for shared) + // Todo -- store to array (currently causes escape for shared) + + Consume(t); + return array.Length + 42; + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsEscape() { @@ -384,6 +412,15 @@ static int AllocateArrayWithNonGCElementsEscape() return array[24]; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithGCElementsEscape() + { + string[] array = new string[42]; + Use(ref array[24]); + GC.Collect(); + return array[24].Length * 21; + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsOutOfRangeRight() { @@ -424,6 +461,12 @@ static void Use(ref int v) v = 42; } + [MethodImpl(MethodImplOptions.NoInlining)] + static void Use(ref string s) + { + s = "42"; + } + [MethodImpl(MethodImplOptions.NoInlining)] private static void ZeroAllocTest() { From 079e48b9f7adc60959cebeabdacec38d3694f193 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 19 Feb 2025 13:19:24 -0800 Subject: [PATCH 7/8] update test case --- .../ObjectStackAllocationTests.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 6065b9cb3ae7a5..086c4c55ef9cbc 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -162,6 +162,12 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayT, 84, expectedAllocationKind); + // Span captures + CallTestAndVerifyAllocation(SpanCaptureArray1, 83, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanCaptureArray2, 142, 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 +182,9 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsEscape, 42, expectedAllocationKind); CallTestAndVerifyAllocation(AllocateArrayWithGCElementsEscape, 42, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArray1, 100, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArray2, 100, expectedAllocationKind); + // This test calls CORINFO_HELP_OVERFLOW CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); @@ -403,6 +412,69 @@ static int AllocateArrayT() return array.Length + 42; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArray1() + { + Span span = new int[100]; + span[10] = 41; + Use(span); + 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] = 100; + Use(Identity(span)); + return span[10] + span[42]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanCaptureArray3() + { + Span span = new int[128]; + span[10] = 100; + Span x = Identity(span); + Use(x); + Use(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 SpanEscapeArray1() => SpanEscapeArray1Helper()[10]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static Span SpanEscapeArray1Helper() + { + int[] x = new int[128]; + Span span = x; + span[10] = 100; + Use(span); + return span; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArray2() => SpanEscapeArray2Helper()[10]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static Span SpanEscapeArray2Helper() + { + int[] x = new int[128]; + Span span = x; + span[10] = 100; + return Identity(span); + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsEscape() { @@ -467,6 +539,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() { From 62cb6fac7aa8c6757de0e9d1d6bd375003b93fee Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 19 Feb 2025 17:00:42 -0800 Subject: [PATCH 8/8] more tests; handle out/ref arg cases --- src/coreclr/jit/objectalloc.cpp | 47 +++++++++++- .../ObjectStackAllocationTests.cs | 72 ++++++++++++++++++- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index c039b40dd16928..6813aa93aaacc3 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1214,6 +1214,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent { GenTree* const retBuf = retBufArg->GetNode(); + // Is it possible to assign directly to a struct's struct field, + // or is the retbuf always a standalone struct? Seems like it has + // to be the latter to give proper no-alias guarantees. + // if (retBuf->OperIs(GT_LCL_ADDR)) { GenTreeLclVarCommon* const dstLocal = retBuf->AsLclVarCommon(); @@ -1272,8 +1276,49 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent canLclVarEscapeViaParentStack = false; } - // Todo: the callee may also assign to any ref or out params... + // The callee may also assign to any ref or out params. + // Try and model this as an assignment, if possible. // + for (CallArg& arg : call->gtArgs.Args()) + { + if (!arg.IsUserArg()) + { + continue; + } + + GenTree* const argNode = arg.GetNode(); + + if ((argNode != tree) && (argNode->TypeIs(TYP_BYREF))) + { + // Try and resolve to a local and model as an assignment. + // (todo: could also be a field of a local) + // + if (argNode->OperIs(GT_LCL_ADDR)) + { + GenTreeLclVarCommon* const argLocal = argNode->AsLclVarCommon(); + const unsigned argLclNum = argLocal->GetLclNum(); + LclVarDsc* const argLclDsc = comp->lvaGetDesc(argLclNum); + + if (argLclDsc->HasGCPtr()) + { + // Model as assignment of the argument to the byref local + // + JITDUMP(" ... (possible) byref out param. Modelling as V%02u <- V%02u\n", argLclNum, + lclNum); + AddConnGraphEdge(argLclNum, lclNum); + } + } + else + { + // Can't easily tell what this byref refers to, assume the worst. + // (points-to analysis anyone)? + // Would be nice to track byref referent types too. + // + JITDUMP(" ... byref arg [%06u]; assuming escape\n", comp->dspTreeID(argNode)); + canLclVarEscapeViaParentStack = true; + } + } + } } break; } diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 086c4c55ef9cbc..5c0e216be0a2d8 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; @@ -184,6 +191,8 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(SpanEscapeArray1, 100, expectedAllocationKind); CallTestAndVerifyAllocation(SpanEscapeArray2, 100, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArray3, 99, expectedAllocationKind); + CallTestAndVerifyAllocation(SpanEscapeArray4, 22, expectedAllocationKind); // This test calls CORINFO_HELP_OVERFLOW CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); @@ -451,7 +460,12 @@ static int SpanCaptureArrayT() } [MethodImpl(MethodImplOptions.NoInlining)] - static int SpanEscapeArray1() => SpanEscapeArray1Helper()[10]; + static int SpanEscapeArray1() + { + Span y = SpanEscapeArray1Helper(); + TrashStack(); + return y[10]; + } [MethodImpl(MethodImplOptions.NoInlining)] static Span SpanEscapeArray1Helper() @@ -464,7 +478,12 @@ static Span SpanEscapeArray1Helper() } [MethodImpl(MethodImplOptions.NoInlining)] - static int SpanEscapeArray2() => SpanEscapeArray2Helper()[10]; + static int SpanEscapeArray2() + { + Span y = SpanEscapeArray2Helper(); + TrashStack(); + return y[10]; + } [MethodImpl(MethodImplOptions.NoInlining)] static Span SpanEscapeArray2Helper() @@ -475,6 +494,45 @@ static Span SpanEscapeArray2Helper() return Identity(span); } + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArray3() => SpanEscapeArray3Helper()[10]; + + [MethodImpl(MethodImplOptions.NoInlining)] + static Span SpanEscapeArray3Helper() + { + Span x = new int[44]; + Span y; + SpanEscapeArray3HelperHelper(x, out y); + TrashStack(); + return y; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void SpanEscapeArray3HelperHelper(Span a, out Span b) + { + a[10] = 99; + b = a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int SpanEscapeArray4() + { + Span x = new int[44]; + SpanKeeper y; + SpanEscapeArray4Helper(x, out y); + TrashStack(); + return y.span[10]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void SpanEscapeArray4Helper(Span a, out SpanKeeper b) + { + a[10] = 22; + b.a = 1; + b.span = a; + b.b = 2; + } + [MethodImpl(MethodImplOptions.NoInlining)] static int AllocateArrayWithNonGCElementsEscape() { @@ -607,6 +665,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); } }