12 KiB
Contributing
For general contribution to Wasmtime, see Wasmtime's contributing docs.
Adding an instruction to Pulley
So you want to add an instruction to Pulley. If you're reading this in the not-so-distant future Pulley probably doesn't support all of WebAssembly yet and you're interested in helping to improve the situation. This is intended to be a small guide about how to add an instruction to Pulley through an early example of doing so.
Choose a test to get passing
First off find a test in this repository, probably a *.wast test, which isn't
currently passing. Check out the WastTest::should_fail method in
crates/wast-util/src/lib.rs which has a list of unsupported tests for
Pulley. Here we're going to select ./tests/misc_testsuite/control-flow.wast
as it's a reasonably small test.
See the test failure
Run this command:
$ cargo run --features pulley -- wast --target pulley64 ./tests/misc_testsuite/control-flow.wast
This builds the wasmtime CLI with Pulley support enabled (--features pulley), runs the wast subcommand, executes with the pulley target
(--target pulley64), and then runs our test. As of now this shows:
$ cargo run --features pulley -- wast --target pulley64 ./tests/misc_testsuite/control-flow.wast
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.08s
Running `target/debug/wasmtime wast --target pulley64 ./tests/misc_testsuite/control-flow.wast`
Error: failed to run script file './tests/misc_testsuite/control-flow.wast'
Caused by:
0: failed directive on ./tests/misc_testsuite/control-flow.wast:77:1
1: Compilation error: Unsupported feature: should be implemented in ISLE: inst = `v5 = sdiv.i32 v2, v3`, type = `Some(types::I32)`
Note that if you run cargo test --test wast control-flow.wast it'll also run
the same test under a variety of configurations, but the test is expected to
fail under Pulley. You can update the WastTest::should_fail method in
crates/wast-util/src/lib.rs to say the test is expected to pass, and then you
can see a similar failure.
Adding an instruction: Pulley
Here the failure is what's the most common failure in Pulley right now -- the Pulley Cranelift backend is not yet complete and is missing a lowering. This means that there is CLIF that cannot be lowered to Pulley bytecode just yet.
The first thing to do is to probably add a new opcode to Pulley itself as Pulley probably can't execute this operation just yet. Here we're interested in signed 32-bit division.
Pull up pulley/src/lib.rs and you'll be editing the for_each_op! macro
definition. If this is a "rare" opcode you can edit the for_each_extended_op!
macro instead. The syntax is the same between the two macros.
Here this is a simple instruction, so let's add it directly:
/// `dst = src1 / src2` (signed)
xdiv32_s = XDiv32S { operands: BinaryOperands<XReg> };
This defines the snake-case name of the instruction (xdiv32_s) that is used
by the disassembler and visitor trait, the upper-camel-case name of the
instruction (XDiv32S) used for the Rust type and enum variant, and
immediates and operands in the instruction itself. In this case it's a binary
operation using integer ("x") registers.
Note: By convention, we tend to include the class ("x") and width ("32") of registers operated upon by an instruction in its name. This distinguishes between, for example, 32-bit integer addition (
xadd32) and 64-bit floating point addition (fadd64).
Rerun our test command and we see:
$ cargo run --features pulley -- wast --target pulley64 ./tests/misc_testsuite/control-flow.wast
Compiling pulley-interpreter v29.0.0 (/home/alex/code/wasmtime/pulley)
error[E0046]: not all trait items implemented, missing: `xdiv32_s`
--> pulley/src/interp.rs:807:1
|
807 | impl OpVisitor for Interpreter<'_> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `xdiv32_s` in implementation
|
::: pulley/src/decode.rs:574:17
|
574 | fn $snake_name(&mut self $( $( , $field : $field_ty )* )? ) -> Self::Return;
| ---------------------------------------------------------------------------- `xdiv32_s` from trait
Compiling cranelift-codegen-meta v0.116.0 (/home/alex/code/wasmtime/cranelift/codegen/meta)
For more information about this error, try `rustc --explain E0046`.
This indicates that we need to actually implement the new opcode in the
interpreter. Open up pulley/src/interp.rs and append to impl OpVisitor for Interpreter or impl ExtendedOpVisitor for Interpreter as appropriate. Here
we'll add:
fn xdiv32_s(&mut self, operands: BinaryOperands<XReg>) -> ControlFlow<Done> {
let a = self.state[operands.src1].get_i32();
let b = self.state[operands.src2].get_i32();
match a.checked_div(b) {
Some(result) => {
self.state[operands.dst].set_i32(result);
ControlFlow::Continue(())
}
None => self.done_trap::<crate::XDiv32S>(),
}
}
Note that division needs to handle the case that the divisor is 0 or causes an
overflow, hence the use of checked_div here. If that happens then a trap is
returned, otherwise interpretation will continue. Also note that the get_i32
method is used to specifically match the width and signedness of the
instruction itself, signed 32-bit division. Look around at other instructions
in interp.rs for inspiration of how to do various operations.
Running our test again we get the same error as before!
$ cargo run --features pulley -- wast --target pulley64 ./tests/misc_testsuite/control-flow.wast
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.08s
Running `target/debug/wasmtime wast --target pulley64 ./tests/misc_testsuite/control-flow.wast`
Error: failed to run script file './tests/misc_testsuite/control-flow.wast'
Caused by:
0: failed directive on ./tests/misc_testsuite/control-flow.wast:77:1
1: Compilation error: Unsupported feature: should be implemented in ISLE: inst = `v5 = sdiv.i32 v2, v3`, type = `Some(types::I32)`
That leads us to the next part...
Adding a Cranelift Lowering
Next up we need to actually fix the error at hand, a new lowering rule needs to
be added to Cranelift. Here we'll be working in
cranelift/codegen/src/isa/pulley_shared/lower.isle.
Our ISLE lowering rules are generally written like this:
(rule (lower <CLIF>)
<Pulley>)
This means "when lowering Cranelift's mid-level IR down to Pulley bytecode, and
we match the snippet <CLIF>, replace it with <Pulley>". (For more details,
see the ISLE Language
Reference
and How ISLE is Integrated with
Cranelift)
In our case, we need to match an sdiv CLIF instruction that is operating on
32-bit values (this is the (has_type $I32 ...) bit) and then replace it with
our new Pulley xdiv32_s instruction:
;;;; Rules for `idiv` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(rule (lower (has_type $I32 (sdiv a b)))
(pulley_xdiv32_s a b))
Note that ISLE constructors for Pulley instructions, including the
pulley_xdivs32_s constructor for our new xdiv32_s Pulley instruction, are
automatically generated from the for_each_op! macro.
Running our test again yields:
Error: failed to run script file './tests/misc_testsuite/control-flow.wast'
Caused by:
0: failed directive on ./tests/misc_testsuite/control-flow.wast:83:1
1: Compilation error: Unsupported feature: should be implemented in ISLE: inst = `v26 = band.i32 v2, v13 ; v13 = 3`, type = `Some(types::I32)`
Progress! This is a different error than before. Now it's time to rinse and
repeat these steps. Be sure to skim the rest of lower.isle for inspiration on
how to implement lowering rules. You can also look at lower.isle for other
architecture backends too for inspiration.
Flagging a test as passing
After implementing a lowering for band.i32 our test case is now passing:
$ cargo run --features pulley -- wast --target pulley64 ./tests/misc_testsuite/control-flow.wast
Finished `dev` profile [unoptimized + debuginfo] target(s) in 13.50s
Running `target/debug/wasmtime wast --target pulley64 ./tests/misc_testsuite/control-flow.wast`
If we run the test suite though we'll see:
$ cargo test --test wast control-flow.wast
Finished `test` profile [unoptimized + debuginfo] target(s) in 29.14s
Running tests/wast.rs (target/debug/deps/wast-f83a3ee5e5dbacde)
running 6 tests
...F.F
failures:
---- CraneliftPulley/./tests/misc_testsuite/control-flow.wast ----
this test is flagged as should-fail but it succeeded
---- CraneliftPulley/pooling/./tests/misc_testsuite/control-flow.wast ----
this test is flagged as should-fail but it succeeded
failures:
CraneliftPulley/./tests/misc_testsuite/control-flow.wast
CraneliftPulley/pooling/./tests/misc_testsuite/control-flow.wast
test result: FAILED. 4 passed; 2 failed; 0 ignored; 0 measured; 4086 filtered out; finished in 0.05s
error: test failed, to rerun pass `--test wast`
This indicates that the test was previously flagged as "should fail", but that
assertion is no longer true! Update the WastTest::should_fail method in
crates/wast-util/src/lib.rs so that it expects the test to pass by deleting
the tests from the unsupported list. Then we'll see:
$ cargo test --test wast control-flow.wast
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.74s
Running tests/wast.rs (target/debug/deps/wast-f83a3ee5e5dbacde)
running 6 tests
......
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 4086 filtered out; finished in 0.05s
Success!
But we aren't quite done yet: the new lowerings we added might have made
additional tests that previously failed start passing as well. If so, then we
also want to mark those tests as expected to pass now. We can double check by
running the full wast test suite:
$ cargo test --test wast Pulley
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.74s
Running tests/wast.rs (target/debug/deps/wast-f83a3ee5e5dbacde)
running 1364 tests
....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
test result: ok. 1364 passed; 0 failed; 0 ignored; 0 measured; 2728 filtered out; finished in 0.93s
Alas, maybe next time!
Clean up and make a PR
All that's left now is to clean things up, document anything necessary, and make a pull request.
To view the complete pull request that implemented xdiv32_s and xand32
Pulley instructions and got ./tests/misc_testsuite/control-flow.wast passing
(and also introduced this documentation) check out
#9765.
Thanks for helping out!