1513499 - Update memory.copy and table.copy encodings

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Lars T Hansen [:lth]]

This commit: https://github.com/WebAssembly/bulk-memory-operations/commit/b19cbf3b367b2a96bacfa7a9d5f370ee3f74fcf7 added a second zero byte to memory.copy and table.copy, without committing to whether that is a straight memory index or possibly a flag byte.  Either way we're now incompatible with the proposal and something needs to be done about it.

Given how far along reftypes are (with its multiple tables) it seems that the time has come to force some action on what the bytes mean for table.copy at least.

Comment 1

[Lars T Hansen [:lth]]

Also this changes the encoding of instructions by reordering immediates: https://github.com/WebAssembly/bulk-memory-operations/commit/446b9c2b8ac5186768bb1d0eccd96052f20a7f03

Comment 2

[Lars T Hansen [:lth]]

Working on getting some clarification on this: https://github.com/WebAssembly/reference-types/issues/18. There are several open questions about encoding and operand order.

Comment 3

[Lars T Hansen [:lth]]

Attachment: bug1513499-encode-mem-and-table-copy.patch

Spec here: https://github.com/WebAssembly/bulk-memory-operations/blob/master/proposals/bulk-memory-operations/Overview.md#instruction-encoding

Comment 4

[Julian Seward [:jseward]]

Comment on attachment 9038503 [details] [diff] [review]
bug1513499-encode-mem-and-table-copy.patch

Review of attachment 9038503 [details] [diff] [review]:
-----------------------------------------------------------------

r+ provided the spec is fixed to make the types of {memory,table}_{dst,src}
be varuint32.

Comment 5

[Lars T Hansen [:lth]]

I will file a bug / followup to the "sync with reftypes" bug, to get this nailed down, it's overdue. It's clear that these need to be varuint32, and the reftypes spec forces that issue, as seen from the definition of eg table_get there.

https://github.com/WebAssembly/reference-types/blob/b70d03c26d39616dc13c167836d055ecaa412fa2/interpreter/binary/decode.ml#L268
https://github.com/WebAssembly/reference-types/blob/b70d03c26d39616dc13c167836d055ecaa412fa2/interpreter/binary/decode.ml#L198

(Same comment for several other bugs hanging off of bug 1413846.)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e914b7f1c09
Update encodings for table.copy and memory.copy.  r=jseward

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/5e914b7f1c09