-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Ignore fallthrough blocks in optRecognizeAndOptimizeSwitchJumps #118614
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a codegen issue in the JIT's switch recognition optimization by addressing two problems: phase ordering and handling of empty fallthrough blocks. The optimization was running too late in the compilation pipeline, after the "optimize bools" phase had already converted conditional jumps back to returns, making switch recognition ineffective.
Key changes:
- Moved switch recognition phase to run before boolean optimization phase
- Added logic to skip empty BBJ_ALWAYS fallthrough blocks during switch pattern matching
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/coreclr/jit/switchrecognition.cpp | Added SkipFallthroughBlocks helper function and updated target resolution to handle empty fallthrough blocks |
src/coreclr/jit/compiler.cpp | Moved PHASE_SWITCH_RECOGNITION to run before PHASE_OPTIMIZE_BOOLS |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
PTAL @dotnet/jit-contrib fixes an optimization that someone broke (see PR desc). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes an optimization that someone broke (see PR desc).
That was probably me. Thanks for fixing this!
@AndyAyersMS I've addressed your feedback, looks like Aman's approval is no longer green, so can you approve? thanks |
Fixes a codegen issue @stephentoub noticed in #107499
There were 2 issues:
fgUpdateFlowGraph
just because of them so I applied a fix here.Diffs