Update SIMD support #1553

silvanshade · 2020-09-27T00:09:09Z

This PR updates SIMD support corresponding to changes in the WebAssembly/simd proposal repo from the following PRs:

WebAssembly/simd#321
WebAssembly/simd#322
WebAssembly/simd#341
WebAssembly/simd#344

This should not be merged until WebAssembly/testsuite#32 is merged since it needs the updated testsuite using the new syntax for the SIMD tests.

For the moment I have pointed the third_party/testsuite to my local branch which has the recent changes from WebAssembly/simd merged.

Note that in the PR I referenced for the WebAssembly/testsuite there will be additional (probably minor) breakage from recent changes in other proposal repos but I didn't include fixes for all of those here since I wanted this PR to focus on SIMD.

silvanshade · 2020-09-29T04:55:27Z

I've updated the PR now that WebAssembly/testsuite has been synchronized with the WebAssembly/simd repo upstream.

All of the simd tests are still passing but one of the new (unrelated) tests pulled in from WebAssembly/spec does not pass and I'm not sure how to fix it.

The test is this one which was added from WebAssembly/spec#1213.

aardappel · 2020-10-02T23:35:17Z

Looks like the func.wast test is still failing.. I was actually looking at that earlier, but no idea what the problem is.

Is this the root cause? out/test/spec/func.wast:448: expected module to be malformed: "out/test/spec/func/func.3.wat" ? Seems some quote modules where added.

silvanshade · 2020-10-03T00:29:24Z

Is this the root cause? out/test/spec/func.wast:448: expected module to be malformed: "out/test/spec/func/func.3.wat" ? Seems some quote modules where added.

@aardappel Yes, that's the one I wasn't sure how to fix either.

It's related to this issue: WebAssembly/spec#1213.

There's a similar test above it that doesn't use the quoted module (here) and that one does fail (correctly).

The problem seemingly has to do with the fact that the type index (here) is out of range and should trigger the malformed error for the quoted module, but doesn't.

Probably there is some sort of validation that has to happen when processing the quoted module to catch this but I don't understand enough about the internals of wabt to know how to fix it.

aardappel · 2020-10-05T15:12:57Z

@rossberg (and @binji if around) any guidance what the new test in WebAssembly/spec#1213 is supposed to do? Both myself and @darinmorrison so far have failed to fix it in WABT (see error above).

rossberg · 2020-10-05T15:19:26Z

@aardappel, can you be more specific? There is more than one test there.

rossberg · 2020-10-05T15:20:21Z

Ah, sorry, didn't read up-thread. Let me look.

rossberg · 2020-10-05T15:39:19Z

Okay, so the difference between this test and the preceding one is that the function also lists a parameter explicitly. This is a form of syntactic sugar in the text format alone, that has no correspondence in the binary format, and thus the AST. Consequently, it has to be desugared in the parser. But desugaring it requires knowing the actual type. So in order to do that, the parser (not just the validator) has to know and access the type definition for type 2 in this case, which doesn't exist. Since it's the parser failing this time, it produces malformed, not invalid.

It is rather annoying, but I'm not sure what the alternative could be, given that we have this sugar. What does wabt produce for this test atm?

silvanshade · 2020-10-05T17:24:38Z

What does wabt produce for this test atm?

@rossberg Currently wabt doesn't produce any error on the second test with the quoted module.

Okay, so the difference between this test and the preceding one is that the function also lists a parameter explicitly. This is a form of syntactic sugar in the text format alone, that has no correspondence in the binary format, and thus the AST. Consequently, it has to be desugared in the parser.

Alright, I think that makes sense.

I know almost nothing about the internals of wabt. This was my first PR, just intended to fix the SIMD tests for some other tools that use wabt downstream.

However, I think there may be something more fundamental going wrong with how wabt is handling the test because even if you remove the parameter (below) so that the quoted module is like the first test, it still gives no error:

(assert_invalid
  (module quote
    "(func $f (result f64) (f64.const 0))"  ;; adds implicit type definition
    "(func $g (param i32))"                 ;; reuses explicit type definition
    "(func $h (result f64) (f64.const 1))"  ;; reuses implicit type definition
    "(type $t (func (param i32)))"

    "(func (type 2))"  ;; does not exist
  )
  "unknown type"
)

