# Contributing For general contribution to Wasmtime, see Wasmtime's [contributing docs][docs]. [docs]: https://docs.wasmtime.dev/contributing.html ## 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: ```rust /// `dst = src1 / src2` (signed) xdiv32_s = XDiv32S { operands: BinaryOperands }; ``` 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: ```rust fn xdiv32_s(&mut self, operands: BinaryOperands) -> ControlFlow { 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::(), } } ``` 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: ```lisp (rule (lower ) ) ``` This means "when lowering Cranelift's mid-level IR down to Pulley bytecode, and we match the snippet ``, replace it with ``". (For more details, see the [ISLE Language Reference](https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/isle/docs/language-reference.md) and [How ISLE is Integrated with Cranelift](https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/docs/isle-integration.md)) 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](https://github.com/bytecodealliance/wasmtime/pull/9765). Thanks for helping out!