JIT: generalize escape analysis delegate invoke expansion slightly #116099
Conversation
Also expand the invoke when the delegate object address has been substituted directly. Addresses example in dotnet#84872.
github-actions
bot
added
the
area-CodeGen-coreclr
There was a problem hiding this comment.
Pull Request Overview
This PR generalizes the escape analysis for delegate invoke expansion by additionally handling cases where the delegate object is referenced via its address, and it introduces a new heap allocation path for cases with GC stress enabled.
- Added a HeapAllocation() method and Test2 overload in Delegates.cs to handle heap-allocated delegates under GC stress.
- Expanded the condition for delegate invoke expansion in objectalloc.cpp to include GT_LCL_ADDR and updated the cloning call accordingly.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tests/JIT/opt/ObjectStackAllocation/Delegates.cs | Added HeapAllocation() method and Test2 overload to support GCStress scenarios and delegate invoke expansion testing. |
| src/coreclr/jit/objectalloc.cpp | Extended condition checks to include GT_LCL_ADDR and updated the clone call to pass a complex flag for proper delegate expansion. |
Comments suppressed due to low confidence (1)
src/coreclr/jit/objectalloc.cpp:2409
- The new condition for identifying stack allocated delegates now includes GT_LCL_ADDR. Please add a clarifying comment explaining why checking GT_LCL_ADDR together with DoesLclVarPointToStack is needed in this context.
bool const isStackAllocatedDelegate = delegateThis->OperIs(GT_LCL_ADDR) || m_allocator->DoesLclVarPointToStack(lcl->GetLclNum());
| { | ||
| 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) | ||
| // |
There was a problem hiding this comment.
Passing the 'complexOk' flag as true ensures proper cloning of address nodes; consider adding a brief inline comment to detail the necessity of this flag in handling GT_LCL_ADDR nodes.
| // | |
| // | |
| // The 'complexOk' flag is set to true to ensure proper cloning of address nodes, | |
| // particularly for GT_LCL_ADDR nodes, which are crucial for delegate invocation expansion. |
Copilot uses AI. Check for mistakes.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
Failures seem unrelated, but will have to do some digging. |
|
Possibly #31719 but that is very old. |
|
/ba-g HostActivation.Tests failing without log |
Also expand the invoke when the delegate object address has been substituted directly.
Addresses example in #84872.