Just to be clear, should the above give an invalid error? (Not malformed since it doesn't involve the desugaring?)

rossberg · 2020-10-06T07:16:02Z

Yes, that should be invalid, just like the preceding test.

aardappel · 2020-10-11T23:51:25Z

@darinmorrison any progress on fixing that last issue?

silvanshade · 2020-10-12T00:49:56Z

@aardappel Unfortunately I don't really know how to fix that test. I think this will need some input from @binji or @sbc100 or some other maintainer who is more familiar with the internals of wabt.

aardappel · 2020-10-12T22:29:27Z

Ok, I will have another look.

aardappel · 2020-10-12T22:47:33Z

Ok, in CheckFuncTypeVarMatchesExplicit try adding this to the if (func_type) check:

    } else {
      errors->emplace_back(ErrorLevel::Error, loc, "type index out of range");
      result = Result::Error;

That made func.wast pass for me, and didn't seem to break any other tests.

silvanshade · 2020-10-13T02:16:34Z

@aardappel Do you mean like this?

Result CheckFuncTypeVarMatchesExplicit(const Location& loc,
                                       const Module& module,
                                       const FuncDeclaration& decl,
                                       Errors* errors) {
  Result result = Result::Ok;
  if (decl.has_func_type) {
    const FuncType* func_type = module.GetFuncType(decl.type_var);
    if (func_type) {
      result |=
          CheckTypes(loc, decl.sig.result_types, func_type->sig.result_types,
                     "function", "result", errors);
      result |=
          CheckTypes(loc, decl.sig.param_types, func_type->sig.param_types,
                     "function", "argument", errors);
    } else {
      errors->emplace_back(ErrorLevel::Error, loc, "type index out of range");
      result = Result::Error;
    }
  }
  return result;
}

Maybe I'm missing something but when I re-run cmake --build out --target run-tests after that change, I get several new failing cases:

**** FAILED ******************************************************************
- test/spec/bulk-memory-operations/imports.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/bulk-memory-operations/imports.wast -o out/test/spec/bulk-memory-operations/imports/imports.json --enable-bulk-memory
- test/spec/call_indirect.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/call_indirect.wast -o out/test/spec/call_indirect/call_indirect.json
- test/spec/func.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/func.wast -o out/test/spec/func/func.json
- test/spec/func_ptrs.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/func_ptrs.wast -o out/test/spec/func_ptrs/func_ptrs.json
- test/spec/imports.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/imports.wast -o out/test/spec/imports/imports.json
- test/spec/reference-types/call_indirect.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/reference-types/call_indirect.wast -o out/test/spec/reference-types/call_indirect/call_indirect.json --enable-reference-types
- test/spec/reference-types/imports.txt
    /home/dwm/development/WebAssembly/wabt/out/wast2json out/test/spec/reference-types/imports.wast -o out/test/spec/reference-types/imports/imports.json --enable-reference-types
- test/wasm2c/spec/call_indirect.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/call_indirect.txt --bindir=/home/dwm/development/WebAssembly/wabt/out --no-error-cmdline -o out/test/wasm2c/spec/call_indirect
- test/wasm2c/spec/func_ptrs.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/func_ptrs.wast --bindir=/home/dwm/development/WebAssembly/wabt/out --no-error-cmdline -o out/test/wasm2c/spec/func_ptrs
- test/wasm2c/spec/func.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/func.txt --bindir=/home/dwm/development/WebAssembly/wabt/out --no-error-cmdline -o out/test/wasm2c/spec/func
- test/wasm2c/spec/imports.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/imports.wast --bindir=/home/dwm/development/WebAssembly/wabt/out --no-error-cmdline -o out/test/wasm2c/spec/imports
FAILED: CMakeFiles/run-tests

For comparison, before the change (just with this current PR), this is the output I get:

**** FAILED ******************************************************************
- test/spec/func.txt
    /home/dwm/development/WebAssembly/wabt/out/spectest-interp out/test/spec/func/func.json
FAILED: CMakeFiles/run-tests

Was there maybe another change that needed to be included?

aardappel · 2020-10-13T04:31:53Z

Did you run run-tests.py -r to update outputs?

silvanshade · 2020-10-13T07:55:08Z

Did you run run-tests.py -r to update outputs?

@aardappel Yes, I did try that with individual tests but I'm still getting errors.

Running run-tests.py -r produces the following:

**** FAILED ******************************************************************
- test/spec/bulk-memory-operations/imports.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/bulk-memory-operations/imports.wast -o out/test/spec/bulk-memory-operations/imports/imports.json --enable-bulk-memory
- test/spec/call_indirect.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/call_indirect.wast -o out/test/spec/call_indirect/call_indirect.json
- test/spec/func.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/func.wast -o out/test/spec/func/func.json
- test/spec/func_ptrs.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/func_ptrs.wast -o out/test/spec/func_ptrs/func_ptrs.json
- test/spec/imports.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/imports.wast -o out/test/spec/imports/imports.json
- test/spec/reference-types/call_indirect.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/reference-types/call_indirect.wast -o out/test/spec/reference-types/call_indirect/call_indirect.json --enable-reference-types
- test/spec/reference-types/imports.txt
    /home/dwm/development/WebAssembly/wabt/bin/wast2json out/test/spec/reference-types/imports.wast -o out/test/spec/reference-types/imports/imports.json --enable-reference-types
- test/wasm2c/spec/call_indirect.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/call_indirect.txt --bindir=/home/dwm/development/WebAssembly/wabt/bin --no-error-cmdline -o out/test/wasm2c/spec/call_indirect
- test/wasm2c/spec/func.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/func.txt --bindir=/home/dwm/development/WebAssembly/wabt/bin --no-error-cmdline -o out/test/wasm2c/spec/func
- test/wasm2c/spec/func_ptrs.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/func_ptrs.wast --bindir=/home/dwm/development/WebAssembly/wabt/bin --no-error-cmdline -o out/test/wasm2c/spec/func_ptrs
- test/wasm2c/spec/imports.txt
    /usr/bin/python3 /home/dwm/development/WebAssembly/wabt/test/run-spec-wasm2c.py out/test/wasm2c/spec/imports.wast --bindir=/home/dwm/development/WebAssembly/wabt/bin --no-error-cmdline -o out/test/wasm2c/spec/imports

If you were able to get it to work (with the tests passing on CI), can you make your branch available? From that, I could just cherry-pick the commit into this PR.

aardappel · 2020-10-13T15:24:14Z

weird, I don't get those errors, instead I get some remaining SIMD errors I haven't solved yet.. I may make a PR later and we can compare

aardappel · 2020-10-15T16:41:49Z

Ok, my changes are here, and indeed also create these new wasm2c problems.. apparently because these don't run on Windows:
#1560

It may be easiest to wait until @binji is back rather than try to debug this further

binji · 2020-10-21T21:13:32Z

Sorry for the long delay on resolving this. Is func.wast the only issue? If so, probably the easiest thing to do is change the expectation for that test to failure, and we can file a bug and resolve it separately.

It looks like other than this problem, the rest of the PR is relatively straightforward.

silvanshade · 2020-10-22T20:26:51Z

@binji Yes, it's just the one test in func.wast. I think I can update the PR to expect that test to fail. Will take a look at that soon.

silvanshade · 2020-10-24T01:55:53Z

@binji this should be good to go now I think.

binji

Awesome, thanks for doing this! lgtm

silvanshade mentioned this pull request Sep 27, 2020

WIP: Update SIMD instructions bytecodealliance/wasm-tools#105

Merged

silvanshade force-pushed the update-simd branch from 7622441 to d33c459 Compare September 29, 2020 04:49

silvanshade mentioned this pull request Oct 2, 2020

Revert "Update repos" WebAssembly/testsuite#35

Closed

aardappel requested a review from sbc100 October 2, 2020 23:35

aardappel mentioned this pull request Oct 15, 2020

Updates for Memory64 proposal tests from latest testsuite #1560

Merged

darinmorrison added 4 commits October 23, 2020 19:54

Update testsuite

7f6d648

Update test results

b1a1e94

Disallow lane nums with optional + for simd ops

7f45bfb

Update simd support

c6d53e9

silvanshade force-pushed the update-simd branch from 9d07cf2 to b351e0b Compare October 24, 2020 01:55

Mark test/spec/func.txt as failing

faf6e33

silvanshade force-pushed the update-simd branch from b351e0b to faf6e33 Compare October 24, 2020 03:37

silvanshade mentioned this pull request Oct 24, 2020

Fix func.wast failures with recent testsuite updates #1561

Closed

binji approved these changes Oct 26, 2020

View reviewed changes

binji merged commit 30af7af into WebAssembly:master Oct 26, 2020

ngzhian mentioned this pull request Mar 17, 2021

Shorten all saturating instructions to "sat" WebAssembly/simd#332

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update SIMD support #1553

Update SIMD support #1553

silvanshade commented Sep 27, 2020

silvanshade commented Sep 29, 2020

aardappel commented Oct 2, 2020

silvanshade commented Oct 3, 2020 •

edited

Loading

aardappel commented Oct 5, 2020

rossberg commented Oct 5, 2020

rossberg commented Oct 5, 2020

rossberg commented Oct 5, 2020

silvanshade commented Oct 5, 2020 •

edited

Loading

rossberg commented Oct 6, 2020

aardappel commented Oct 11, 2020

silvanshade commented Oct 12, 2020

aardappel commented Oct 12, 2020

aardappel commented Oct 12, 2020

silvanshade commented Oct 13, 2020

aardappel commented Oct 13, 2020

silvanshade commented Oct 13, 2020

aardappel commented Oct 13, 2020

aardappel commented Oct 15, 2020

binji commented Oct 21, 2020

silvanshade commented Oct 22, 2020

silvanshade commented Oct 24, 2020

binji left a comment

Update SIMD support #1553

Update SIMD support #1553

Conversation

silvanshade commented Sep 27, 2020

silvanshade commented Sep 29, 2020

aardappel commented Oct 2, 2020

silvanshade commented Oct 3, 2020 • edited Loading

aardappel commented Oct 5, 2020

rossberg commented Oct 5, 2020

rossberg commented Oct 5, 2020

rossberg commented Oct 5, 2020

silvanshade commented Oct 5, 2020 • edited Loading

rossberg commented Oct 6, 2020

aardappel commented Oct 11, 2020

silvanshade commented Oct 12, 2020

aardappel commented Oct 12, 2020

aardappel commented Oct 12, 2020

silvanshade commented Oct 13, 2020

aardappel commented Oct 13, 2020

silvanshade commented Oct 13, 2020

aardappel commented Oct 13, 2020

aardappel commented Oct 15, 2020

binji commented Oct 21, 2020

silvanshade commented Oct 22, 2020

silvanshade commented Oct 24, 2020

binji left a comment

Choose a reason for hiding this comment

silvanshade commented Oct 3, 2020 •

edited

Loading

silvanshade commented Oct 5, 2020 •

edited

Loading