Open sbc100 opened 2 months ago
Looks good, although I wouldn't consider wabt a blocker for phase 4 if we have support in Binaryen.
My understanding of ocaml is shaky at best but by my read the current implementation of the binary encoding of a table type is to use the third bit of the prefix byte is used to indicate whether the index type is i64 (where if it's 0 then it's i32, as-is today). This makes sense for memories where the second bit was already claimed for the shared
bit by the threads proposal, but I wanted to raise a question here if this is the desired encoding for tables. That means that the second bit in the table type prefix byte is currently unused.
Given the shared-everything-threads proposal it seems like we'll get a shared
table type one day so thinking about this I think it might make sense to leave the hole, but I wanted to be sure to flag this and ensure that it was intentional and not accidental (e.g. I can see how it would be easy to just reuse what was already in the interpreter)
Yes, the idea is tie match the flags that we use for memories.
For example, in wabt we have: https://github.com/WebAssembly/wabt/blob/356931a867c7d642bc282fff46a1c95ab0e843f3/include/wabt/binary.h#L24-L29 and these flags are used for both memories and tables. It makes sense to me to re-use these, even though we don't currently have shared tables. Are there downsides to doing this that you are thinking of?
I don't think there's any downside to doing this, no. In Wasmtime we don't parse limits in the same way between tables/memories because for so long the encoding of tables/memories has diverged - for example for quite some times memories can be shared or 64-bit but tables can't be. Even with the table64 extension and a hypothetical shared table type the custom-page-sizes proposal still has different parsing of the flags byte which won't ever get mapped to tables.
Basically while the parsing of limits was trivially the same between memories/tables I don't think we should take it as a design guideline that they must match for all future proposals as well. The 64-bit-ness and shared-ness makes sense to match since we might as well, though.
Unless there is a strong reason to do otherwise (like running out of bits), I think it's preferable to keep them in sync. (We have been doing the same for elem/data segments.)
Now that #46 is decided we need get this extension implemented in various places:
External places: