Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get all .wast tests passing under Pulley #9783

Open
fitzgen opened this issue Dec 11, 2024 · 2 comments
Open

Get all .wast tests passing under Pulley #9783

fitzgen opened this issue Dec 11, 2024 · 2 comments
Labels
cranelift:E-compiler Compiler issues. cranelift:E-compiler-easy Beginner–Intermediate compiler issues. cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! good first issue Issues that are good for new contributors to tackle! help wanted isle Related to the ISLE domain-specific language pulley Issues related to the Pulley interpreter

Comments

@fitzgen
Copy link
Member

fitzgen commented Dec 11, 2024

Let's get all .wast tests passing under Pulley!

Pulley is Wasmtime's portable, optimizing interpreter. When using Pulley, Wasmtime still uses Cranelift (our optimizing compiler) for optimizations, but Cranelift ultimately emits Pulley bytecode instead of machine code. Then, Wasmtime uses Pulley's interpreter to interpret the bytecode, instead of executing native machine code.

Diagram of the compiler pipeline with Pulley
         +------+
         | Wasm |
         +------+
            |
            |
  Wasm-to-CLIF translation (Wasmtime)
            |
            V
         +------+
         | CLIF |
         +------+
            |
            |
   mid-end optimizations (Cranelift)
            |
            V
         +------+
         | CLIF |
         +------+
            |
            |
    ISLE lowering rules (Cranelift)
            |
            V
   +-----------------+
   | Pulley bytecode |
   +-----------------+

This is a good first issue for new contributors that are already somewhat familiar with Rust development and are excited to dive into Wasmtime/Cranelift/compilers/interpreters/etc. To help out with this effort, follow this guide:

Guide: How to Add Missing Instructions to Pulley and Get .wast Tests Passing

Note that we have docs for contributing to Wasmtime and Cranelift in general as well.

For the most up-to-date list of which .wast tests are passing and which are failing under Pulley, see the WastTest::should_fail method in crates/wast-util/src/lib.rs.

