Contradiction in multi-memory binary format instruction rules
ashleynh opened this issue · 3 comments
Hi,
I'm implementing multi-memories in Binaryen and noticed a contradiction while following the Multiple Memories for Wasm proposal.
Under Binary format, the proposal says "For loads and stores: Reinterpret the alignment value in the memarg as a bitfield; if bit 6 (the MSB of the first LEB byte) is set, then an i32 memory index follows after the offset immediate." The problem is this ordering requires parsing the offset before the memory index. While the memory index is always i32, with the introduction of 64-bit, the offset can now be i32 or i64. Before adding multi-memories support, the code checked the index type of the memory before parsing either an i32 or i64 offset.
As a result, the following steps are now performed to follow the proposal as currently written:
- Create an i64 for the offset
- Identify the memory based on the index following the offset
- Check whether the memory index type is i32
- If step 3 is true, check if the offset is a larger number than what fits into an i32
- If step 4 is true, throw an error
- If step 4 is false, down-cast the i64 offset to an i32 offset
It would be better if the i32 memory index immediately followed the alignment instead of the offset, because then the index type would be known and the code would parse the offset accordingly. I propose changing the sentence above in bold to "then an i32 memory index follows after the alignment".
@rossberg @titzer @kmiller68 @eqrion @jkummerow - seeking thoughts & sign off, thanks!
Ah, this is an interference with the Wasm64 extension that I hadn't considered.
From first glance, it makes total sense to me to change the immediates around to make this easier. I'll try that out.
Awesome! Thanks for handling so quickly. LGTM!