Testing

[kripken]

Still early here, but might be nice to add testing at some point.

One option is to check output .wast files for identity.

Another is to also execute them, for example, testing that fibonacci(5) has the right output. This is actually not that hard, we need to

1. Add imports for testing prints, (import $print_i32 "spectest" "print" (param i32)) for printing an i32, for example. The spectest module is a special module that is supported in wasm test runners (it won't be supported on the web, but could be polyfilled).
2. Call the import using call_import to print values.
3. Add a start method (BinaryenSetStart), a void(void) function that is called when the module is loaded.
4. Execute that webassembly using the binaryen shell, which should already be present since we build binaryen. Something like binaryen-shell test.wast will load the module and run the start method.
5. Verify the output is as expected.

[lqd]

I was about to add an issue on this topic as well :)

1. absolutely, and easy to do: the "main" void(void)" Rust function would be perfect for start
2. additionally, it would be interesting to validate the module during generation, at the very least during development. (But it seems I'm getting some weird errors right now, possibly related to the VS2015 ones: on the same module, BinaryenModuleValidate returns false and prints error messages, while binaryen-shell doesn't mention a problem if I feed it the wast. Not that big of a deal :)
3. Maybe launching the interpreter directly from our compilation process ? (if there's no way to do that through the API, we can call the tools ourselves, they're accessible in ./target/$mode/build/mir2wasm-$RANDOM/out/bin)
4. I also used assert_returns (but I don't believe there's a way to emit those from the binaryen API ?)
5. It would also interesting to keep in mind throughout testing the browsers which will run the code. IIRC emscripten's compilation also provides scaffolding, eg html files. In the same vein, we should be able to output assembled wasm files, and a way to run the compiled output in a browser (even though everything is very much in flux right now AFAICT. In my limited testing I couldn't get to validate the wasm modules produced here, probably because of spec support differences with such a fast moving target)

Point 1 at the very least makes me wonder about "wasm intrinsics" such as the print or assert functions, and how to model them from Rust, maybe @brson already had thoughts here ?

[kripken]

2. Strange, validation should be identical in the shell and in memory. Unless writing it out changed something, which sounds like a binaryen bug. Can you print out the wast in memory and paste that + the validation errors? Even better would be if there is an easy way to print out the C API calls that are made (thus generating a compilable C testcase reproducing the problem)?

3. Yes, assert_return is not actually part of wasm, nor part of the module. It's just a hack for wasm test runners, basically, so not part of the binaryen C API. But we could append them in text before running the shell on a wast. However, that is liable to change (wasm text format will change a lot), so might be better to avoid assert_return.

4. Yeah, spec changes are still frequent enough that I doubt it's worth testing in a native wasm engine like a browser. For now the binaryen interpreter should be enough for testing, it will be updated in lockstep with the binaryen C API that we use here.

[lqd]

@kripken here's the validation errors https://gist.github.com/lqd/9a1bc7548b2841495ecb4b2a4e397cf8
and the module which I print right after validation https://gist.github.com/lqd/9752e7facd424fc006598be105952625

it could just be some vs2015 shenanigans (like the linker removing your code...)

getting the C api calls might take a bit more time

[kripken]

Thanks, I see what's going wrong here, it's an interaction between the relooper and return statements. Testcase reproducing the problem + fix are in WebAssembly/binaryen#600

[brson]

Point 1 at the very least makes me wonder about "wasm intrinsics" such as the print or assert functions, and how to model them from Rust, maybe @brson already had thoughts here ?

I imagine both wasm-specific intrinsics and emscripten runtime calls will require modifying the compiler in someway. For wasm intrinsics it may be appropriate to define them as rustc 'platform intrinsics'. I don't know exactly how they are defined but this PR is adding a bunch.

In the short term I think it's best to hack around it and not try to solve the problem upstream. As an example of a hacky way to do it, mir2wasm could transform calls to the following into calls to an intrinsic:

#[binaryen_intrinsic]
extern {
    fn print(...);
}

That's the same as a typical rust intrinsic declaration, but it's got an attribute on it that tells mir2wasm to do its own magic.

[brson]

Add imports for testing prints, (import $print_i32 "spectest" "print" (param i32)) for printing an i32, for example. The spectest module is a special module that is supported in wasm test runners (it won't be supported on the web, but could be polyfilled).

If these behave like C functions then in Rust we can probably declare them as regular "C" extern fns.

Call the import using call_import to print values.
Add a start method (BinaryenSetStart), a void(void) function that is called when the module is loaded.

If this is a different entry point than normal compilation I suggest it be done by having mir2wasm interpret a custom attribute, like #[binaryen_test_start].

Execute that webassembly using the binaryen shell, which should already be present since we build binaryen. Something like binaryen-shell test.wast will load the module and run the start method.
Verify the output is as expected.

Setting up the kind of testing we need here, where each test is a single Rust source file that is processed on some arbitrary way, will take some customization of Rust's test framework. The way I might set this up is to make a directory of test cases (not src/tests since cargo interprets that in a specific way) that contain a bunch of sources, annotated with special comments indicating the expected output. Then in a build script, read that directory and generate a test/spectests.rs containing typical Rust #[test] cases that issue the actual commands to check the source. It's a bit convoluted. I'd also be fine with hacking it up any other way, even just a Python script to start.

[kripken]

If these behave like C functions then in Rust we can probably declare them as regular "C" extern fns.

Yes, they behave exactly as C externs.

[lqd]

Then maybe something like

mod wasm {
    extern {
        pub fn print_i32(i: i32);
    }
}

so we'd just call it like unsafe { wasm::print_i32(1); } ?

[brson]

@lqd that looks right to me. I'd expect you to write that code by hand in the Rust source, and mir2wasm to generate a C-ABI call to an external function, just like any other C call. It'll probably involve calling some binaryen function to declare an extern with that name, then using the normal function call path to call it.

[kripken]

Seems like we could translate all externs like that into wasm imports, which is what external callable code is, basically. The only extra thing is that imports use a 2-level namespace, and for test printing, the module is spectest. So that extern needs to turn into an import of print_i32 from spectest.

And more generally, for importing other wasm modules, it would be nice to be able to specify for a rust extern which module it is from, then the translation to a wasm import is clear. Could that be annotated somehow in the rust source?

[lqd]

@brson indeed, and I already have the "import, and call print" part working, I'll just need to look for the extern in the mir blocks

[lqd]

Being able to run the interpreter directly after compiling would now be super useful, with print_i32 and start support from this PR

[kripken]

Ok, adding that to the binaryen c api in WebAssembly/binaryen#601

[lqd]

@kripken oooh sorry I meant doing it manually in mir2wasm like I mentioned on IRC, launching binaryen-shell after the build. It'll however be helpful for testing for sure, so thank you :)
I think running the shell binary has value, like choosing the optimization passes and so on, so we could probably still do that for the other use cases

[kripken]

No problem, I've been thinking about it since yesterday anyhow. My main concern was code size in libbinaryen, but I realized that we're going to need the interpreter in there anyhow for other reasons (planned optimizations). So there is really no downside, and it was just 10 minutes of work to add :)

[axic]

@kripken any clear ideas yet on how to support imports? Will it support defining the wasm module also?

Currently it seems like I need to add code manually to trans.rs.

[kripken]

@axic: I think @lqd had a way to annotate rust code to define imports. Basically a C-style import should just define the module and base names, in the source, and then when generating in binaryen an import can be created with those, and some internal name that is used for calls to it.

[lqd]

@axic yeah right now only the spectest's print works and is harcoded in trans.
Eventually we'll make function imports: 1) extensible, 2) actually work — using either existing machinery around FFI, extern and the likes (say, link/link_args) or maybe roll our own if we have to (which is probable). In the meantime I've opened #33 to track the issue so we can talk more about it there.

[axic]

@kripken @lqd thanks! Will keep an eye on #33.