Compile wasm builtins with the tail convention, not host (#9640)

* Compile wasm builtins with the `tail` convention, not host

This commit updates how Cranelift compiles the shims that go from
WebAssembly to various libcall builtins (e.g. `memory.grow`). Previously
the small trampolines in Cranelift were compiled with the native host
calling convention and this commit instead changes them to the wasm
calling convention (e.g. "tail"). This helps keep caller/callee matched
with calling conventions in wasm code itself and shifts the boundary of
the wasm->host transition to the indirect call of the host function
pointer itself. The general hope is that this makes Pulley a bit easier
since it'll want to minimize handling of wasm->host transitions.

* Fix wasm calling convention on Winch

* Handle different calling conventions in Winch too

Author: alexcrichton

crates/cranelift/src/compiler.rs

@@ -516,10 +516,12 @@ impl wasmtime_environ::Compiler for Compiler {
         let isa = &*self.isa;
         let ptr_size = isa.pointer_bytes();
         let pointer_type = isa.pointer_type();
-        let sig = BuiltinFunctionSignatures::new(isa).signature(index);
+        let sigs = BuiltinFunctionSignatures::new(isa, &self.tunables);
+        let wasm_sig = sigs.wasm_signature(index);
+        let host_sig = sigs.host_signature(index);
 
         let mut compiler = self.function_compiler();
-        let func = ir::Function::with_name_signature(Default::default(), sig.clone());
+        let func = ir::Function::with_name_signature(Default::default(), wasm_sig.clone());
         let (mut builder, block0) = compiler.builder(func);
         let vmctx = builder.block_params(block0)[0];
 
@@ -554,8 +556,10 @@ impl wasmtime_environ::Compiler for Compiler {
         // Forward all our own arguments to the libcall itself, and then return
         // all the same results as the libcall.
         let block_params = builder.block_params(block0).to_vec();
-        let sig = builder.func.import_signature(sig);
-        let call = builder.ins().call_indirect(sig, func_addr, &block_params);
+        let host_sig = builder.func.import_signature(host_sig);
+        let call = builder
+            .ins()
+            .call_indirect(host_sig, func_addr, &block_params);
         let results = builder.func.dfg.inst_results(call).to_vec();
         builder.ins().return_(&results);
         builder.finalize();

crates/cranelift/src/func_environ.rs

@@ -37,9 +37,9 @@ pub(crate) struct BuiltinFunctions {
 }
 
 impl BuiltinFunctions {
-    fn new(isa: &dyn TargetIsa) -> Self {
+    fn new(isa: &dyn TargetIsa, tunables: &Tunables) -> Self {
         Self {
-            types: BuiltinFunctionSignatures::new(isa),
+            types: BuiltinFunctionSignatures::new(isa, tunables),
             builtins: [None; BuiltinFunctionIndex::builtin_functions_total_number() as usize],
         }
     }
@@ -49,7 +49,7 @@ impl BuiltinFunctions {
         if let Some(f) = cache {
             return *f;
         }
-        let signature = func.import_signature(self.types.signature(index));
+        let signature = func.import_signature(self.types.wasm_signature(index));
         let name =
             ir::ExternalName::User(func.declare_imported_user_function(ir::UserExternalName {
                 namespace: crate::NS_WASMTIME_BUILTIN,
@@ -169,7 +169,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
         wmemcheck: bool,
         wasm_func_ty: &'module_environment WasmFuncType,
     ) -> Self {
-        let builtin_functions = BuiltinFunctions::new(isa);
+        let builtin_functions = BuiltinFunctions::new(isa, tunables);
 
         // Avoid unused warning in default build.
         #[cfg(not(feature = "wmemcheck"))]

crates/cranelift/src/lib.rs

@@ -154,12 +154,8 @@ fn array_call_signature(isa: &dyn TargetIsa) -> ir::Signature {
     sig
 }
 
-/// Get the internal Wasm calling convention signature for the given type.
-fn wasm_call_signature(
-    isa: &dyn TargetIsa,
-    wasm_func_ty: &WasmFuncType,
-    tunables: &Tunables,
-) -> ir::Signature {
+/// Get the internal Wasm calling convention for the target/tunables combo
+fn wasm_call_conv(isa: &dyn TargetIsa, tunables: &Tunables) -> CallConv {
     // The default calling convention is `CallConv::Tail` to enable the use of
     // tail calls in modules when needed. Note that this is used even if the
     // tail call proposal is disabled in wasm. This is not interacted with on
@@ -169,7 +165,7 @@ fn wasm_call_signature(
     // which call Winch-generated functions. The winch calling convention is
     // only implemented for x64 and aarch64, so assert that here and panic on
     // other architectures.
-    let call_conv = if tunables.winch_callable {
+    if tunables.winch_callable {
         assert!(
             matches!(
                 isa.triple().architecture,
@@ -180,7 +176,16 @@ fn wasm_call_signature(
         CallConv::Winch
     } else {
         CallConv::Tail
-    };
+    }
+}
+
+/// Get the internal Wasm calling convention signature for the given type.
+fn wasm_call_signature(
+    isa: &dyn TargetIsa,
+    wasm_func_ty: &WasmFuncType,
+    tunables: &Tunables,
+) -> ir::Signature {
+    let call_conv = wasm_call_conv(isa, tunables);
     let mut sig = blank_sig(isa, call_conv);
     let cvt = |ty: &WasmValType| ir::AbiParam::new(value_type(isa, *ty));
     sig.params.extend(wasm_func_ty.params().iter().map(&cvt));
@@ -327,14 +332,16 @@ struct BuiltinFunctionSignatures {
     #[cfg(feature = "gc")]
     reference_type: ir::Type,
 
-    call_conv: CallConv,
+    host_call_conv: CallConv,
+    wasm_call_conv: CallConv,
 }
 
 impl BuiltinFunctionSignatures {
-    fn new(isa: &dyn TargetIsa) -> Self {
+    fn new(isa: &dyn TargetIsa, tunables: &Tunables) -> Self {
         Self {
             pointer_type: isa.pointer_type(),
-            call_conv: CallConv::triple_default(isa.triple()),
+            host_call_conv: CallConv::triple_default(isa.triple()),
+            wasm_call_conv: wasm_call_conv(isa, tunables),
 
             #[cfg(feature = "gc")]
             reference_type: ir::types::I32,
@@ -379,7 +386,7 @@ impl BuiltinFunctionSignatures {
         AbiParam::new(ir::types::I8)
     }
 
-    fn signature(&self, builtin: BuiltinFunctionIndex) -> Signature {
+    fn wasm_signature(&self, builtin: BuiltinFunctionIndex) -> Signature {
         let mut _cur = 0;
         macro_rules! iter {
             (
@@ -394,7 +401,7 @@ impl BuiltinFunctionSignatures {
                         return Signature {
                             params: vec![ $( self.$param() ),* ],
                             returns: vec![ $( self.$result() )? ],
-                            call_conv: self.call_conv,
+                            call_conv: self.wasm_call_conv,
                         };
                     }
                     _cur += 1;
@@ -406,6 +413,12 @@ impl BuiltinFunctionSignatures {
 
         unreachable!();
     }
+
+    fn host_signature(&self, builtin: BuiltinFunctionIndex) -> Signature {
+        let mut sig = self.wasm_signature(builtin);
+        sig.call_conv = self.host_call_conv;
+        sig
+    }
 }
 
 /// If this bit is set on a GC reference, then the GC reference is actually an

crates/winch/src/compiler.rs

@@ -11,7 +11,7 @@ use wasmtime_environ::{
     FunctionLoc, ModuleTranslation, ModuleTypesBuilder, PrimaryMap, RelocationTarget,
     StaticModuleIndex, TrapEncodingBuilder, Tunables, VMOffsets, WasmFunctionInfo,
 };
-use winch_codegen::{BuiltinFunctions, TargetIsa};
+use winch_codegen::{BuiltinFunctions, CallingConvention, TargetIsa};
 
 /// Function compilation context.
 /// This struct holds information that can be shared globally across
@@ -51,7 +51,11 @@ impl Compiler {
             let vmoffsets = VMOffsets::new(pointer_size, &translation.module);
             CompilationContext {
                 allocations: Default::default(),
-                builtins: BuiltinFunctions::new(&vmoffsets, self.isa.wasmtime_call_conv()),
+                builtins: BuiltinFunctions::new(
+                    &vmoffsets,
+                    self.isa.wasmtime_call_conv(),
+                    CallingConvention::Default,
+                ),
             }
         })
     }

tests/disas/epoch-interruption.wat

@@ -9,7 +9,7 @@
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
 ;;     gv3 = vmctx
-;;     sig0 = (i64 vmctx) -> i64 system_v
+;;     sig0 = (i64 vmctx) -> i64 tail
 ;;     fn0 = colocated u1:16 sig0
 ;;     stack_limit = gv2
 ;;

tests/disas/gc/drc/array-new-fixed.wat

@@ -14,7 +14,7 @@
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
 ;;     gv3 = vmctx
-;;     sig0 = (i64 vmctx, i32 uext, i32 uext, i32 uext, i32 uext) -> i32 system_v
+;;     sig0 = (i64 vmctx, i32 uext, i32 uext, i32 uext, i32 uext) -> i32 tail
 ;;     fn0 = colocated u1:27 sig0
 ;;     stack_limit = gv2
 ;;

tests/disas/gc/drc/array-new.wat

@@ -14,7 +14,7 @@
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
 ;;     gv3 = vmctx
-;;     sig0 = (i64 vmctx, i32 uext, i32 uext, i32 uext, i32 uext) -> i32 system_v
+;;     sig0 = (i64 vmctx, i32 uext, i32 uext, i32 uext, i32 uext) -> i32 tail
 ;;     fn0 = colocated u1:27 sig0
 ;;     stack_limit = gv2
 ;;

tests/disas/gc/drc/br-on-cast-fail.wat

@@ -22,7 +22,7 @@
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
 ;;     gv3 = vmctx
-;;     sig0 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext system_v
+;;     sig0 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext tail
 ;;     sig1 = (i64 vmctx, i64) tail
 ;;     sig2 = (i64 vmctx, i64) tail
 ;;     fn0 = colocated u1:35 sig0

tests/disas/gc/drc/br-on-cast.wat

@@ -22,7 +22,7 @@
 ;;     gv1 = load.i64 notrap aligned readonly gv0+8
 ;;     gv2 = load.i64 notrap aligned gv1
 ;;     gv3 = vmctx
-;;     sig0 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext system_v
+;;     sig0 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext tail
 ;;     sig1 = (i64 vmctx, i64) tail
 ;;     sig2 = (i64 vmctx, i64) tail
 ;;     fn0 = colocated u1:35 sig0

tests/disas/gc/drc/call-indirect-and-subtyping.wat

@@ -23,8 +23,8 @@
 ;;     gv3 = vmctx
 ;;     gv4 = load.i64 notrap aligned readonly gv3+136
 ;;     sig0 = (i64 vmctx, i64) tail
-;;     sig1 = (i64 vmctx, i32 uext, i64) -> i64 system_v
-;;     sig2 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext system_v
+;;     sig1 = (i64 vmctx, i32 uext, i64) -> i64 tail
+;;     sig2 = (i64 vmctx, i32 uext, i32 uext) -> i32 uext tail
 ;;     fn0 = colocated u1:9 sig1
 ;;     fn1 = colocated u1:35 sig2
 ;;     stack_limit = gv2