Support compilation-only build by adding a runtime feature

[adambratschikaye]

Initial draft of changes discussed here: #7652

[github-actions]

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "cranelift", "wasmtime:api", "wasmtime:config"

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

• peterhuene: wasmtime:api

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

Learn more.

[github-actions]

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

• If you added a new Config method, you wrote extensive documentation for
  it.

  Our documentation should be of the following form:

  Short, simple summary sentence.
  
  More details. These details can be multiple paragraphs. There should be
  information about not just the method, but its parameters and results as
  well.
  
  Is this method fallible? If so, when can it return an error?
  
  Can this method panic? If so, when does it panic?
  
  # Example
  
  Optional example here.
  

• If you added a new Config method, or modified an existing one, you
  ensured that this configuration is exercised by the fuzz targets.

  For example, if you expose a new strategy for allocating the next instance
  slot inside the pooling allocator, you should ensure that at least one of our
  fuzz targets exercises that new strategy.

  Often, all that is required of you is to ensure that there is a knob for this
  configuration option in wasmtime_fuzzing::Config (or one
  of its nested structs).

  Rarely, this may require authoring a new fuzz target to specifically test this
  configuration. See our docs on fuzzing for more details.

• If you are enabling a configuration option by default, make sure that it
  has been fuzzed for at least two weeks before turning it on by default.

---

To modify this label's message, edit the .github/label-messager/wasmtime-config.md file.

To add new label messages or remove existing label messages, edit the
.github/label-messager.json configuration file.

Learn more.

[github-actions]

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasi", "wasmtime:c-api"

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

• peterhuene: wasmtime:c-api

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

Learn more.

[adambratschikaye]

Ok, I've made most of the suggested changes:

• Could the runtime module, at its root, have everything reexported of it's internals? My hope would be that in lib.rs we'd have:

  #[cfg(feature = "runtime")]
  mod runtime;
  #[cfg(feature = "runtime")]
  pub use runtime::*;

  I think cargo fmt has issues with modules defined via macro expansions so moving the mod runtime out of a macro may benefit that niche use case.

Yes this seems to be cleaner.

• I think it's ok to move some more things into runtime that are technically agnostic like limits, resources, and stack.

I moved these three as well as coredump and debug.

• Can you add a builder to CI to build the wasmtime crate for the wasm32-wasi target with the runtime feature disabled?

I added an extra build job in the CI (that runs on x86 linux). Hope this makes sense within the general CI setup of the project.

• Splitting engine.rs feels not great since it's now so spread out (and requires a large struct with a bunch of pub(crate) which never feels great. This might be something where it may be worth some more #[cfg]. Do you know if a design such as this would work?

  impl Engine { /* common methods */ }
  #[cfg(feature = "runtime")] 
  impl Engine { /* runtime methods */ }
  #[cfg(feature = "compile")] 
  impl Engine { /* compilation methods */ }

  that might help keep everything in a single file, albeit there'd probably be a lot of #[cfg] related to use directives at the top of the file perhaps.

Yep this works fine, except I didn't add a compile feature. It would just be the same as any(feature = "cranelift", feature = "winch") so I just used that everywhere.

• Do you think it would make sense to have a top level mod compile; with all compilation-related bits under that? Right now it's sort of spread out at the root it seems.

I put some things that were at the top-level under the compile module. But there's still a bunch of compilation specific stuff under cfg directives in the runtime module. Many of these seem like they wouldn't make sense to move over to the compile because they rely on types that wouldn't be used without the runtime feature. For example, Func::new requires compilation, but I don't see how it could be used without the runtime feature (there are similar examples for Linker, Module, and Component).

[adambratschikaye]

Can you paste an example of buliding wasmtime like this? I checked out your repo and pointed it at this branch and this passed:

$ wasm-tools validate ./target/wasm32-wasi/release/wasmtime-in-wasm.wasm

but I expected that it might otherwise fail

