From d72cc62871e3aa6ea8534cfba9b86ba7883b7ebb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 27 Nov 2024 12:21:20 -0800 Subject: [PATCH] Don't statically enumerate host signatures in Pulley I'm doing some other refactoring which makes it a pain to maintain a list in two locations of what all the host signatures are. Instead remove the pulley `for_each_host_signature!` macro entirely. My thinking is to instead implement a different system for host calls in pulley: * The same relocation-style mechanism is used with some number-embedded-in-the-bytecode. * The interpreter exits with "imma call the host" when it sees this special opcode. * The interpreter embedder, aka Wasmtime, is responsible for then invoking the actual function pointer. * Wasmtime already has static knowledge of all its function signatures, e.g. via various macros. This will prevent the need to list all function signatures twice and risk them getting out of sync. Most of the Pulley-level integration work here is left to a future commit. --- crates/cranelift/src/compiler.rs | 122 +++++++++++++------------------ pulley/src/interp.rs | 73 +----------------- pulley/src/lib.rs | 58 --------------- 3 files changed, 53 insertions(+), 200 deletions(-) diff --git a/crates/cranelift/src/compiler.rs b/crates/cranelift/src/compiler.rs index f33fb4f34bba..c4f9bfe05b77 100644 --- a/crates/cranelift/src/compiler.rs +++ b/crates/cranelift/src/compiler.rs @@ -142,78 +142,58 @@ impl Compiler { // Wasmtime itself. assert_eq!(signature.call_conv, self.isa.default_call_conv()); - #[cfg(feature = "pulley")] - { - use cranelift_codegen::ir::types::{I32, I64, I8}; - - // If pulley is enabled, even if we're not targeting it, determine - // what pulley signature that the input `signature` maps to. This is - // done to ensure that even on native platforms we've always got a - // signature listed in pulley for all platform intrinsics. In theory - // the set of signatures here doesn't change over time all that - // much. If a new signature is added then the `pulley/src/lib.rs` - // file and the `for_each_host_signature!` macro need to be updated. - // In theory that's all that needs to happen as well... - macro_rules! pulley_signum { - ($(fn($($args:ident),*) $(-> $ret:ident)?;)*) => {'outer: { - - let mut ret = 0; - - $( - let mut params = signature.params.iter().map(|p| p.value_type); - let mut results = signature.returns.iter().map(|p| p.value_type); - if true - $(&& params.next() == Some($args))* - && params.next().is_none() - $(&& results.next() == Some($ret))? - && results.next().is_none() - { - break 'outer ret; - } - ret += 1; - )* - - let _ = ret; - unimplemented!("no pulley host signature found for {signature:?}"); - }}; - } - - let pulley_signum = pulley_interpreter::for_each_host_signature!(pulley_signum); - - let is_pulley = match self.isa.triple().architecture { - target_lexicon::Architecture::Pulley32 => true, - target_lexicon::Architecture::Pulley64 => true, - _ => false, - }; - - // If this target is actually pulley then a custom `call` - // instruction is emitted. This will generate a new function with - // the Cranelift-name of a "backend intrinsic" which is how the - // Pulley backend models this special opcode that doesn't otherwise - // map into the Cranelift set of opcodes. - if is_pulley { - let mut new_signature = signature.clone(); - new_signature - .params - .insert(0, ir::AbiParam::new(self.isa.pointer_type())); - let new_sig = builder.func.import_signature(new_signature); - let name = ir::ExternalName::User(builder.func.declare_imported_user_function( - ir::UserExternalName { - namespace: crate::NS_PULLEY_HOSTCALL, - index: pulley_signum, + // If this target is actually pulley then a custom `call` + // instruction is emitted. This will generate a new function with + // the Cranelift-name of a "backend intrinsic" which is how the + // Pulley backend models this special opcode that doesn't otherwise + // map into the Cranelift set of opcodes. + let is_pulley = match self.isa.triple().architecture { + target_lexicon::Architecture::Pulley32 => true, + target_lexicon::Architecture::Pulley64 => true, + _ => false, + }; + if is_pulley { + let mut new_signature = signature.clone(); + new_signature + .params + .insert(0, ir::AbiParam::new(self.isa.pointer_type())); + let new_sig = builder.func.import_signature(new_signature); + let name = ir::ExternalName::User(builder.func.declare_imported_user_function( + ir::UserExternalName { + namespace: crate::NS_PULLEY_HOSTCALL, + // FIXME: this'll require some more refactoring to get this + // working entirely. The goal is to enumerate all possible + // reasons to call the host in some sort of enum, probably + // something like: + // + // enum wasmtime_environ::HostCall { + // ArrayCall, + // Builtin(BuiltinFunctionIndex), + // ComponentCall, + // ComponentBuiltin(ComponentBuiltinFunctionIndex), + // } + // + // that doesn't exist yet though but would be pretty + // reasonable to encode within a `u32` here. Doing that work + // is left as a future refactoring for Pulley. + index: { + let pulley_hostcall_index = || { + unimplemented!(); + }; + pulley_hostcall_index() }, - )); - let func = builder.func.import_function(ir::ExtFuncData { - name, - signature: new_sig, - // This is the signal that a special `call_indirect_host` - // opcode is used to jump from pulley to the host. - colocated: false, - }); - let mut raw_args = vec![addr]; - raw_args.extend_from_slice(args); - return builder.ins().call(func, &raw_args); - } + }, + )); + let func = builder.func.import_function(ir::ExtFuncData { + name, + signature: new_sig, + // This is the signal that a special `call_indirect_host` + // opcode is used to jump from pulley to the host. + colocated: false, + }); + let mut raw_args = vec![addr]; + raw_args.extend_from_slice(args); + return builder.ins().call(func, &raw_args); } builder.ins().call_indirect(sig, addr, args) diff --git a/pulley/src/interp.rs b/pulley/src/interp.rs index 4d39244571c8..e0f06f5f32aa 100644 --- a/pulley/src/interp.rs +++ b/pulley/src/interp.rs @@ -1245,77 +1245,8 @@ impl ExtendedOpVisitor for Interpreter<'_> { ControlFlow::Continue(()) } - /// This instructions is sort of like a `call` instruction except that it - /// delegates to the host itself. That means that ABI details are baked in - /// here such as where various arguments are. - /// - /// This will load the arguments from the `xN` registers, the first being - /// the function pointer on the host to call. Since we don't have a - /// libffi-like solution here the way this works is that the - /// `for_each_host_signature!` macro statically enumerates all possible - /// signatures. The `sig` payload here selects one of the mwhich we dispatch - /// to here. Note that we mostly just try to get the width of each argument - /// correct, whether or not it's a pointer is not actually tracked here. - /// That means that, like the rest of Pulley, this isn't compatible with - /// strict provenance pointer rules. fn call_indirect_host(&mut self, sig: u8) -> ControlFlow { - let raw = self.state[XReg::x0].get_ptr::(); - let mut n = 0; - let mut arg = 1; - - type I8 = i8; - type I32 = i32; - type I64 = i64; - - macro_rules! call_host { - ($(fn($($args:ident),*) $(-> $ret:ident)?;)*) => {$( - // We're relying on LLVM to boil away most of this boilerplate - // as this is a bunch of `if` statements that should be a - // `match`. - if sig == n { - union Convert { - raw: *mut u8, - f: unsafe extern "C" fn($($args),*) $(-> $ret)?, - } - let ptr = Convert { raw }.f; - - // Arguments are loaded from subsequent registers after - // `x0` and are tracked by `arg`. - let ret = ptr( - $({ - let reg = XReg::new_unchecked(arg); - arg += 1; - let reg = &self.state[reg]; - call_host!(@get $args reg) - },)* - ); - let _ = arg; // ignore the last increment of `arg` - - // If this function produce a result the ABI is that we - // place it into `x0`. - let dst = &mut self.state[XReg::x0]; - $(call_host!(@set $ret dst ret);)? - let _ = (ret, dst); // ignore if there was no return value - } - n += 1; - )*}; - - (@get I8 $reg:ident) => ($reg.get_i32() as i8); - (@get I32 $reg:ident) => ($reg.get_i32()); - (@get I64 $reg:ident) => ($reg.get_i64()); - - (@set I8 $dst:ident $val:ident) => ($dst.set_i32($val.into());); - (@set I32 $dst:ident $val:ident) => ($dst.set_i32($val);); - (@set I64 $dst:ident $val:ident) => ($dst.set_i64($val);); - - } - - unsafe { - for_each_host_signature!(call_host); - } - - let _ = n; // ignore the last increment of `n` - - ControlFlow::Continue(()) + let _ = sig; // TODO: should stash this somewhere + ControlFlow::Break(Done::ReturnToHost) } } diff --git a/pulley/src/lib.rs b/pulley/src/lib.rs index a306cc37316c..11b37528318b 100644 --- a/pulley/src/lib.rs +++ b/pulley/src/lib.rs @@ -212,64 +212,6 @@ macro_rules! for_each_extended_op { }; } -/// All known signatures that Wasmtime needs to invoke for host functions. -/// -/// This is used in conjunction with the `call_indirect_host` opcode to jump -/// from interpreter bytecode back into the host to peform tasks such as -/// `memory.grow` or call imported host functions. -/// -/// Each function signature here correspond to a "builtin" either for core wasm -/// or for the component model. This also includes the "array call abi" for -/// calling host functions. -/// -/// TODO: this probably needs a "pointer type" to avoid doubling the size of -/// this on 32-bit platforms. That's left for a future refactoring when it's -/// easier to start compiling everything for 32-bit platforms. That'll require -/// more of the pulley backend fleshed out and the integration with Wasmtime -/// more fleshed out as well. -#[macro_export] -macro_rules! for_each_host_signature { - ($m:ident) => { - $m! { - fn(I64, I32, I32, I32) -> I32; - fn(I64, I32, I32) -> I32; - fn(I64, I32, I32) -> I64; - fn(I64, I64, I32, I64, I64, I64, I8, I64, I64) -> I8; - fn(I64, I64, I64, I64, I64) -> I64; - fn(I64, I64, I64, I64) -> I64; - fn(I64, I64, I64) -> I64; - fn(I64, I64, I64); - fn(I64); - fn(I64, I32, I32, I32, I32, I32); - fn(I64, I32) -> I32; - fn(I64, I32, I32, I32, I32, I32, I32); - fn(I64, I32, I32, I32, I32) -> I32; - fn(I64, I32, I32, I64, I32, I32); - fn(I64, I32, I32, I64, I64, I64); - fn(I64, I32, I32) -> I32; - fn(I64, I32, I32) -> I64; - fn(I64, I32, I64, I32, I64); - fn(I64, I32, I64, I32) -> I64; - fn(I64, I32, I64, I32, I64, I64); - fn(I64, I32, I64, I32, I64) -> I32; - fn(I64, I32, I64, I32, I64); - fn(I64, I32, I64, I32) -> I32; - fn(I64, I32, I64, I64, I64) -> I32; - fn(I64, I32, I64, I64, I64); - fn(I64, I32, I64, I64) -> I64; - fn(I64, I32, I64) -> I64; - fn(I64, I32) -> I64; - fn(I64, I32); - fn(I64, I64, I32) -> I64; - fn(I64, I64, I64, I64) -> I8; - fn(I64, I64) -> I32; - fn(I64, I8); - fn(I64) -> I64; - fn(I64); - } - }; -} - #[cfg(feature = "decode")] pub mod decode; #[cfg(feature = "disas")]