Repository: bytecodealliance/wasmtime
Pull request #8543 (2024-05-03 18:40:32)

Indirect call caching: put options under `-O` namespace.
As per [this comment], the call-indirect caching options should really be in the `-O` option namespace (for optimizations), not `-W` (for semantically-visible Wasm standards options); the original PR simply put them in the wrong place, and this PR moves them.

[this comment]: https://github.com/bytecodealliance/wasmtime/pull/8509#discussion_r1589594152

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/crates/cli-flags/src/lib.rs b/crates/cli-flags/src/lib.rs
index 320b76e6c..985993e4c 100644
--- a/crates/cli-flags/src/lib.rs
+++ b/crates/cli-flags/src/lib.rs
@@ -101,6 +101,17 @@ wasmtime_option_group! {
         /// The maximum number of WebAssembly stacks which can be created with
         /// the pooling allocator.
         pub pooling_total_stacks: Option<u32>,
+
+        /// Whether to enable call-indirect caching.
+        pub cache_call_indirects: Option<bool>,
+
+        /// The maximum call-indirect cache slot count.
+        ///
+        /// One slot is allocated per indirect callsite; if the module
+        /// has more indirect callsites than this limit, then the
+        /// first callsites in linear order in the code section, up to
+        /// the limit, will receive a cache slot.
+        pub max_call_indirect_cache_slots: Option<usize>,
     }
 
     enum Optimize {
@@ -244,15 +255,6 @@ wasmtime_option_group! {
         pub function_references: Option<bool>,
         /// Configure support for the GC proposal.
         pub gc: Option<bool>,
-        /// Whether to enable call-indirect caching.
-        pub cache_call_indirects: Option<bool>,
-        /// The maximum call-indirect cache slot count.
-        ///
-        /// One slot is allocated per indirect callsite; if the module
-        /// has more indirect callsites than this limit, then the
-        /// first callsites in linear order in the code section, up to
-        /// the limit, will receive a cache slot.
-        pub max_call_indirect_cache_slots: Option<usize>,
     }
 
     enum Wasm {
@@ -553,10 +555,10 @@ impl CommonOptions {
         if let Some(enable) = self.opts.memory_init_cow {
             config.memory_init_cow(enable);
         }
-        if let Some(enable) = self.wasm.cache_call_indirects {
+        if let Some(enable) = self.opts.cache_call_indirects {
             config.cache_call_indirects(enable);
         }
-        if let Some(max) = self.wasm.max_call_indirect_cache_slots {
+        if let Some(max) = self.opts.max_call_indirect_cache_slots {
             config.max_call_indirect_cache_slots(max);
         }
 
diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs
index e063c2650..c7cf11958 100644
--- a/crates/wasmtime/src/config.rs
+++ b/crates/wasmtime/src/config.rs
@@ -1708,6 +1708,24 @@ impl Config {
         self
     }
 
+    pub(crate) fn conditionally_enable_defaults(&mut self) {
+        // If tail calls were not explicitly enabled/disabled (i.e. tail_callable is None), enable
+        // them if we are targeting a backend that supports them. Currently the Cranelift
+        // compilation strategy is the only one that supports tail calls, but not targeting s390x.
+        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)
diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs
index 540aa35c5..9b0453cf9 100644
--- a/crates/wasmtime/src/engine.rs
+++ b/crates/wasmtime/src/engine.rs
@@ -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"))]


Test patch:
diff --git a/tests/all/defaults.rs b/tests/all/defaults.rs
new file mode 100644
index 000000000..0e869e567
--- /dev/null
+++ b/tests/all/defaults.rs
@@ -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(())
+}
diff --git a/tests/all/main.rs b/tests/all/main.rs
index f778edfde..62aaf46ce 100644
--- a/tests/all/main.rs
+++ b/tests/all/main.rs
@@ -7,6 +7,7 @@ mod code_too_large;
 mod component_model;
 mod coredump;
 mod debug;
+mod defaults;
 mod epoch_interruption;
 mod externals;
 mod fuel;
diff --git a/tests/disas/indirect-call-caching-exclude-0-index.wat b/tests/disas/indirect-call-caching-exclude-0-index.wat
index 6744cf7b8..93b31f59b 100644
--- a/tests/disas/indirect-call-caching-exclude-0-index.wat
+++ b/tests/disas/indirect-call-caching-exclude-0-index.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y" ]
+;;! flags = [ "-Ocache-call-indirects=y" ]
 
 ;; This test checks that we do *not* get the indirect-call caching optimization
 ;; when it must not be used: in this case, because there is a non-null element
diff --git a/tests/disas/indirect-call-caching-exclude-table-export.wat b/tests/disas/indirect-call-caching-exclude-table-export.wat
index 30ca6f0d5..e9434c14c 100644
--- a/tests/disas/indirect-call-caching-exclude-table-export.wat
+++ b/tests/disas/indirect-call-caching-exclude-table-export.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y" ]
+;;! flags = [ "-Ocache-call-indirects=y" ]
 
 ;; This test checks that we do *not* get the indirect-call caching optimization
 ;; when it must not be used: in this case, because the table is exported so
diff --git a/tests/disas/indirect-call-caching-exclude-table-writes.wat b/tests/disas/indirect-call-caching-exclude-table-writes.wat
index d167a16ee..a05a28888 100644
--- a/tests/disas/indirect-call-caching-exclude-table-writes.wat
+++ b/tests/disas/indirect-call-caching-exclude-table-writes.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y" ]
+;;! flags = [ "-Ocache-call-indirects=y" ]
 
 ;; This test checks that we do *not* get the indirect-call caching optimization
 ;; when it must not be used: in this case, because the table is updated with a
diff --git a/tests/disas/indirect-call-caching-slot-limit-1.wat b/tests/disas/indirect-call-caching-slot-limit-1.wat
index 937c0f9fe..afd08e507 100644
--- a/tests/disas/indirect-call-caching-slot-limit-1.wat
+++ b/tests/disas/indirect-call-caching-slot-limit-1.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y", "-Wmax-call-indirect-cache-slots=2" ]
+;;! flags = [ "-Ocache-call-indirects=y", "-Omax-call-indirect-cache-slots=2" ]
 
 ;; This test checks that we properly bound the number of call-indirect
 ;; cache slots. The first case (here) is when the limit falls in the
diff --git a/tests/disas/indirect-call-caching-slot-limit-2.wat b/tests/disas/indirect-call-caching-slot-limit-2.wat
index a8b89451c..4b62ff0f9 100644
--- a/tests/disas/indirect-call-caching-slot-limit-2.wat
+++ b/tests/disas/indirect-call-caching-slot-limit-2.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y", "-Wmax-call-indirect-cache-slots=2" ]
+;;! flags = [ "-Ocache-call-indirects=y", "-Omax-call-indirect-cache-slots=2" ]
 
 ;; This test checks that we properly bound the number of call-indirect
 ;; cache slots. The second case (here) is when the limit falls
diff --git a/tests/disas/indirect-call-caching.wat b/tests/disas/indirect-call-caching.wat
index 5ca21a148..0e19aa79a 100644
--- a/tests/disas/indirect-call-caching.wat
+++ b/tests/disas/indirect-call-caching.wat
@@ -1,5 +1,5 @@
 ;;! target = "x86_64"
-;;! flags = [ "-Wcache-call-indirects=y" ]
+;;! flags = [ "-Ocache-call-indirects=y" ]
 
 ;; This test checks that we get the indirect-call caching optimization
 ;; where it should be applicable (immutable table, null 0-index).