Skip to content

JIT: don't use a jump-threaded block's sharpened predicate for dominator-based inference - #132281

Open
EgorBo wants to merge 1 commit into
dotnet:mainfrom
EgorBo:fix-130700-rbo-stale-predicate
Open

JIT: don't use a jump-threaded block's sharpened predicate for dominator-based inference#132281
EgorBo wants to merge 1 commit into
dotnet:mainfrom
EgorBo:fix-130700-rbo-stale-predicate

Conversation

@EgorBo

@EgorBo EgorBo commented Aug 13, 2026

Copy link
Copy Markdown
Member

Fixes #130700 bug

When RBO jump threads through a block, it reroutes some of the block's preds directly to the block's successors. If the block is left with a single (ambiguous) pred, optJumpThreadCore sharpens the block's predicate VN to the value flowing in from that pred. The preds that were rerouted, however, were classified against the old VN, so the sharpened predicate does not hold on the paths that now bypass the block.

Dominator info is not updated as we thread, so the bypassed block still looks like a dominator of its successors, and optRedundantBranch could use its sharpened predicate to fold a branch in a block that is also reachable via the rerouted edges. In the reported case this removed the null check on an isinst result:

       mov      r14, rbx                       ; r14 = obj (non-null)
       cmp      qword ptr [r14], <MergeHierarchy MT>
       jne      SHORT G_M49973_IG17
       xor      r14, r14                       ; obj is MergeHierarchy => (obj as MergeFile) == null
       mov      dword ptr [rbp+0x20], 0        ; MemberType = MergeFile  <-- wrong arm, null check gone
       mov      rbx, gword ptr [r14+0x08]      ; <-- NullReferenceException

Fix: flag such blocks with BBF_STALE_PREDICATE and skip them in the two dominator-based inference walks in this phase.

No SPMI asm diffs

…tor-based inference

When RBO jump threads through a block, it reroutes some of the block's preds
directly to the block's successors. If the block is left with a single
(ambiguous) pred, optJumpThreadCore sharpens the block's predicate VN to the
value flowing in from that pred. The preds that were rerouted, however, were
classified against the *old* VN, so the sharpened predicate does not hold on
the paths that now bypass the block.

Dominator info is not updated as we thread, so the bypassed block still looks
like a dominator of its successors, and optRedundantBranch could use its
sharpened predicate to fold a branch in a block that is also reachable via the
rerouted edges. In the reported case this removed the null check on an isinst
result, so an "is MergeFile" arm was entered with a null value.

Flag such blocks with BBF_STALE_PREDICATE and skip them in the two
dominator-based inference walks in this phase.

Fixes dotnet#130700

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 049f4632-06bf-47b3-810f-437d77d5938d
Copilot AI lite review requested due to automatic review settings August 13, 2026 16:04
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 13, 2026
@azure-pipelines

Copy link
Copy Markdown
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.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a JIT miscompilation hazard in Redundant Branch Optimization (RBO) where jump-threading can “sharpen” a block’s predicate VN in a way that no longer holds on all paths (due to rerouted edges), yet dominator-based inference can still incorrectly treat it as a dominating predicate.

Changes:

  • Introduces a new BasicBlock flag (BBF_STALE_PREDICATE) to mark predicates that became path-specific after jump threading.
  • Updates RBO dominator-based inference to ignore (or bail out on) dominators marked with BBF_STALE_PREDICATE, and clears the flag at the end of the phase when changes occur.
  • Adds a regression test (Runtime_130700) configured to reproduce the issue under tiered compilation + PGO.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/coreclr/jit/redundantbranchopts.cpp Marks sharpened-predicate blocks as stale and prevents dominator-based inference from using those stale predicates during RBO.
src/coreclr/jit/block.h Adds the BBF_STALE_PREDICATE flag definition.
src/coreclr/jit/block.cpp Adds display support for the new block flag in debug dumps.
src/tests/JIT/Regression/JitBlue/Runtime_130700/Runtime_130700.cs New regression test exercising the problematic control-flow + inference pattern.
src/tests/JIT/Regression/JitBlue/Runtime_130700/Runtime_130700.csproj Test project wiring, including process isolation and enabling tiered compilation + PGO via environment variables.

@EgorBo

EgorBo commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@AndyAyersMS Could you please take a look? jump threading bug. I tried to make the fix surgical for a potential backport. No diffs cc @dotnet/jit-contrib

@AndyAyersMS AndyAyersMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

We could probably scrutinize PHIs vs preds and reach the same conclusion, but a flag seems reasonable too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamic PGO miscompilation: elided null check produces NullReferenceException

3 participants