Skip to content

Conversation

@tlively
Copy link
Member

@tlively tlively commented Apr 24, 2024

Updating just one or the other of these tools would cause the tests
spec/import-after-*.fail.wast to fail, since only the updated tool would
correctly fail to parse its contents. To avoid this, update both tools at
once. (The tests erroneously pass before this change because check.py does not
ensure that .fail.wast tests fail, only that failing tests end in .fail.wast.)

In wasm-shell, to minimize the diff, only use the new parser to parse modules
and instructions. Continue using the legacy parsing based on s-expressions for
the other wast commands. Updating the parsing of the other commands to use
Lexer instead of SExpressionParser is left as future work. The boundary
between the two parsing styles is somewhat hacky, but it is worth it to enable
incremental development.

Update the tests to fix incorrect wast rejected by the new parser. Many of the
spec/old_* tests use non-standard forms from before Wasm MVP was standardized,
so fixing them would have been onerous. All of these tests have non-old_*
variants, so simply delete them.

@tlively tlively requested a review from kripken April 24, 2024 03:25
Copy link
Member Author

tlively commented Apr 24, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @tlively and the rest of your teammates on Graphite Graphite

(assert_return (invoke "i32x4.trunc_sat_f32x4_s" (v128.const f32x4 42 nan infinity -infinity)) (v128.const i32x4 42 0 2147483647 -2147483648))
(assert_return (invoke "i32x4.trunc_sat_f32x4_u" (v128.const f32x4 42 nan infinity -infinity)) (v128.const i32x4 42 0 4294967295 0))
(assert_return (invoke "i32x4.trunc_sat_f32x4_s" (v128.const f32x4 42 nan inf -inf)) (v128.const i32x4 42 0 2147483647 -2147483648))
(assert_return (invoke "i32x4.trunc_sat_f32x4_u" (v128.const f32x4 42 nan inf -inf)) (v128.const i32x4 42 0 4294967295 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lot of SIMD changes... did we write this by hand, or did the upstream spec change in the middle, I'm just curious?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we wrote this one by hand as we were implementing SIMD interpretation. That was probably before there were any upstream spec tests.

Copy link
Member Author

tlively commented Apr 24, 2024

Merge activity

  • Apr 24, 2:06 PM EDT: @tlively started a stack merge that includes this pull request via Graphite.
  • Apr 24, 2:07 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 24, 2:07 PM EDT: @tlively merged this pull request with Graphite.

Base automatically changed from shell-unknown-commands to main April 24, 2024 18:06
tlively added 2 commits April 24, 2024 18:06
Updating just one or the other of these tools would cause the tests
spec/import-after-*.fail.wast to fail, since only the updated tool would
correctly fail to parse its contents. To avoid this, update both tools at
once. (The tests erroneously pass before this change because check.py does not
ensure that .fail.wast tests fail, only that failing tests end in .fail.wast.)

In wasm-shell, to minimize the diff, only use the new parser to parse modules
and instructions. Continue using the legacy parsing based on s-expressions for
the other wast commands. Updating the parsing of the other commands to use
`Lexer` instead of `SExpressionParser` is left as future work. The boundary
between the two parsing styles is somewhat hacky, but it is worth it to enable
incremental development.

Update the tests to fix incorrect wast rejected by the new parser. Many of the
spec/old_* tests use non-standard forms from before Wasm MVP was standardized,
so fixing them would have been onerous. All of these tests have non-old_*
variants, so simply delete them.
@tlively tlively force-pushed the shell-new-parser-incremental branch from feb509d to 0ea892a Compare April 24, 2024 18:06
@tlively tlively merged commit c183dc9 into main Apr 24, 2024
@tlively tlively deleted the shell-new-parser-incremental branch April 24, 2024 18:07
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants