Uh oh!
There was an error while loading. Please reload this page.
[release/8.0-staging] Fix loop hoist memory-dependence tracking - #128381
Merged
JulieLeeMSFT merged 3 commits intoMay 20, 2026
Merged
Conversation
…et#122057) Port the .NET 9 fix in `optRecordLoopMemoryDependence` so that memory dependencies coming from nested loops are recorded against the closest ancestor loop that contains the consuming tree, rather than being silently discarded. Without this fix, when the loop hoister considers an IND whose value VN has been folded to a constant via VN's MapStore/MapSelect machinery, the `IsTreeLoopMemoryInvariant` check finds no entry in `NodeToLoopMemoryBlockMap` and returns `true` vacuously. The IND is then relocated to the loop preheader where it loads a stale value instead of the post-store value the VN-folded constant represents. Example pattern from the linked issue: ```csharp for (sbyte i = -126; i > -128; i--) { for (byte j = 2; j > 0; j--) { s_rt.Checksum(""c_0"", var4); // virtual call inside inner loop } s_2 = 1; s_2 = s_2--; // hoisted IND reads stale s_2 } ``` Fix: walk up the parent chain of the update loop until one is found that contains the tree's block (the .NET 9 `FlowGraphNaturalLoops` version generalizes this; the 8.0 port uses `optLoopTable[..].lpParent`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AndyAyersMS
commented
May 19, 2026
MemberAuthor
@jakobbotsch PTAL |
Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Backport to release/8.0-staging that fixes JIT loop-hoisting safety by correctly tracking loop memory dependencies across nested/sibling loops, preventing invalid LICM that can lead to silent bad codegen. Adds a regression test covering the reported scenario.
Changes:
- Update
optRecordLoopMemoryDependenceto walk up the update loop’s parent chain to find an ancestor loop that encloses the consumer loop before discarding the dependence. - Add a new JIT regression test (
Runtime_122057) and project file, forcing FullOpts viaDOTNET_TieredCompilation=0.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/jit/optimizer.cpp | Fixes loop memory-dependence recording by resolving dependence to an enclosing ancestor loop when needed. |
| src/tests/JIT/Regression/JitBlue/Runtime_122057/Runtime_122057.csproj | Adds a new isolated JIT test project and sets runtime env to reproduce FullOpts behavior. |
| src/tests/JIT/Regression/JitBlue/Runtime_122057/Runtime_122057.cs | Adds a reduced repro + checksum-based validation for the loop-hoisting memory dependence bug. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Remove unused System.Collections.Generic and System.Threading usings. - Remove non-English section-header comments from the Fuzzlyn-derived repro; replace the runtime class header with an English one-liner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jakobbotsch
approved these changes
May 19, 2026
JulieLeeMSFT
approved these changes
May 19, 2026
JulieLeeMSFT
commented
May 19, 2026
Member
Fixes #122057. |
JulieLeeMSFT
commented
May 20, 2026
Member
@AndyAyersMS, please check build failures. |
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 freeto 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.
Manual backport of #106076 to release/8.0-staging
Customer Impact
#122057
Silent bad code caused by invalid loop invariant code motion. An error in tracking memory dependence can allow the JIT to hoist an expression that is not loop invariant out of a loop.
While customer reported, the issue was found by Fuzzlyn.
The known repro cases require a two-level loop nest, with an expression that is VN invariant (eg a constant) but memory dependent in the outerloop sitting below an inner loop.
Regression
Testing
Validated the repro case now passes. The issue was fixed in 9.0, though the fix had a slightly different form.
Risk
Low. This make the JIT more conservative about optimizing some constructs. No SPMI diffs for the 9.0 version of the fix; no SPMI diffs for libraries with the current fix.