-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[Wasm RyuJit] Slightly smaller code for switches #122198
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
[Wasm RyuJit] Slightly smaller code for switches #122198
Conversation
Wasm switches (`br_table`) have a default case, so take advantage of this and stop peeling off the default case in lower.
|
@dotnet/jit-contrib PTAL |
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 optimizes WebAssembly switch statement code generation by leveraging the native default case support in Wasm's br_table instruction. Previously, the JIT would peel off the default case during lowering; this change preserves it for Wasm targets, resulting in smaller generated code.
Key changes:
- Modified switch lowering to skip default case peeling for Wasm targets
- Updated assertions and comments throughout to reflect that Wasm switches now retain their default cases
- Adjusted optimization passes to handle the different switch IR representation on Wasm (GT_SWITCH vs GT_SWITCH_TABLE)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/jit/lower.cpp | Added early return for TARGET_WASM in LowerSwitch to prevent default case peeling |
| src/coreclr/jit/fgwasm.cpp | Updated comments and changed assertion from !HasDefaultCase() to HasDefaultCase() to reflect new behavior |
| src/coreclr/jit/fgopt.cpp | Added TARGET_WASM conditional for switchTree operator assertion (GT_SWITCH vs GT_SWITCH_TABLE) |
| src/coreclr/jit/codegenwasm.cpp | Updated assertion to expect default case and added assertion for empty switch fallthrough case |
adamperlin
left a comment
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.
This looks good to me! It's neat we can completely map GT_SWITCH -> br_table
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Wasm switches (
br_table) have a default case, so take advantage of this and stop peeling off the default case in lower.