eqrion opened PR #1362 from sm/ref-2
to master
:
The reftypes proposal has introduced a typed select instruction to ensure efficient validation in the future with complex subtyping relationships. In addition, a proper 'nullref' type has been added. Both of these changes are supported in
wasmparser
but there are some additional updates needed tocranelift-wasm
.
eqrion requested yurydelendik for a review on PR #1362.
fitzgen submitted PR Review.
fitzgen created PR Review Comment:
We should also add a test like
(func (param anyref) (result anyref) ref.null local.get 0 i32.const 1 select (result anyref))to exercise when the values are distinct subtypes of the typed select's result type. Would be even better if we had enough support for
ref.func
that we could have turn either anullref
or afuncref
into ananyref
but I don't think we are there yet, are we?
fitzgen submitted PR Review.
fitzgen created PR Review Comment:
Just to make sure I understand correctly: the reason we don't care about
ty
is because we already validated the wasm (either in wasmtime or spidermonkey) and know that it is a supertype of botharg1
andarg2
, and thebuilder.ins().select(..)
just treats them all reference types asr32
orr64
, right?
eqrion created PR Review Comment:
Yes,
wasmparser
itself contains validation code which will check thatarg1
andarg2
are subtypes of the specified type. Spidermonkey uses its own equivalent code. And after validation, there's not a need for a stricter type in Cranelift IR, so it all becomes R32/R64.I'll add a comment to this effect in the code here.
eqrion submitted PR Review.
eqrion submitted PR Review.
eqrion created PR Review Comment:
Good idea, I'll add that test.
I'm not sure if we can test
funcref
well. Right now the DummyEnvironment used in wasmtests will convert a wasmref.func
into a clifr32/64.null
. So while it would test wasmparser's validation (which has it's own tests), the CL IR would be the same as the other test. (that's if I understand the purpose of wasmtests correctly)
eqrion updated PR #1362 from sm/ref-2
to master
:
The reftypes proposal has introduced a typed select instruction to ensure efficient validation in the future with complex subtyping relationships. In addition, a proper 'nullref' type has been added. Both of these changes are supported in
wasmparser
but there are some additional updates needed tocranelift-wasm
.
eqrion merged PR #1362.
Last updated: Dec 23 2024 at 13:07 UTC