JIT: relax constraint in conditional escape analysis#117222
Merged
AndyAyersMS merged 1 commit intodotnet:mainfrom Jul 2, 2025
Merged
JIT: relax constraint in conditional escape analysis#117222AndyAyersMS merged 1 commit intodotnet:mainfrom
AndyAyersMS merged 1 commit intodotnet:mainfrom
Conversation
Enable conditional escape analysis if the allocation was at one time guarded by a GDV, even if that GDV gets resolved by the new GDV cleanup pass that runs before escape analysis. Fixes dotnet#117204.
github-actions
bot
added
the
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR relaxes the constraint for conditional escape analysis by tracking allocations that were once guarded by a GDV even after the guard has been cleaned up.
- Removed the
m_domBlockmember and its assignments fromCloneInfo - Eliminated the
IsGuardedcheck so allGT_ALLOCOBJallocations are now unconditionally tracked - Cleaned up related debug dumps and comments
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/objectalloc.h | Removed m_domBlock field from CloneInfo |
| src/coreclr/jit/objectalloc.cpp | Removed guard check (IsGuarded), always track allocations, and cleaned up info->m_domBlock assignment and debug dump |
Comments suppressed due to low confidence (3)
src/coreclr/jit/objectalloc.cpp:3320
- [nitpick] The comment still references GDV but the logic now tracks all allocations unconditionally. Consider updating it to reflect that allocations are always tracked regardless of any guard.
// This may be an allocation of a concrete class under GDV.
src/coreclr/jit/objectalloc.cpp:3317
- The change relaxes escape analysis constraints but introduces new behavior paths. Please add or update unit tests to cover allocations that were previously skipped when the guard was removed.
//
src/coreclr/jit/objectalloc.cpp:3320
- [nitpick] By removing the guard check, every
GT_ALLOCOBJgoes through the cloning logic, which could increase compile-time work. Consider benchmarking or adding heuristics to avoid unintended performance regressions.
// This may be an allocation of a concrete class under GDV.
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Member
Author
|
PTAL @dotnet/jit-contrib Because of the way Enumerator ALLOCOBJ flagging works we don't need to verify the allocation is currently guarded by a GDV; it only gets flagged when (in earlier phases) it was guarded. |
amanasifkhalid
approved these changes
Jul 2, 2025
AndyAyersMS
added a commit
to AndyAyersMS/runtime
that referenced
this pull request
Jul 4, 2025
Proceed with CEA cloning even when the allocation site dominates the assignment to the enumerator var. The cloning will be a bit wasteful as the original code will become unreachable, but the code is set up to specialize the clone and so this is the less risky fix. Also restore recognition of `IEnumerable<T>.GetEnumerator` as this gives a useful inlining boost in some cases. Fixes dotnet#117204 cases that were not fixed by dotnet#117222.
AndyAyersMS
added a commit
that referenced
this pull request
Jul 7, 2025
Proceed with CEA cloning even when the allocation site dominates the assignment to the enumerator var. The cloning will be a bit wasteful as the original code will become unreachable, but the code is set up to specialize the clone and so this is the less risky fix. Also restore recognition of `IEnumerable<T>.GetEnumerator` as this gives a useful inlining boost in some cases. Fixes #117204 cases that were not fixed by #117222.
This was referenced Jul 8, 2025
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable conditional escape analysis if the allocation was at one time guarded by a GDV, even if that GDV gets resolved by the new GDV cleanup pass that runs before escape analysis.
Fixes #117204.