-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[Wasm RyuJit] Fix emission of block and label instructions #123522
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] Fix emission of block and label instructions #123522
Conversation
We were dropping the type field.
|
@dotnet/jit-contrib PTAL |
|
Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib |
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 WebAssembly RyuJIT emission for structured control-flow instructions so that the required block type byte is emitted for block/loop.
Changes:
- Add a dedicated
emitter::emitIns_Bhelper that records theIF_BLOCKinstruction format for wasm block-like instructions. - Route wasm
INS_blockandINS_loopemission ingenEmitStartBlockthroughemitIns_Binstead of the genericinstGen, ensuring the block type byte is written. - Keep label emission via
emitIns_JandIF_RAW_ULEB128unchanged, maintaining correct encoding forbr_tablelabels.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/coreclr/jit/emitwasm.h |
Declares the new emitIns_B entry point on the emitter for wasm block-style instructions. |
src/coreclr/jit/emitwasm.cpp |
Implements emitIns_B to use IF_BLOCK, which in turn causes the encoder to output the extra block-type byte (currently fixed as void). |
src/coreclr/jit/codegenwasm.cpp |
Updates genEmitStartBlock to emit INS_block/INS_loop via emitIns_B, fixing the previously missing type field on these wasm control-flow instructions. |
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.
Looks good to me!
We were dropping the type field.