Stream: wasmtime

Topic: cranelift-wasm vs wasmtime-environ ModuleEnvironment


view this post on Zulip bjorn3 (May 16 2023 at 15:01):

cranelift-wasm has a ModuleEnvironment trait, while wasmtime-environ has a ModuleEnvironment struct. Both seem to independently parse wasm modules. Why does this split exist and when is which one used? I'm getting confused where I have to implement support for handling the tags section used by the exception handling proposal.

view this post on Zulip Alex Crichton (May 16 2023 at 15:05):

Long ago the trait was used to multiplex the spidermonkey embedding and the Wasmtime embedding. Since then Spidermonkey stopped using it and I also had a patch awhile back to have Wasmtime stop using it because it got quite onerous to be updated. At the time it was decided not to remove it, so it's sort of in a zombie state nowadays. I think it may be used in some cranelift testing still which is why it sticks around, but otherwise I don't think anyone really wants to keep it any more at this point

view this post on Zulip Alex Crichton (May 16 2023 at 15:05):

If it's still only in use by the tests I think it's ok to insert panics/todo!()/etc

view this post on Zulip bjorn3 (May 16 2023 at 15:06):

I see. Thanks.

view this post on Zulip bjorn3 (May 25 2023 at 16:30):

Would it make sense to move all cranelift/filetests/filetests/wasm tests to either wasmtime or replace them with .clif tests? That would allow fully merging cranelift-wasm into wasmtime-environ.

view this post on Zulip Alex Crichton (May 25 2023 at 16:53):

I'd personally be in favor of that, but I'm a bit biased in that I don't like maintaining the dummy.rs environment traits. I do think that integrating with Wasmtime would be nice to additionally reflect Wasmtime's exact implementations of the func_environ.rs trait hooks which currently can't have assembly references tested

view this post on Zulip Chris Fallin (May 25 2023 at 16:53):

When we added this mode the idea was to be able to test specific aspects of the wasm translation without a full dependence on Wasmtime, and to be able to assert on the generated assembly. I guess we could add equivalent test functionality to Wasmtime itself but assertions on the assembly output wouldn't fit too well with the wast test infra, which is otherwise compiler-backend-agnostic. In other words it's kind of a distinct need that led to the distinct tier of tests

view this post on Zulip Chris Fallin (May 25 2023 at 16:55):

although, perhaps this would allow for a different factoring / sharing of effort wrt the Winch assembly tests: a new Wasmtime-level test file directive could assert "with this backend, this should be the assembly output"

view this post on Zulip bjorn3 (May 25 2023 at 17:31):

Can't we add a test mode for wasmtime tests which asserts clif ir? It would then use the real translation code as used by wasmtime, but stop before compiling it down to machine code.

view this post on Zulip Chris Fallin (May 25 2023 at 17:34):

We do want specifically to assert on the assembly in some cases; e.g. @fitzgen (he/him) added tests that show that heap-access bounds checks are properly optimized by Cranelift's mid-end, and we probably want to retain the end-to-end testing of efficient lowering there

view this post on Zulip Chris Fallin (May 25 2023 at 17:35):

so maybe a "compiler test" mode in a wast could be something like: assert CLIF or Cranelift compiled output or Winch compiled output

view this post on Zulip Chris Fallin (May 25 2023 at 17:35):

FWIW, on broader context here: I do generally like the idea of removing indirection where we can; the distinction between the environ, the generic wasm translator, and Wasmtime's hooks was pretty confusing to me when was first working on CL/Wasmtime

view this post on Zulip Jamey Sharp (May 25 2023 at 17:37):

I was just yesterday trying to figure out how to optimize constant-sized table bounds-checks in Wasmtime and am now confused by this too, so I'm also in favor of cleaning it up somehow :laughing:

view this post on Zulip Alex Crichton (May 25 2023 at 17:38):

I don't think that adding this test infra to Wasmtime would be intractable, but I definitely agree it's not easily slotted in

view this post on Zulip Alex Crichton (May 25 2023 at 17:39):

I do think though the best world is that we test what we actually want to, which is Wasmtime's lowerings, which in the case of like heap accesses and stuff needs to pull in all the support from the wasmtime-cranelft crate which is doing the lowering and classifying the kinds of memories


Last updated: Nov 22 2024 at 17:03 UTC