[release/10.0] JIT: fix widened IV init placed in unreachable block#130324
Open
EgorBo wants to merge 1 commit into
Open
[release/10.0] JIT: fix widened IV init placed in unreachable block#130324EgorBo wants to merge 1 commit into
EgorBo wants to merge 1 commit into
Conversation
Fixes dotnet#130189. `optWidenPrimaryIV` initializes the widened IV in the block that holds the narrow IV's reaching def when that block is not the preheader. Redundant branch opts' dominator-based jump threading can bypass that def block on the path that reaches the loop while its SSA def still reaches the loop (stale SSA). The widened IV init was then emitted into that (now dead) block, which a later flow-graph pass removes, so the widened IV was never initialized and the loop produced a wrong result. Fix: only use the reaching-def block for the init when the reaching def is **not** a PHI. RBO's jump threading only leaves stale SSA at join points (PHIs), so a PHI reaching def is exactly the case that can no longer reach the loop; a non-PHI def always dominates its uses and therefore reaches the loop. When the reaching def is a PHI we fall back to the preheader, which always reaches the loop. A DFS-reachability check (`m_dfsTree->Contains`) is not sufficient here: a PHI's block can remain reachable via another path while the path that reaches the loop was threaded away, so it would still be selected even though it no longer reaches the loop. The PHI-def condition does not have that hole. Notes: - The root cause is stale SSA left by RBO dominator-based jump threading. Making RBO bail in that case is correct but pessimizes many cases where the stale SSA is harmless, so this instead hardens the only consumer that trips on it (IV widening). - SPMI (x64) shows this only moves a handful of widened-IV inits into the preheader: +85 bytes over ~676K contexts (libraries.pmi/crossgen2/aspnet2/realworld), mostly neutral placement churn, some improvements. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com> (cherry picked from commit 5f44651)
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Backport of a JIT wrong-code fix for IV widening where the widened IV initialization could be placed into a block that redundant branch opts (RBO) later makes unreachable due to stale SSA at PHIs, causing the widened IV to never be initialized. Adds a focused regression test for #130189.
Changes:
- Harden
optWidenPrimaryIVto only choose the narrow IV’s reaching-def block for widened-IV initialization when that reaching def is not a PHI; otherwise fall back to the loop preheader. - Add a new JIT regression test project
Runtime_130189that reproduces the miscompile and asserts the correct result. - Add the corresponding minimal csproj for building the regression test with optimizations enabled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/inductionvariableopts.cpp | Avoid emitting widened-IV init into a potentially-dead PHI-def block by falling back to the always-reaching preheader when the reaching def is a PHI. |
| src/tests/JIT/Regression/JitBlue/Runtime_130189/Runtime_130189.cs | New regression test asserting correct behavior for the miscompile scenario in #130189. |
| src/tests/JIT/Regression/JitBlue/Runtime_130189/Runtime_130189.csproj | Adds the test project configuration (optimized build) for the new regression. |
jakobbotsch
approved these changes
Jul 7, 2026
This was referenced Jul 8, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport of #130190 to release/10.0
/cc @EgorBo
Customer Impact
Bad codegen. IV widening could emit the widened IV initialization into a block that redundant branch opts' jump threading had made unreachable (stale SSA). A later flow-graph pass removes that dead block, so the widened IV is never initialized and the loop computes a wrong result (#130189).
Regression
Latent wrong-code bug introduced in .NET 9
Testing
Added regression test
Runtime_130189(fails without the fix, passes with it).Risk
Low
Note
This PR description was generated with the help of GitHub Copilot.