1459552 - Wasm: memory.fill and memory.copy: fix opcode assignments per recent committee decision

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[Julian Seward [:jseward]]

Recently landed bug 1446930 adds an initial implementation for 
memory.fill and memory.copy, but with "unofficial" opcode assignment.
This bug is to update it as much as we can, towards an "official"
assignment, per details at 
https://bugzilla.mozilla.org/show_bug.cgi?id=1446930#c9

Comment 1

[Julian Seward [:jseward]]

Attachment: bug1459552-3.diff

Comment 2

[Lars T Hansen [:lth]]

Comment on attachment 8973939 [details] [diff] [review]
bug1459552-3.diff

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

::: js/src/jit-test/lib/wasm-binary.js
@@ +100,2 @@
>  const SimdPrefix = 0xfd;
>  const AtomicPrefix = 0xfe;

Might as well fix this as ThreadPrefix while you're driving past.

::: js/src/jit-test/tests/wasm/binary.js
@@ +495,5 @@
>  //
>  // Feb 2018 numeric draft:
>  //
> +//  0x00 .. 0x07 are saturating truncation ops.  0x40 and 0x41 are
> +//  from the bulk memory proposal.

Add something about 0x40 and 0x41 being unofficial.

::: js/src/wasm/WasmBinaryConstants.h
@@ +362,5 @@
>      I64TruncUSatF32                      = 0x05,
>      I64TruncSSatF64                      = 0x06,
>      I64TruncUSatF64                      = 0x07,
>  
> +    // Bulk memory operations.

I would add "Unofficial opcodes." at the end of this line just to be safe.

::: js/src/wasm/WasmValidate.cpp
@@ +818,2 @@
>                  CHECK(iter.readConversion(ValType::F64, ValType::I64, &nothing));
> +#endif

Missing code here for bulkmem ops?

Comment 3

[Julian Seward [:jseward]]

(In reply to Lars T Hansen [:lth] from comment #2)

Thanks for the review.  I'll fix all this before landing, except ...

> ::: js/src/wasm/WasmValidate.cpp
> @@ +818,2 @@
> >                  CHECK(iter.readConversion(ValType::F64, ValType::I64, &nothing));
> > +#endif
> 
> Missing code here for bulkmem ops?

... which I've done in the followup bug 1459893, because that involved
also messing with test cases.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f75694d9f82
Wasm: memory.fill and memory.copy: fix opcode assignments per recent committee decision.  r=lth.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3f75694d9f82