Closed Bug 1459552 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: jseward, Assigned: jseward)

References

Details

Attachments

(1 file)

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
Attachment #8973939 - Flags: review?(lhansen)
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?
Attachment #8973939 - Flags: review?(lhansen) → review+
Blocks: 1459893
(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.
Pushed by jseward@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f75694d9f82
Wasm: memory.fill and memory.copy: fix opcode assignments per recent committee decision.  r=lth.
https://hg.mozilla.org/mozilla-central/rev/3f75694d9f82
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1461304
Assignee: nobody → jseward
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: