Wasmtime GitHub notifications bot (Jun 12 2025 at 19:07): Wasmtime GitHub notifications bot (Jun 12 2025 at 19:07):fitzgen opened PR #11030 from fitzgen:wasm-types-in-translation to bytecodealliance:main:
This lets us generate better CLIF based on what we know of the Wasm types. There is a bunch we can do here, but this commit just lays down the initial infrastructure and plumbs the type info through to the
ref.is_nullimplementation. We will now avoid actually performing null check when the input to aref.is_nullinstruction is a non-nullable ref type.Note that this depends on both an eventual
wasm-toolsversion bump and that we fix a bug in the arity stuff inwasmparser, which I have not tracked down yet. (One of thedisastests currently panics because we fail to provide an arity for a valid, reachableendinstruction.)<!--
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 (Jun 12 2025 at 19:23):fitzgen edited PR #11030:
This lets us generate better CLIF based on what we know of the Wasm types. There is a bunch we can do here, but this commit just lays down the initial infrastructure and plumbs the type info through to the
ref.is_nullimplementation. We will now avoid actually performing null check when the input to aref.is_nullinstruction is a non-nullable ref type.Note that this depends on a fix for https://github.com/bytecodealliance/wasm-tools/issues/2235 and then a new
wasm-toolsrelease.<!--
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 (Jun 13 2025 at 15:57):fitzgen commented on PR #11030:
With https://github.com/bytecodealliance/wasm-tools/pull/2237, this is now passing all tests locally, so I'll cut a wasm-tools release, update those crates in this repo, and then this PR can rebase and merge.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:06):fitzgen commented on PR #11030:
wasm-tools dep update in https://github.com/bytecodealliance/wasmtime/pull/11034
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:23):fitzgen updated PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:23):fitzgen has marked PR #11030 as ready for review.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:23):fitzgen requested alexcrichton for a review on PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:23):fitzgen requested wasmtime-core-reviewers for a review on PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:23):fitzgen requested wasmtime-default-reviewers for a review on PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:43):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:43):alexcrichton created PR review comment:
Mind leaving a comment that the option-ality is due to the fact that we're not certain that
opis valid yet? In that if it's invalid the arity may beNone. Once we get pastvalidator.op, though, then it's known thatoperand_typesis valid. Although we also can't move this block becausevalidator.opmutates the validator.Also, can this
.unwrap()after the validator to ensure that the signatures of the before/after/translate all take a slice, not an optional slice?
Wasmtime GitHub notifications bot (Jun 13 2025 at 16:43):alexcrichton created PR review comment:
There's a TODO just above where I'm commenting here which can be resolved now
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:12):fitzgen created PR review comment:
Also, can this
.unwrap()after the validator to ensure that the signatures of the before/after/translate all take a slice, not an optional slice?Unfortunately not because even if the op is valid, if it is in unreachable code, the op might want to pop more values from the stack than actually exist on the stack (which is allowed in unreachable code) so even if we can get arity, we are only guaranteed to have operand types for ops that are not only valid but also reachable.
We could instead pass
&[Option<WasmValType>]through, which would only ever containNones inside valid-but-unreachable code, but I think the ergonomics are even worse with this approach.I've added comments explaining this however.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:12):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:15):fitzgen requested cfallin for a review on PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:15):fitzgen requested wasmtime-compiler-reviewers for a review on PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:15):fitzgen updated PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:15):fitzgen has enabled auto merge for PR #11030.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:19):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:19):alexcrichton created PR review comment:
Ah ok makes ense, and yeah I don't think
&[Option<_>]is correct here, so threading around option but documenting the unreachable-ness sounds good to me.
Wasmtime GitHub notifications bot (Jun 13 2025 at 17:50):fitzgen merged PR #11030.
Last updated: Dec 06 2025 at 07:03 UTC