Ah yep, that's working for me as well. Doing the debug build hits the limit though:

❯ cargo build
    Finished dev [unoptimized + debuginfo] target(s) in 0.16s

❯ wasm-tools validate target/wasm32-wasi/debug/wasmtime-in-wasm.wasm 
error: func 35030 failed to validate

Caused by:
    0: too many locals: locals exceed maximum (at offset 0x111420c)

Would it make sense to add that code as an example along with a CI job do the compilation in Wasm and then execute on the host system? And would you want that job to run on all hosts in the matrix?

[alexcrichton]

Ok thanks again for you work on this, it's all looking great to me! I've got some minor nits and things but I think it's probably best to go ahead and land this and I can follow-up myself with anything else I'd like. I'd also like to get the wasmtime CLI itself compiling to wasm with just the cranelift/winch features enabled, but no need to do that here either! I think there's a rebase conflict but otherwise I'm happy to throw this at the merge queue.

Would it make sense to add that code as an example along with a CI job do the compilation in Wasm and then execute on the host system? And would you want that job to run on all hosts in the matrix?

I think so yeah, but I'll do that as part of a follow-up to get the wasmtime CLI itself compiling. I tried for a bit to figure out what function was causing issues unsuccessfully, but this is also ok to work on as a follow-up.

My guess is that it's a function in cranelift-codegen for lowering since that's the main source of "big generated code". I have yet to confirm this though.

[alexcrichton]

Ah ok so I found the function:

$ wasm-tools validate ./target/wasm32-wasi/debug/wasmtime-in-wasm.wasm
error: func 39100 failed to validate

Caused by:
    0: too many locals: locals exceed maximum (at offset 0x12ebabe)
$ wasm-tools print ./target/wasm32-wasi/debug/wasmtime-in-wasm.wasm --skeleton | rg 39100
  (func $_ZN4core6option19Option$LT$$RF$T$GT$6cloned17h3ad039100bcc27f6E (;11910;) (type 2) (param i32 i32) ...)
  (func $_ZN17cranelift_codegen2ir7builder11InstBuilder8f64const17h4e1391004dd942baE (;18861;) (type 4) (param i32 i32) (result i32) ...)
  (func $_ZN17cranelift_codegen4opts14generated_code20constructor_simplify17h2e155f4b5f5d37f9E (;39100;) (type 9) (param i32 i32 i32) ...)
$ rustfilt _ZN17cranelift_codegen4opts14generated_code20constructor_simplify17h2e155f4b5f5d37f9E
cranelift_codegen::opts::generated_code::constructor_simplify

That corresponds to the ISLE-generated code for optimizing CLIF. I'm not personally aware of any low-hanging fruit on optimizing the output of ISLE for debug builds, so this may be a case where cranelift-codegen the crate needs to be optimized, even in debug builds.

[adambratschikaye]

Ok thanks again for you work on this, it's all looking great to me! I've got some minor nits and things but I think it's probably best to go ahead and land this and I can follow-up myself with anything else I'd like. I'd also like to get the wasmtime CLI itself compiling to wasm with just the cranelift/winch features enabled, but no need to do that here either! I think there's a rebase conflict but otherwise I'm happy to throw this at the merge queue.

I resolved the conflicts. Happy to follow up with compiling the CLI to wasm as well.

[alexcrichton]

Ok sounds good! I don't necessarily have a concrete use case in mind but it seems like a neat way to demo how Cranelift can be compiled to wasm. I'll poke a bit more at the internals of the wasmtime crate and leave the CLI bits to you.

[alexcrichton]

I was curious to play around with this and went a bit far down the rabbit hole! I was able to hack things up and get things working though with #7842 and #7844, however. That's perhaps a bit of a taste of how cross-compilation, especially across pointer widths, is not the most well trodden path in Wasmtime so there may be a few other lingering bugs. Mostly just something to be aware of, and I'd hope that we can start adding regression tests in CI once we've got CLI support!