Add support for generalized variable length wasm instructions to the emitter#125862
Add support for generalized variable length wasm instructions to the emitter#125862davidwrighton wants to merge 2 commits intodotnet:mainfrom
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR updates the WASM JIT instruction encoding to support spec-style prefixed opcodes with variable-length (ULEB128) sub-opcodes, enabling instruction definitions to match the WebAssembly opcode format rather than a packed 2-byte representation.
Changes:
- Introduce an
INST2(prefix, opcode)instruction-table form for WASM prefixed instructions. - Update the WASM emitter to output prefixed opcodes as
prefix-byte + ULEB128(subopcode)and size them accordingly. - Wire
INST2through existing instruction enum/name/info/format/opcode tables.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/jit/instrswasm.h | Adds INST2 usage for prefixed WASM instructions (e.g., 0xFC-prefixed ops) and enforces INST2 definition. |
| src/coreclr/jit/instr.h | Extends WASM instruction enum generation to accept INST2. |
| src/coreclr/jit/instr.cpp | Extends WASM instruction name table generation to accept INST2. |
| src/coreclr/jit/emitwasm.cpp | Adds prefix/subopcode tables and emits prefixed opcodes as prefix + ULEB128; updates sizing and an emission site to use the new opcode output helper. |
AndyAyersMS
left a comment
AndyAyersMS
left a comment
There was a problem hiding this comment.
LGTM, just one comment you might consider.
| // control flow | ||
| // | ||
| INST(invalid, "INVALID", 0, IF_NONE, BAD_CODE) | ||
| INST2(invalid, "INVALID", 0, IF_NONE, 0xFC, BAD_CODE) |
There was a problem hiding this comment.
As is this 0xFC nnnn won't ever get emitted, either we need to change this to specify IF_OPCODE (and rework some internal asserts to catch this case) or else the value doesn't matter.
My previous fix only made 2 byte instructions work, but in theory wasm can have larger variable sized instructions. This change allows us to define the instructions in the same format that the spec defines them, and supports emitting any of them (as long as the variable sized opcode fits in a uint16_t which should be good enough for now.