From 7d80ec8520ef07e574f4d0c3d3212bc9b10d1476 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 29 May 2025 09:00:16 -0700 Subject: [PATCH 1/2] JIT: generalize escape analysis delegate invoke expansion slightly Also expand the invoke when the delegate object address has been substituted directly. Addresses example in #84872. --- src/coreclr/jit/objectalloc.cpp | 11 +++++--- .../opt/ObjectStackAllocation/Delegates.cs | 26 +++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 881ce16abd99e2..c13140f1d50305 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -2401,25 +2401,28 @@ void ObjectAllocator::RewriteUses() CallArg* const thisArg = call->gtArgs.GetThisArg(); GenTree* const delegateThis = thisArg->GetNode(); - if (delegateThis->OperIs(GT_LCL_VAR)) + if (delegateThis->OperIs(GT_LCL_VAR, GT_LCL_ADDR)) { GenTreeLclVarCommon* const lcl = delegateThis->AsLclVarCommon(); + bool const isStackAllocatedDelegate = + delegateThis->OperIs(GT_LCL_ADDR) || m_allocator->DoesLclVarPointToStack(lcl->GetLclNum()); - if (m_allocator->DoesLclVarPointToStack(lcl->GetLclNum())) + if (isStackAllocatedDelegate) { JITDUMP("Expanding delegate invoke [%06u]\n", m_compiler->dspTreeID(call)); + // Expand the delgate invoke early, so that physical promotion has // a chance to promote the delegate fields. // // Note the instance field may also be stack allocatable (someday) // - GenTree* const cloneThis = m_compiler->gtClone(lcl); + GenTree* const cloneThis = m_compiler->gtClone(lcl, /* complexOk */ true); unsigned const instanceOffset = m_compiler->eeGetEEInfo()->offsetOfDelegateInstance; GenTree* const newThisAddr = m_compiler->gtNewOperNode(GT_ADD, TYP_I_IMPL, cloneThis, m_compiler->gtNewIconNode(instanceOffset, TYP_I_IMPL)); - // For now assume the instance is heap... + // For now assume the instance field is on the heap... // GenTree* const newThis = m_compiler->gtNewIndir(TYP_REF, newThisAddr); thisArg->SetEarlyNode(newThis); diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs b/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs index d0b49cfcfdd125..e32e094d590105 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs @@ -34,6 +34,17 @@ static AllocationKind StackAllocation() return expectedAllocationKind; } + static AllocationKind HeapAllocation() + { + AllocationKind expectedAllocationKind = AllocationKind.Heap; + if (GCStressEnabled()) + { + Console.WriteLine("GCStress is enabled"); + expectedAllocationKind = AllocationKind.Undefined; + } + return expectedAllocationKind; + } + static int CallTestAndVerifyAllocation(Test test, int expectedResult, AllocationKind expectedAllocationsKind, bool throws = false) { string methodName = test.Method.Name; @@ -122,4 +133,19 @@ public static int Test1() RunTest1(); return CallTestAndVerifyAllocation(RunTest1, 100, StackAllocation()); } + + // Here the delegate gets stack allocated, but not the closure. + // With PGO the delegate is also inlined. + // + [MethodImpl(MethodImplOptions.NoInlining)] + static int Test2(int a) => InvokeFunc(x => x + a); + + static int InvokeFunc(Func func) => func(101); + + [Fact] + public static int Test2() + { + RunTest2(); + return CallTestAndVerifyAllocation(RunTest2, 100, HeapAllocation()); + } } From 9c78a3c2c0da9a34d7e8a76092b70ce7011d5e0a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 29 May 2025 11:08:58 -0700 Subject: [PATCH 2/2] fix test --- src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs b/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs index e32e094d590105..35000eb1e397fb 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs @@ -138,14 +138,16 @@ public static int Test1() // With PGO the delegate is also inlined. // [MethodImpl(MethodImplOptions.NoInlining)] - static int Test2(int a) => InvokeFunc(x => x + a); + static int RunTest2Inner(int a) => InvokeFunc(x => x + a); static int InvokeFunc(Func func) => func(101); + [MethodImpl(MethodImplOptions.NoInlining)] + static int RunTest2() => RunTest2Inner(-1); + [Fact] public static int Test2() { - RunTest2(); return CallTestAndVerifyAllocation(RunTest2, 100, HeapAllocation()); } }