[WIP][multibyte] Add multibyte array store instructions. #8059
Conversation
Prototype implementation of the new multibyte array proposal that reuses
the existing memory instructions.
The syntax for the new instructions is as follows:
<i32|i64|f32|f64>.store type=array <array ref> <index> <value>
Some spec related TODOs:
- Decide on new instructions vs existing memory instructions
with memarg flag.
- If using existing instructions:
- What does the wat syntax look like?
e.g. `i32.store type=array x y` or `i32.store array x y` or ???
- Do we support immediate offsets?
- Do we support i32.store8? (effectively the same as array.set on
i8)
- Do we support atomic instructions?
Some implementation TODOs:
- Add v128.store.
- Go through the various `visitArrayStore` functions and implement
them or make sure the copied versions makes sense.
- Add fuzzing support.
- Make it so there's only one `WasmBinaryReader::getMemarg`
- Add JS API
| // TODO is this needed? | ||
| MemoryOrder order = MemoryOrder::Unordered; |
There was a problem hiding this comment.
Probably in the long run, but not urgently. I would leave it out for now.
| ) | ||
|
|
||
| ;; CHECK: (func $stores (type $1) | ||
| ;; CHECK-NEXT: (i32.store8 type=array |
There was a problem hiding this comment.
The text format is going to need a type index immediate to give the type of the array that is being accessed. Maybe we can just use the presence of that immediate to signal that we're accessing an array? So we might have i32.store8 $myArray instead of i32.store8 type=array. Obviously we can't settle that here, but we're definitely going to need the immediate.
There was a problem hiding this comment.
What's the reason for the type immediate if it's only ever going to be an (array (mut i8))? I guess it could be a memref in the future or?
There was a problem hiding this comment.
There is an infinite set of (array (mut i8)) types, and I would expect these instructions to work with all of them. For example, $A, $B, $C, and $D are all different types here:
(type $A (sub final (array (mut i8))))
(type $B (sub (array (mut i8))))
(type $C (sub $B (array (mut i8))))
(rec (type $D (array (mut i8))) (type (struct)))
Given that there are several types that could be accepted, we need the type index to make sure we can look at an instruction with its immediates in isolation and say what its input and output types will be. (This ensures we maintain the principal types property, which is important for analyzing instruction sequences in isolation. For example, it is important for outlining.)
There was a problem hiding this comment.
The alternative is to accept just a single, well-known array type, e.g. (rec (type (sub final (array (mut i8))))).0, but that would be somewhat limiting for producers, would require extra work in Binaryen, and would be unprecedented for core Wasm instructions.
There was a problem hiding this comment.
I started playing with i32.store8 $myArray, but if I'm following this correctly, I don't think there's a way to distinguish multimemory from array store e.g. i32.store8 1 could be a memory or type index.
There was a problem hiding this comment.
oh yeah, good point :/ I guess we do need to distinguish this somehow. For prototyping I guess we can do whatever, but it would also be good to open a issue on the proposal repo to see what folks think is best here.
| case BinaryConsts::I32StoreMem8: { | ||
| auto [mem, align, offset] = getMemarg(); | ||
| return builder.makeStore(1, offset, align, Type::i32, mem); | ||
| auto [mem, align, offset, backing] = getMemargWithBacking(); |
There was a problem hiding this comment.
The binary format will need the type immediate as well, so this will have to look something like this:
| auto [mem, align, offset, backing] = getMemargWithBacking(); | |
| auto [mem, align, offset, backing] = getMemargWithBacking(); | |
| if (backing == BackingType::Array) { | |
| HeapType type = getIndexedHeapType(); | |
| ... |
At that point we might as well add a separate IRBuilder method for making array stores, which should help separate concerns more.
2 participants
Prototype implementation of the new multibyte array proposal that reuses the existing memory instructions.
The syntax for the new instructions is as follows:
<i32|i64|f32|f64>.store type=array
Some spec related TODOs:
i32.store type=array x yori32.store array x yor ???Some implementation TODOs:
visitArrayStorefunctions and implement them or make sure the copied versions makes sense.WasmBinaryReader::getMemarg