@fitzgen fitzgen mentioned this issue Dec 11, 2024
12 tasks
@fitzgen fitzgen added help wanted good first issue Issues that are good for new contributors to tackle! cranelift:E-compiler Compiler issues. cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift:E-compiler-easy Beginner–Intermediate compiler issues. isle Related to the ISLE domain-specific language pulley Issues related to the Pulley interpreter labels Dec 11, 2024
@fitzgen fitzgen changed the title [**Get all .wast tests passing under Pulley!!!!**](https://github.com/bytecodealliance/wasmtime/blob/main/pulley/CONTRIBUTING.md) Get all .wast tests passing under Pulley Dec 11, 2024
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

This issue or pull request has been labeled: "isle"

Thus the following users have been cc'd because of the following labels:

  • cfallin: isle
  • fitzgen: isle

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
This commit redefines previous Pulley instructions working with
conditional values and results to always operate on the low 32-bits of a
register rather than the full 64-bit width of integer registers. This
should help 32-bit platforms work with just a word and avoid an
extraneous load of top bits that are likely always zero.

The previous `br_if` and `br_if_not` instructions now have a "32" suffix
to make it clear that they're only operating on 32-bit register widths.
Additionally the `xeq32` family of instructions (compare-and-set) now
all only define the low 32-bits of the destination register.

Finally, lowerings of `select` in CLIF were added for integer and
floating-point registers.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
This commit first fixed an issue with table access codegen to disable
spectre mitigations on Pulley targets like how spectre is disabled for
memory accesses as well. This unblocked many tests related to tables
which then led to a different error about a `trapnz` with an 8-bit value
not being supported.

In fixing `trapnz` with 8-bit values this PR went ahead and did a
general-purpose refactoring for how conditional branches are managed.
Previously conditional traps and conditional branches had some
duplicated logic and the goal was to unify everything. There is now a
single `Cond` which represents the condition of a conditional jump which
is used uniformly for all locations such as `select`, `brif`, and
`trap[n]z`. This new type represents all the sorts of conditional
branches that can be done in Pulley, for example integer comparisons and
whether or not a register is zero. This `Cond` type has various helpers
for printing it, inverting it, collecting operands, emission, etc.

The end result is that it's a bit wordy to work with `Cond` right now
due to the size of the variants but all locations working with
conditional traps are deduplicated and now it's just repetitive logic
rather than duplicated logic.

Putting all of this together gets a large batch of spec tests working.

I'll note that this does remove a feature where `trapnz` was turned into
nothing or an unconditional trap if the argument was a constant, but
that feels like an optimization perhaps best left for the middle-end
rather than doing it in the backend.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Implement float-to-int bitcasts as well as the xor operation for
integers.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Needed the `imul` CLIF instruction to get implemented so 32/64-bit
multiplication have now been added.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
This required some extra plumbing to shepherd the precise reason why
signed division trapped to Wasmtime which is done through an extra
`TrapKind` side channel now added.

This then additionally fixes the signed remainder interpreter function
to return 0 on `MIN % -1` which is different from what Rust specifies
(which is to return `None` or panic).

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Gets the `conversions.wast` test running along with a few other misc
ones.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Fill out enough to get `f32.wast` and `f64.wast` spec tests working. A
minor ABI issue was discovered along the way which is also required to
get a new test working on both 32 and 64-bit platforms.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Fix a few typos here and there related to floats to get some tests
passing.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
Fill out a lowering for CLIF's `ineg` instruction.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
This commit redefines previous Pulley instructions working with
conditional values and results to always operate on the low 32-bits of a
register rather than the full 64-bit width of integer registers. This
should help 32-bit platforms work with just a word and avoid an
extraneous load of top bits that are likely always zero.

The previous `br_if` and `br_if_not` instructions now have a "32" suffix
to make it clear that they're only operating on 32-bit register widths.
Additionally the `xeq32` family of instructions (compare-and-set) now
all only define the low 32-bits of the destination register.

Finally, lowerings of `select` in CLIF were added for integer and
floating-point registers.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 11, 2024
This commit first fixed an issue with table access codegen to disable
spectre mitigations on Pulley targets like how spectre is disabled for
memory accesses as well. This unblocked many tests related to tables
which then led to a different error about a `trapnz` with an 8-bit value
not being supported.

In fixing `trapnz` with 8-bit values this PR went ahead and did a
general-purpose refactoring for how conditional branches are managed.
Previously conditional traps and conditional branches had some
duplicated logic and the goal was to unify everything. There is now a
single `Cond` which represents the condition of a conditional jump which
is used uniformly for all locations such as `select`, `brif`, and
`trap[n]z`. This new type represents all the sorts of conditional
branches that can be done in Pulley, for example integer comparisons and
whether or not a register is zero. This `Cond` type has various helpers
for printing it, inverting it, collecting operands, emission, etc.

The end result is that it's a bit wordy to work with `Cond` right now
due to the size of the variants but all locations working with
conditional traps are deduplicated and now it's just repetitive logic
rather than duplicated logic.

Putting all of this together gets a large batch of spec tests working.

I'll note that this does remove a feature where `trapnz` was turned into
nothing or an unconditional trap if the argument was a constant, but
that feels like an optimization perhaps best left for the middle-end
rather than doing it in the backend.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
Fill out a lowering for CLIF's `ineg` instruction.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
Gets some `embenchen_*.wast` tests passing.

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2024
Fill out some more misc float ops.

cc #9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
Fill out a lowering for CLIF's `ineg` instruction.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
Gets some `embenchen_*.wast` tests passing.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
This commit fills out some of the initial infrastructure necessary for
supporting the SIMD proposal to WebAssembly in the Pulley interpreter,
namely 128-bit simd. The `VRegVal` union has been filled out with
various types, endianness questions are settled, and initial
implementations of a suite of opcodes are added to get a basic set of
tests working throughout the backend.

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2024
Fill out a lowering for CLIF's `ineg` instruction.

cc #9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
Gets some `embenchen_*.wast` tests passing.

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2024
Gets some `embenchen_*.wast` tests passing.

cc #9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 13, 2024
This commit fills out some of the initial infrastructure necessary for
supporting the SIMD proposal to WebAssembly in the Pulley interpreter,
namely 128-bit simd. The `VRegVal` union has been filled out with
various types, endianness questions are settled, and initial
implementations of a suite of opcodes are added to get a basic set of
tests working throughout the backend.

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 14, 2024
Gets another `*.wast` test passing

cc #9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 14, 2024
This commit fills out some of the initial infrastructure necessary for
supporting the SIMD proposal to WebAssembly in the Pulley interpreter,
namely 128-bit simd. The `VRegVal` union has been filled out with
various types, endianness questions are settled, and initial
implementations of a suite of opcodes are added to get a basic set of
tests working throughout the backend.

cc bytecodealliance#9783
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 14, 2024
This commit fills out some of the initial infrastructure necessary for
supporting the SIMD proposal to WebAssembly in the Pulley interpreter,
namely 128-bit simd. The `VRegVal` union has been filled out with
various types, endianness questions are settled, and initial
implementations of a suite of opcodes are added to get a basic set of
tests working throughout the backend.

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 14, 2024
* pulley: Initial scaffold of SIMD support

This commit fills out some of the initial infrastructure necessary for
supporting the SIMD proposal to WebAssembly in the Pulley interpreter,
namely 128-bit simd. The `VRegVal` union has been filled out with
various types, endianness questions are settled, and initial
implementations of a suite of opcodes are added to get a basic set of
tests working throughout the backend.

cc #9783

* Avoid dealing with big-endian vectors

* Change wasm `global`s to store `v128` in little-endian format.
* Change pulley stack loads/stores to work with vectors in little-endian
  format.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Dec 16, 2024
Gets a few spec tests and CLIF tests passing

cc bytecodealliance#9783
github-merge-queue bot pushed a commit that referenced this issue Dec 17, 2024
* pulley: Implement SIMD `splat` instruction

Gets a few spec tests and CLIF tests passing

cc #9783

* Fix typo
@Xuanwo
Copy link

Xuanwo commented Dec 28, 2024

For newcomers to this issue:

  • "misc_testsuite/simd/canonicalize-nan.wast"
  • "misc_testsuite/simd/issue_3327_bnot_lowering.wast"
  • "misc_testsuite/simd/v128-select.wast"
  • "spec_testsuite/proposals/annotations/simd_lane.wast"
  • "spec_testsuite/proposals/relaxed-simd/i16x8_relaxed_q15mulr_s.wast"
  • "spec_testsuite/proposals/relaxed-simd/i32x4_relaxed_trunc.wast"
  • "spec_testsuite/proposals/relaxed-simd/relaxed_dot_product.wast"
  • "spec_testsuite/proposals/relaxed-simd/relaxed_madd_nmadd.wast"
  • "spec_testsuite/proposals/memory64/simd_lane.wast"
  • "spec_testsuite/proposals/memory64/relaxed_madd_nmadd.wast"
  • "spec_testsuite/proposals/memory64/relaxed_dot_product.wast"
  • "spec_testsuite/proposals/memory64/i16x8_relaxed_q15mulr_s.wast"
  • "spec_testsuite/proposals/memory64/i32x4_relaxed_trunc.wast"
  • "spec_testsuite/simd_f32x4_arith.wast" Make Pulley pass simd_f32x4_arith.wast #9897
  • "spec_testsuite/simd_f32x4_cmp.wast"
  • "spec_testsuite/simd_f32x4_pmin_pmax.wast"
  • "spec_testsuite/simd_f64x2_arith.wast"
  • "spec_testsuite/simd_f64x2_cmp.wast"
  • "spec_testsuite/simd_f64x2_pmin_pmax.wast"
  • "spec_testsuite/simd_i16x8_extadd_pairwise_i8x16.wast"
  • "spec_testsuite/simd_i16x8_q15mulr_sat_s.wast"
  • "spec_testsuite/simd_i16x8_sat_arith.wast"
  • "spec_testsuite/simd_i32x4_arith.wast" feat: Implement simd_i32x4_arith2 for pulley #9907
  • "spec_testsuite/simd_i32x4_dot_i16x8.wast"
  • "spec_testsuite/simd_i32x4_extadd_pairwise_i16x8.wast"
  • "spec_testsuite/simd_i32x4_trunc_sat_f32x4.wast"
  • "spec_testsuite/simd_i32x4_trunc_sat_f64x2.wast"
  • "spec_testsuite/simd_i64x2_arith2.wast"
  • "spec_testsuite/simd_i8x16_arith2.wast"
  • "spec_testsuite/simd_i8x16_sat_arith.wast"
  • "spec_testsuite/simd_lane.wast"
  • "spec_testsuite/simd_load.wast"
  • "spec_testsuite/simd_splat.wast"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:E-compiler Compiler issues. cranelift:E-compiler-easy Beginner–Intermediate compiler issues. cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! good first issue Issues that are good for new contributors to tackle! help wanted isle Related to the ISLE domain-specific language pulley Issues related to the Pulley interpreter
Projects
None yet
Development

No branches or pull requests

2 participants