Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the tail calling convention by default #8540

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions crates/wasmtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,23 @@ impl Config {
self
}

pub(crate) fn conditionally_enable_defaults(&mut self) {
// Enable tail calls by default when cranelift is going to be used as the compilation
// strategy, and we're not targeting s390x, which currently lacks tail-call support.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind also editing this to cover that this logic is only applicable if tail calls were not otherwise explicitly disabled or enabled? (aka explaining the .is_none())

if self.tunables.tail_callable.is_none() {
#[cfg(feature = "cranelift")]
let default_tail_calls = self.compiler_config.strategy == Strategy::Cranelift
&& self.compiler_config.target.as_ref().map_or_else(
|| target_lexicon::Triple::host().architecture,
|triple| triple.architecture,
) != Architecture::S390x;
#[cfg(not(feature = "cranelift"))]
let default_tail_calls = false;

self.wasm_tail_call(default_tail_calls);
}
}

pub(crate) fn validate(&self) -> Result<Tunables> {
if self.features.contains(WasmFeatures::REFERENCE_TYPES)
&& !self.features.contains(WasmFeatures::BULK_MEMORY)
Expand Down
7 changes: 6 additions & 1 deletion crates/wasmtime/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ impl Engine {
crate::runtime::vm::debug_builtins::ensure_exported();
}

let config = config.clone();
let config = {
let mut config = config.clone();
config.conditionally_enable_defaults();
config
};

let tunables = config.validate()?;

#[cfg(any(feature = "cranelift", feature = "winch"))]
Expand Down
62 changes: 62 additions & 0 deletions tests/all/defaults.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use wasmtime::*;

#[test]
fn test_tail_call_default() -> Result<()> {
for (expected, cfg) in [
(
true,
Config::new()
.strategy(Strategy::Cranelift)
.target("x86_64")?,
),
(
true,
Config::new()
.strategy(Strategy::Cranelift)
.target("aarch64")?,
),
(
true,
Config::new()
.strategy(Strategy::Cranelift)
.target("riscv64")?,
),
(
false,
Config::new()
.strategy(Strategy::Cranelift)
.target("s390x")?,
),
(
false,
Config::new().strategy(Strategy::Winch).target("x86_64")?,
),
(
false,
Config::new().strategy(Strategy::Winch).target("aarch64")?,
),
(
false,
Config::new()
.strategy(Strategy::Cranelift)
.wasm_tail_call(false)
.target("x86_64")?,
),
] {
let engine = Engine::new(cfg)?;

let wat = r#"
(module $from_name_section
(func (export "run") (return_call 0))
)
"#;

let result = engine.precompile_module(wat.as_bytes()).map(|_| ());

eprintln!("for config {cfg:?}, got: {result:?}");

assert_eq!(expected, result.is_ok());
}

Ok(())
}
1 change: 1 addition & 0 deletions tests/all/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod code_too_large;
mod component_model;
mod coredump;
mod debug;
mod defaults;
mod epoch_interruption;
mod externals;
mod fuel;
Expand Down
Loading