Update memory.copy and table.copy encodings

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: lth, Assigned: lth)

Tracking

(Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

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.

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

Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Summary: Update memory.copy and table.copy encodings → Update memory.copy/fill/init/etc and table.copy/fill/init/etc encodings

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.

Summary: Update memory.copy/fill/init/etc and table.copy/fill/init/etc encodings → Update memory.copy and table.copy encodings
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.
Attachment #9038503 - Flags: review?(jseward) → review+

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.)

Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e914b7f1c09
Update encodings for table.copy and memory.copy.  r=jseward
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
See Also: → 1523081
You need to log in before you can comment on or make changes to this bug.