Update some of the test assertions in simd_lane.wast
#140
Conversation
I believe these constants were placed there by mistake since the tests seem to be for the immediate argument of the instruction rather than the text format.
The tests here seem to be for the text format either specifying too few or too few lanes, so this moves them to `assert_malformed` instead of inline as an expected-valid module. This way parsers don't need to handle a variable number of lanes but can always expect 16 lanes to be specified.
test/core/simd/simd_lane.wast
Outdated
| (assert_malformed (module quote "(func v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14)") "invalid lane length") | ||
| (assert_malformed (module quote "(func v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") "invalid lane length") |
There was a problem hiding this comment.
Would this work like this without arguments?
There was a problem hiding this comment.
Hm sorry I'm not sure I quite understand, could you clarify what you're thinking should work without arguments?
There was a problem hiding this comment.
Does the shuffle instruction need all arguments for this test to work correctly? You are not providing the two v128 arguments to it.
There was a problem hiding this comment.
I agree this should be malformed, but I think we should provide the two v128 arguments to distinguish the malformed is from missing arguments or the invalid lane length. BTW, missing arguments is another test that worth testing.
There was a problem hiding this comment.
Ah ok, I see what you mean, and sure! I've updated the test to typecheck but still have this expected assertion failure.
Sorry I also don't know how to have multiline strings in the wat format so it's a bit messy...
There was a problem hiding this comment.
@alexcrichton, thank you! LGTM except the test for "invalid lane length", I think we should keep the two v128 arguments.
test/core/simd/simd_lane.wast
Outdated
| (assert_malformed (module quote "(func v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14)") "invalid lane length") | ||
| (assert_malformed (module quote "(func v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") "invalid lane length") |
There was a problem hiding this comment.
I agree this should be malformed, but I think we should provide the two v128 arguments to distinguish the malformed is from missing arguments or the invalid lane length. BTW, missing arguments is another test that worth testing.
test/core/simd/simd_lane.wast
Outdated
| (v128.const i8x16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0) | ||
| (v128.const i8x16 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15)))) "invalid lane length") | ||
| (assert_malformed (module quote "(func (param v128) (result v128) local.get 0 local.get 0 v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14)") "invalid lane length") | ||
| (assert_malformed (module quote "(func (param v128) (result v128) local.get 0 local.get 0 v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") "invalid lane length") |
There was a problem hiding this comment.
| (assert_malformed (module quote "(func (param v128) (result v128) local.get 0 local.get 0 v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") "invalid lane length") | |
| (assert_malformed (module quote "(func (param v128) (result v128) " | |
| "(local.get 0) (local.get 0)" | |
| "v8x16.shuffle 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16)") | |
| "invalid lane length") |
There was a problem hiding this comment.
It's more readable with above suggestion. :)
This updates a few of the assertions in
simd_lane.wastfound after writing a parser which consumes*.wastfiles and choking on a few of the test cases here. I'm not 100% sure if all of these changes are correct, so feedback is appreciated!Additionally, are these tests currently run through a reference implementation somewhere? I'm not sure if the spec interpreter in this repo has been updated for the tests yet.