JIT: handle redefined enumerators in conditional escape analysis - #134658
Conversation
Reject cloning when a later guarded definition may reach uses assigned to an earlier definition. Fixes dotnet#134605 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ed9b45e3-0af5-4425-a189-1d0e0cbce294
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Later mapped definitions that cannot be stack allocated and missing test-runtime gating leave unresolved correctness and test reliability issues.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes JIT conditional escape analysis for redefined enumerator locals while adding class and boxed-struct regression coverage.
Changes:
- Detects conflicting enumerator redefinitions during RPO analysis.
- Rejects unsafe cloning candidates while preserving valid sequential cases.
- Adds regression and allocation tests.
| File | Summary |
|---|---|
src/tests/JIT/opt/ObjectStackAllocation/Runtime_134605.csproj |
Configures the class-enumerator regression test. |
src/tests/JIT/opt/ObjectStackAllocation/Runtime_134605.cs |
Adds enumerator reassignment and allocation scenarios. |
src/tests/JIT/opt/ObjectStackAllocation/Runtime_134605_Struct.csproj |
Configures the boxed-struct variant. |
src/coreclr/jit/objectalloc.h |
Adds clone-candidate conflict state. |
src/coreclr/jit/objectalloc.cpp |
Detects and rejects unsafe enumerator redefinitions. |
|
@EgorBo ptal Tried to keep this one fairly surgical so it can be a backport. A more "robust" solution would be to run reaching defs or similar. But probably ends up a the same place, since we can't handle allocations in loops yet. |
|
/backport to release/11.0 |
|
Started backporting to |
|
/backport to release/10.0 |
|
Started backporting to |
Fix conditional escape analysis when an enumerator local has multiple guarded definitions.
The RPO walk now rejects an earlier cloning candidate when a later definition may reach its guarded uses, while preserving dominated sequential redefinitions.
Validated with class and boxed-struct regressions, the disjoint zero-allocation case, Runtime_127075, and Runtime_131713.
Resolves #134605
Note
This pull request description was generated with GitHub Copilot.