As our first producer for this new feature (\o/), Alex noticed we're currently deviating from the spec (at least overview.md) in not having any memory/table index immediates. Right now things just work b/c there's only 1 memory/table, but in the multi-memory/table future, we need the index to say which one. Practically, we'll always just encode 0 and fail validation if it's not 0 (or even if it is zero and there is no memory/table section).
Another slight devation I think is that the spec says (for memory.init): "The instruction also has three i32 operands: an offset into the source segment, an offset into the target memory, and a length to copy" I think in Spidermonkey though right now the order of arguments is [destination offset, source offset, length] instead of [source, destination, length]?
(Also FWIW the debugger seems to crash or at least always show a completely blank pane for trying to look at the disassembly of wasm which contains a memory.init/memory.drop instruction)
(In reply to Alex Crichton [:acrichto] from comment #1) > I think in Spidermonkey though right now the order of arguments is > [destination offset, source offset, length] instead of [source, destination, > length]? The spec was changed so as to consistently have destinations as the leftmost argument, a la C memcpy. See https://github.com/WebAssembly/bulk-memory-operations/issues/24
QA Contact: ajones
QA Contact: ajones
Ah, that was an oversight. In bug 1494231 I added memory indices to "active" data and element segments; this is the other side of that coin, where instructions that process passive segments must themselves hold the index.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Priority: -- → P3
Simple patch: encode a zero byte when translating from text; read a byte and check that it is zero when processing binary; adjust a test case for the new binary encoding.
Attachment #9017824 - Flags: review?(jseward)
Comment on attachment 9017824 [details] [diff] [review] bug1496582-mem-table-flag.patch Looks OK to me.
Attachment #9017824 - Flags: review?(jseward) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fe962bfc351a add required flags to bulk memory/table operations. r=jseward
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.