[multibyte] Support all array element types in the interpreter - #9115
Open
brendandahl wants to merge 1 commit into
Open
brendandahl wants to merge 1 commit into
brendandahl wants to merge 1 commit into
Conversation
Since primitive GC arrays are stored as raw byte buffers (WebAssembly#9051) the interpreter now only needs to be updated to handle calculating the effective address with the offset. Also fix a printing bug that only showed up once arrays of wider element types were executed: an unreachable access was printed as i32.load/i32.store regardless of its alignment, e.g. "i32.load align=8", which fails validation. Print a type that is valid for the alignment, as we do for loads from memory. Add execution tests for all the valid element types (i8, i16, i32, i64, f32, f64, v128): a new lit exec test that runs them in the interpreter before and after optimization, new spec test modules that perform the same accesses on arrays of each element type and cover the offset and align immediates, and a ctor-eval test that evaluates multibyte stores and serializes the results back as elements. Also expand the validation test with immutable non-i8 arrays, non-numeric element types, alignment and offset limits, and the feature requirement.
tlively
reviewed
Sep 18, 2026
| (i64.store (type $i32_array) (global.get $i32) (i32.const 4) | ||
| (i64.const 0x1122334455667788) | ||
| ) | ||
| ;; An unaligned i32 store covers parts of two i16 elements. |
Member
There was a problem hiding this comment.
Suggested change
| ;; An unaligned i32 store covers parts of two i16 elements. | |
| ;; An unaligned i32 store covers parts of three i16 elements. |
| (func $i8-array (export "i8-array") (result i32) | ||
| (local $a (ref $i8)) | ||
| (local.set $a (call $new-i8)) | ||
| (i32.store (type $i8) (local.get $a) (i32.const 4) (i32.const 0x12345678)) |
Member
There was a problem hiding this comment.
It would be good to use decimal numbers in the input to make it easier to verify that it matches the output.
| ;; Loads of every width and sign, on an array whose elements are wider than | ||
| ;; the access. | ||
| ;; CHECK: [fuzz-exec] export load-widths-i32 | ||
| ;; CHECK-NEXT: [fuzz-exec] note result: load-widths-i32 => 65284 |
Member
There was a problem hiding this comment.
It would be nice if these results were easier to verify, too.
Member
There was a problem hiding this comment.
Do we need this in addition to the spec test?
Collaborator
Author
There was a problem hiding this comment.
I was meaning to ask about this... Is this folder just for tests that aren't covered by spec tests?
Member
There was a problem hiding this comment.
Yeah, it's a little unclear. Generally we should prefer (upstream!) spec tests, but sometimes there are tests that are very specific to the interpreter or otherwise don't make sense as spec tests. I think we should probably prefer spec tests more than we have historically.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since primitive GC arrays are stored as raw byte buffers (#9051) the interpreter now only needs to be updated to handle calculating the effective address with the offset.
Also fix a printing bug that only showed up once arrays of wider element types were executed: an unreachable access was printed as i32.load/i32.store regardless of its alignment, e.g. "i32.load align=8", which fails validation. Print a type that is valid for the alignment, as we do for loads from memory.
Add execution tests for all the valid element types (i8, i16, i32, i64, f32, f64, v128): a new lit exec test that runs them in the interpreter before and after optimization, new spec test modules that perform the same accesses on arrays of each element type and cover the offset and align immediates, and a ctor-eval test that evaluates multibyte stores and serializes the results back as elements. Also expand the validation test with immutable non-i8 arrays, non-numeric element types, alignment and offset limits, and the feature requirement.