[multibyte] Add multibyte array store instructions.#8059
Conversation
| // 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.
| 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.
46cdca3 to
60f68c8
Compare
0a6ba01 to
2f3a132
Compare
Add support for multibyte array store instructions as proposed in the WebAssembly multibyte proposal. These instructions allow storing values of various byte widths directly into i8 arrays, avoiding the need for manual bit manipulation and masking. Link: https://github.com/WebAssembly/multibyte
2f3a132 to
0847ada
Compare
| } | ||
| note(&curr->ref, Type(*ht, Nullable)); | ||
| note(&curr->index, Type::i32); | ||
| note(&curr->value, valueType ? *valueType : curr->value->type); |
There was a problem hiding this comment.
We should self().noteUnknown() if curr->value->type == Type::unreachable.
How long would you recommend letting it run for? |
|
I would give it half a day, personally. |
| // As in readFromData, normalize to the proper cone. | ||
| auto cone = refContents.getCone(); | ||
| auto normalizedDepth = getNormalizedConeDepth(cone.type, cone.depth); | ||
| if (multibyte) { |
There was a problem hiding this comment.
@kripken Here's where I'm having issues.
(module
(type $0 (func))
(type $1 (array (mut i8)))
(global $global$0 (ref $1) (array.new_default $1
(i32.const 4)
))
(func $0
(f64.store (type $1)
(global.get $global$0)
(i32.const 1)
(f64.const 2)
)
)
)with
wasm-opt a.wasm -o b.wasm --discard-global-effects --roundtrip --signature-refining --gufa-cast-all --shrink-level=1 --closed-world --dce --closed-world -all
There was a problem hiding this comment.
I am guessing that filterExpressionContents applies the type of the expression in the wasm, and here it writes a different type, so the filtering ends up making things worse.
Updating filterExpressionContents should fix this. I assume the issue is on ArrayGet. When such a get sees data of another type in a numeric array, rather than combining them to get Many, we can apply PossibleContents::fromType(value->type). That is, an unknown value in array.get is still going to be of the type that particular get reads.
There was a problem hiding this comment.
Alright here's what I came up with in c148eaf . I'll let it cook on the fuzzer for a bit.
There was a problem hiding this comment.
@kripken This has run against the fuzzer for a day. Did you want to review the above changes or is this good to land?
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 $type_idx) <array ref> <index> <value>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