Fix tail calls being turned on by default (#8682)

* Fix tail calls being turned on by default

Logic in `Config` to conditionally enable tail calls wasn't handling the
case where the configured compiler strategy was `Strategy::Auto` meaning
that by default tail calls weren't actually enabled. This commit
refactors handling of `Strategy` to avoid storing `Strategy::Auto` in
`CompilerConfig` so tests against it can use either cranelift or winch.

* Update disas tests

* Update Winch tests to using winch

crates/wasmtime/src/config.rs

@@ -162,7 +162,7 @@ struct ConfigTunables {
 #[cfg(any(feature = "cranelift", feature = "winch"))]
 #[derive(Debug, Clone)]
 struct CompilerConfig {
-    strategy: Strategy,
+    strategy: Option<Strategy>,
     target: Option<target_lexicon::Triple>,
     settings: HashMap<String, String>,
     flags: HashSet<String>,
@@ -174,9 +174,9 @@ struct CompilerConfig {
 
 #[cfg(any(feature = "cranelift", feature = "winch"))]
 impl CompilerConfig {
-    fn new(strategy: Strategy) -> Self {
+    fn new() -> Self {
         Self {
-            strategy,
+            strategy: Strategy::Auto.not_auto(),
             target: None,
             settings: HashMap::new(),
             flags: HashSet::new(),
@@ -210,7 +210,7 @@ impl CompilerConfig {
 #[cfg(any(feature = "cranelift", feature = "winch"))]
 impl Default for CompilerConfig {
     fn default() -> Self {
-        Self::new(Strategy::Auto)
+        Self::new()
     }
 }
 
@@ -721,7 +721,8 @@ impl Config {
     /// programs to implement some recursive algorithms with *O(1)* stack space
     /// usage.
     ///
-    /// This feature is disabled by default.
+    /// This is `true` by default except on s390x or when the Winch compiler is
+    /// enabled.
     ///
     /// [WebAssembly tail calls proposal]: https://github.com/WebAssembly/tail-call
     pub fn wasm_tail_call(&mut self, enable: bool) -> &mut Self {
@@ -1023,7 +1024,7 @@ impl Config {
     /// The default value for this is `Strategy::Auto`.
     #[cfg(any(feature = "cranelift", feature = "winch"))]
     pub fn strategy(&mut self, strategy: Strategy) -> &mut Self {
-        self.compiler_config.strategy = strategy;
+        self.compiler_config.strategy = strategy.not_auto();
         self
     }
 
@@ -1745,7 +1746,7 @@ impl Config {
         // 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
+            let default_tail_calls = self.compiler_config.strategy == Some(Strategy::Cranelift)
                 && self.compiler_config.target.as_ref().map_or_else(
                     || target_lexicon::Triple::host().architecture,
                     |triple| triple.architecture,
@@ -1835,11 +1836,7 @@ impl Config {
         // If we're going to compile with winch, we must use the winch calling convention.
         #[cfg(any(feature = "cranelift", feature = "winch"))]
         {
-            tunables.winch_callable = match self.compiler_config.strategy {
-                Strategy::Auto => !cfg!(feature = "cranelift") && cfg!(feature = "winch"),
-                Strategy::Cranelift => false,
-                Strategy::Winch => true,
-            };
+            tunables.winch_callable = self.compiler_config.strategy == Some(Strategy::Winch);
 
             if tunables.winch_callable && tunables.tail_callable {
                 bail!("Winch does not support the WebAssembly tail call proposal");
@@ -1918,17 +1915,15 @@ impl Config {
 
         let mut compiler = match self.compiler_config.strategy {
             #[cfg(feature = "cranelift")]
-            Strategy::Auto => wasmtime_cranelift::builder(target)?,
-            #[cfg(all(feature = "winch", not(feature = "cranelift")))]
-            Strategy::Auto => wasmtime_winch::builder(target)?,
-            #[cfg(feature = "cranelift")]
-            Strategy::Cranelift => wasmtime_cranelift::builder(target)?,
+            Some(Strategy::Cranelift) => wasmtime_cranelift::builder(target)?,
             #[cfg(not(feature = "cranelift"))]
-            Strategy::Cranelift => bail!("cranelift support not compiled in"),
+            Some(Strategy::Cranelift) => bail!("cranelift support not compiled in"),
             #[cfg(feature = "winch")]
-            Strategy::Winch => wasmtime_winch::builder(target)?,
+            Some(Strategy::Winch) => wasmtime_winch::builder(target)?,
             #[cfg(not(feature = "winch"))]
-            Strategy::Winch => bail!("winch support not compiled in"),
+            Some(Strategy::Winch) => bail!("winch support not compiled in"),
+
+            None | Some(Strategy::Auto) => unreachable!(),
         };
 
         if let Some(path) = &self.compiler_config.clif_dir {
@@ -2219,6 +2214,23 @@ pub enum Strategy {
     Winch,
 }
 
+impl Strategy {
+    fn not_auto(&self) -> Option<Strategy> {
+        match self {
+            Strategy::Auto => {
+                if cfg!(feature = "cranelift") {
+                    Some(Strategy::Cranelift)
+                } else if cfg!(feature = "winch") {
+                    Some(Strategy::Winch)
+                } else {
+                    None
+                }
+            }
+            other => Some(*other),
+        }
+    }
+}
+
 /// Possible optimization levels for the Cranelift codegen backend.
 #[non_exhaustive]
 #[derive(Copy, Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]

tests/all/module.rs

@@ -297,3 +297,36 @@ fn call_indirect_caching_out_of_bounds_table_index() -> Result<()> {
     );
     Ok(())
 }
+
+#[test]
+fn tail_call_defaults() -> Result<()> {
+    let wasm_with_tail_calls = "(module (func $a return_call $a))";
+    if cfg!(target_arch = "s390x") {
+        // off by default on s390x
+        let res = Module::new(&Engine::default(), wasm_with_tail_calls);
+        assert!(res.is_err());
+    } else {
+        // on by default
+        Module::new(&Engine::default(), wasm_with_tail_calls)?;
+
+        // on by default for cranelift
+        Module::new(
+            &Engine::new(Config::new().strategy(Strategy::Cranelift))?,
+            wasm_with_tail_calls,
+        )?;
+    }
+
+    if cfg!(target_arch = "x86_64") {
+        // off by default for winch
+        let err = Module::new(
+            &Engine::new(Config::new().strategy(Strategy::Winch))?,
+            wasm_with_tail_calls,
+        );
+        assert!(err.is_err());
+
+        // can't enable with winch
+        let err = Engine::new(Config::new().strategy(Strategy::Winch).wasm_tail_call(true));
+        assert!(err.is_err());
+    }
+    Ok(())
+}

tests/disas/arith.wat

@@ -14,7 +14,7 @@
   (data (i32.const 0) "abcdefgh")
 )
 
-;; function u0:0(i64 vmctx, i64) fast {
+;; function u0:0(i64 vmctx, i64) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/basic-wat-test.wat

@@ -9,7 +9,7 @@
     i32.load
     i32.add))
 
-;; function u0:0(i64 vmctx, i64, i32, i32) -> i32 fast {
+;; function u0:0(i64 vmctx, i64, i32, i32) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/br_table.wat

@@ -31,7 +31,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64) -> i32 fast {
+;; function u0:0(i64 vmctx, i64) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -67,7 +67,7 @@
 ;; @002e                               return v2
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64) -> i32 fast {
+;; function u0:1(i64 vmctx, i64) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -103,7 +103,7 @@
 ;; @0044                               return v2
 ;; }
 ;;
-;; function u0:2(i64 vmctx, i64) -> i32 fast {
+;; function u0:2(i64 vmctx, i64) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -127,7 +127,7 @@
 ;; @0054                               return v2
 ;; }
 ;;
-;; function u0:3(i64 vmctx, i64) -> i32 fast {
+;; function u0:3(i64 vmctx, i64) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/byteswap.wat

@@ -71,7 +71,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64, i32) -> i32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -85,7 +85,7 @@
 ;; @0057                               return v19
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i64) -> i64 fast {
+;; function u0:1(i64 vmctx, i64, i64) -> i64 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/call-simd.wat

@@ -15,11 +15,11 @@
   (start $main)
 )
 
-;; function u0:0(i64 vmctx, i64) fast {
+;; function u0:0(i64 vmctx, i64) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
-;;     sig0 = (i64 vmctx, i64, i8x16, i8x16) -> i8x16 fast
+;;     sig0 = (i64 vmctx, i64, i8x16, i8x16) -> i8x16 tail
 ;;     fn0 = colocated u0:1 sig0
 ;;     const0 = 0x00000004000000030000000200000001
 ;;     stack_limit = gv2
@@ -34,7 +34,7 @@
 ;; @0048                               return
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i8x16, i8x16) -> i8x16 fast {
+;; function u0:1(i64 vmctx, i64, i8x16, i8x16) -> i8x16 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/call.wat

@@ -11,11 +11,11 @@
   (start $main)
 )
 
-;; function u0:0(i64 vmctx, i64) fast {
+;; function u0:0(i64 vmctx, i64) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
-;;     sig0 = (i64 vmctx, i64) -> i32 fast
+;;     sig0 = (i64 vmctx, i64) -> i32 tail
 ;;     fn0 = colocated u0:1 sig0
 ;;     stack_limit = gv2
 ;;
@@ -29,7 +29,7 @@
 ;; @0028                               return
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64) -> i32 fast {
+;; function u0:1(i64 vmctx, i64) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/dead-code.wat

@@ -21,7 +21,7 @@
     return
     i32.const 42)
 )
-;; function u0:0(i64 vmctx, i64, i32) fast {
+;; function u0:0(i64 vmctx, i64, i32) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -46,7 +46,7 @@
 ;; @002f                               return
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i32) fast {
+;; function u0:1(i64 vmctx, i64, i32) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -59,7 +59,7 @@
 ;; @0036                               jump block2
 ;; }
 ;;
-;; function u0:2(i64 vmctx, i64) fast {
+;; function u0:2(i64 vmctx, i64) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -72,7 +72,7 @@
 ;; @003f                               return
 ;; }
 ;;
-;; function u0:3(i64 vmctx, i64, i32) -> i32 fast {
+;; function u0:3(i64 vmctx, i64, i32) -> i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/duplicate-loads-dynamic-memory.wat

@@ -22,7 +22,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64, i32) -> i32, i32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -46,7 +46,7 @@
 ;; @005f                               return v12, v12
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i32) -> i32, i32 fast {
+;; function u0:1(i64 vmctx, i64, i32) -> i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/duplicate-loads-static-memory.wat

@@ -17,7 +17,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64, i32) -> i32, i32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -36,7 +36,7 @@
 ;; @005f                               return v8, v8
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i32) -> i32, i32 fast {
+;; function u0:1(i64 vmctx, i64, i32) -> i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/dynamic-memory-no-spectre-access-same-index-different-offsets.wat

@@ -35,7 +35,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64, i32) -> i32, i32, i32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> i32, i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -86,7 +86,7 @@
 ;; @0056                               return v11, v19, v29
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i32, i32, i32, i32) fast {
+;; function u0:1(i64 vmctx, i64, i32, i32, i32, i32) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/dynamic-memory-yes-spectre-access-same-index-different-offsets.wat

@@ -31,7 +31,7 @@
   )
 )
 
-;; function u0:0(i64 vmctx, i64, i32) -> i32, i32, i32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> i32, i32, i32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
@@ -66,7 +66,7 @@
 ;; @0056                               return v13, v23, v35
 ;; }
 ;;
-;; function u0:1(i64 vmctx, i64, i32, i32, i32, i32) fast {
+;; function u0:1(i64 vmctx, i64, i32, i32, i32, i32) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/f32-load.wat

@@ -6,7 +6,7 @@
     local.get 0
     f32.load))
 
-;; function u0:0(i64 vmctx, i64, i32) -> f32 fast {
+;; function u0:0(i64 vmctx, i64, i32) -> f32 tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1

tests/disas/f32-store.wat

@@ -9,7 +9,7 @@
     local.get 1
     f32.store))
 
-;; function u0:0(i64 vmctx, i64, i32, f32) fast {
+;; function u0:0(i64 vmctx, i64, i32, f32) tail {
 ;;     gv0 = vmctx
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1