Skip to content

JIT: Use normalized block for comparison in RBO dominator walk#127434

Merged
AndyAyersMS merged 1 commit intodotnet:mainfrom
hez2010:rbo-fix
Apr 26, 2026
Merged

JIT: Use normalized block for comparison in RBO dominator walk#127434
AndyAyersMS merged 1 commit intodotnet:mainfrom
hez2010:rbo-fix

Conversation

@hez2010
Copy link
Copy Markdown
Contributor

@hez2010 hez2010 commented Apr 26, 2026

Fixes an issue where we bail out early in the RBO dominator walk due to comparing a successor that was skipped through empty BBJ_ALWAYS blocks against the current block.

The jit dump was like

Checking BB04 for redundant dominating branches
BB03 and BB04 have shared successor BB06
Optimizing branch in dominating BB03 with relop [000011] based on BB04's relop [000015]

Redundant dominating branch opt in BB03:

removing useless STMT00003 ( 0x008[E--] ... 0x00A )
N004 (  5,  5) [000012] -----------                         ▌  JTRUE     void   $VN.Void
N003 (  3,  3) [000011] -----------                         └──▌  CNS_INT   int    0
 from BB03

BB03 becomes empty
setting likelihood of BB03 -> BB04 from 0.5 to 1

Conditional folded at BB03
BB03 becomes a BBJ_ALWAYS to BB04
Compiler::optRedundantDominatingBranch removed tree:
N004 (  5,  5) [000012] -----------                         ▌  JTRUE     void   $VN.Void
N003 (  3,  3) [000011] -----------                         └──▌  CNS_INT   int    0

continuing to the next immediate dominator
failed -- BB02 is degnerate

after BB03 is folded into an empty BBJ_ALWAYS to BB04, we continue walking to the next dominator BB02, but when checking BB02, we skipped side effect free blocks on its successors so that we end up comparing BB04 against BB03.

Codegen diff:

 G_M29168_IG01:  ;; offset=0x0000
        sub      rsp, 40
                                                 ;; size=4 bbWeight=0.50 PerfScore 0.12
 G_M29168_IG02:  ;; offset=0x0004
-       cmp      ecx, 2
-       jle      SHORT G_M29168_IG03
        cmp      ecx, 8
        jle      SHORT G_M29168_IG03
        mov      ecx, 1
        call     [System.Console:WriteLine(int)]
-                                                ;; size=21 bbWeight=0.50 PerfScore 2.88
+                                                ;; size=16 bbWeight=0.50 PerfScore 2.25
-G_M29168_IG03:  ;; offset=0x0019
+G_M29168_IG03:  ;; offset=0x0014
        nop
                                                 ;; size=1 bbWeight=1 PerfScore 0.25
-G_M29168_IG04:  ;; offset=0x001A
+G_M29168_IG04:  ;; offset=0x0015
        add      rsp, 40
        ret
                                                 ;; size=5 bbWeight=1 PerfScore 1.25

Fixes #127432

/cc: @AndyAyersMS

Copilot AI review requested due to automatic review settings April 26, 2026 14:12
@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 Apr 26, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 26, 2026
@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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an RBO dominator-walk edge case in optRedundantDominatingBranch where the walk can bail out after prior folding creates an empty BBJ_ALWAYS chain, by ensuring comparisons are done against the normalized (side-effect-free-skipped) “current” block.

Changes:

  • Normalize currentBlock via skipSideEffectFreeBlocks before comparing it against dominator successors in the RBO dominator walk.
  • Add a new dominating-branch test case (Dom_05) to exercise the reported control-flow shape.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/jit/redundantbranchopts.cpp Normalizes the block being compared in the dominator walk to avoid mismatches caused by empty BBJ_ALWAYS chains.
src/tests/JIT/opt/RedundantBranch/RedundantBranchDominating.cs Adds a new nested-dominating-branch scenario (Dom_05) and a corresponding test.

Comment thread src/tests/JIT/opt/RedundantBranch/RedundantBranchDominating.cs
@hez2010
Copy link
Copy Markdown
Contributor Author

hez2010 commented Apr 26, 2026

Diffs

Copy link
Copy Markdown
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this.

@AndyAyersMS AndyAyersMS merged commit 985b176 into dotnet:main Apr 26, 2026
153 of 154 checks passed
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 community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redundant branch not being removed cleanly

3 participants