Wasmtime GitHub notifications bot (Dec 18 2024 at 18:51): Wasmtime GitHub notifications bot (Dec 18 2024 at 18:51):alexcrichton opened PR #9854 from alexcrichton:shuffle-pulley-opcodes to bytecodealliance:main:
In the near future the set of opcodes here are going to be expanded along a number of axes such as:
- More modes of addressing loads/stores other than just
reg + offset32.- Opcodes with immediate operands rather than unconditional register operands.
The 1-byte opcode namespace is already filling up and there's a bit of a mishmash of what's 1-byte and what's "extended" for now. To help bridge this gap in the interim shuffle all float/vector-related opcodes into the "extended" opcode namespace. This frees up a large chunk of the 1-byte opcode namespace for future expansion with extensions like above.
I'll note that I'm not 100% sure that the opcodes will all stay here after this reshuffling. I haven't done any profiling/performance analysis to gauge the impact of this change. The immediate goal is to start experimenting with non-float/vector programs and get them profiling well. This will require more "macro opcodes" such as new addressing modes and opcodes-with-immediates. These are expected to be relatively hot opcodes and thus probably want to be in the "fast" 1-byte namespace, hence the shuffling here.
My plan is to in the future do a bit of an evaluation with a float/vector program and see whether it make sense to shuffle some of them into this 1-bytecode space as well. More radically it might make sense to remove the split between ops/extended ops and instead just have a 2-byte opcode space entirely. That's all left for future evaluations though.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Dec 18 2024 at 18:51):alexcrichton requested fitzgen for a review on PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 18:51):alexcrichton requested wasmtime-default-reviewers for a review on PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 18:51):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 19:23):alexcrichton updated PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 19:29):alexcrichton updated PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 20:45):github-actions[bot] commented on PR #9854:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "pulley"Thus the following users have been cc'd because of the following labels:
- fitzgen: pulley
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Dec 18 2024 at 20:53):fitzgen commented on PR #9854:
More radically it might make sense to remove the split between ops/extended ops and instead just have a 2-byte opcode space entirely. That's all left for future evaluations though.
Yeah, I suspect this is probably what we'll be forced to do eventually. I liked the 1-byte vs extended ops idea in theory, but I think it just isn't working out in practice, given how many opcodes we need to support.
Wasmtime GitHub notifications bot (Dec 18 2024 at 20:55):fitzgen submitted PR review:
:+1:
Wasmtime GitHub notifications bot (Dec 18 2024 at 20:55):fitzgen created PR review comment:
Leftover from debugging or something?
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:10):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:10):alexcrichton created PR review comment:
Ah no this was intentional. The recursive definition required upping
#[recursion_limit]which I'd prefer to not do. In lieu of fanciermacro_rules!features which will make this nicer in the future I opted to have a chain of1 + 1 + ...for each opcode name. That requires using$namein the repetition somewhere though so I came up with this funky way of always evaluating to1but still using$namesomehow.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:12):alexcrichton commented on PR #9854:
I'm waffling on whether we want the 1-byte opcode namespace myself. I'm starting to lean towards "yes" so in the end I think we will want benchmarking one way or another (or size analysis) to show this.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:23):alexcrichton requested pchickey for a review on PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:23):alexcrichton requested wasmtime-core-reviewers for a review on PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:23):alexcrichton updated PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:23):alexcrichton has enabled auto merge for PR #9854.
Wasmtime GitHub notifications bot (Dec 18 2024 at 21:52):alexcrichton merged PR #9854.
Last updated: Dec 23 2024 at 12:05 UTC