Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: enhance RBO inference for similar compares to constants #111766

Merged
merged 2 commits into from
Jan 25, 2025

Conversation

AndyAyersMS
Copy link
Member

Thanks to #95234, RBO can draw inferences when the same value is compared to different constants, if the initial comparison dominates and was false. Generalize this to also handle cases where the initial comparison dominates and is true.

Fixes #111725.

Thanks to dotnet#95234, RBO can draw inferences when the same value is compared to
different constants, if the initial comparison dominates and was false. Generalize this
to also handle cases where the initial comparison dominates and is true.

Fixes dotnet#111725.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 23, 2025
@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

Should be a few hundred diffs.

@AndyAyersMS
Copy link
Member Author

Seeing some failures, so evidently not quite so simple.

@AndyAyersMS
Copy link
Member Author

Egor narrowed this down to System.Uri:CheckCanonical where the new inference enables jump threading since there's a fully redundant dominating branch:

Dominator BB07 of BB10 can infer value of dominated relop
N003 (  3,  3) [000044] J----+-N---                         *  LT        int    <l:$356, c:$357>
N001 (  1,  1) [000042] -----+-----                         +--*  LCL_VAR   int    V09 loc4         u:1 <l:$352, c:$353>
N002 (  1,  1) [000043] -----+-----                         \--*  CNS_INT   int    127 $4c
 Redundant compare; current relop:
N003 (  3,  3) [000048] J----+-N---                         *  LE        int    <l:$35a, c:$35b>
N001 (  1,  1) [000046] -----+-----                         +--*  LCL_VAR   int    V09 loc4         u:1 <l:$352, c:$353>
N002 (  1,  1) [000047] -----+-----                         \--*  CNS_INT   int    126 $4e
Both successors of idom BB07 reach BB10 -- attempting jump threading
BB07 is a false pred
BB08 is a true pred
Optimizing via jump threading
Jump flow from pred BB07 -> BB10 implies predicate false; we can safely redirect flow to be BB07 -> BB11
Jump flow from pred BB08 -> BB10 implies predicate true; we can safely redirect flow to be BB08 -> BB

and jump threading makes the wrong inference, because it's not used to seeing inferred relops (apparently)...

@AndyAyersMS
Copy link
Member Author

Going to refactor RBO a bit so rii is always fully populated and clients can rely on rii->reverseSense in all cases. Will PR that separately as a no-diff change.

AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Jan 24, 2025
Update clients to rely on `reverseSense` rather than parsing `vnRelation`.

Also revised the and/or inference slightly as I found it hard to follow;
hopefully it's clearer now.

Fixes an issue that came up in testing dotnet#111766.
@AndyAyersMS
Copy link
Member Author

Waiting on #111803

AndyAyersMS added a commit that referenced this pull request Jan 24, 2025
)

Update clients to rely on `reverseSense` rather than parsing `vnRelation`.

Also revised the and/or inference slightly as I found it hard to follow;
hopefully it's clearer now.

Fixes an issue that came up in testing #111766.
@AndyAyersMS
Copy link
Member Author

@EgorBo I fixed the issue, PTAL

@AndyAyersMS
Copy link
Member Author

Diffs

Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

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

Nice!

@AndyAyersMS AndyAyersMS merged commit 06c3929 into dotnet:main Jan 25, 2025
110 of 112 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: Implied branch is not removed
2 participants