Add ability to call CLIF functions with arbitrary arguments in filetests

This resolves the work started in https://github.com/bytecodealliance/cranelift/pull/1231 and https://github.com/bytecodealliance/wasmtime/pull/1436. Cranelift filetests currently have the ability to run CLIF functions with a signature like `() -> b*` and check that the result is true under the `test run` directive. This PR adds the ability to call functions with arbitrary arguments and non-boolean returns and either print the result or check against a list of expected results:
 - `run` commands look like `; run: %add(2, 2) == 4` or `; run: %add(2, 2) != 5` and verify that the executed CLIF function returns the expected value
 - `print` commands look like `; print: %add(2, 2)` and print the result of the function to stdout

To make this work, this PR compiles a single Cranelift `Function` into a `CompiledFunction` using a `SingleFunctionCompiler`. Because we will not know the signature of the function until runtime, we use a `Trampoline` to place the values in the appropriate location for the calling convention; this should look a lot like what @alexcrichton is doing with `VMTrampoline` in wasmtime (see 3b7cb6ee64/crates/api/src/func.rs (L510-L526), 3b7cb6ee64/crates/jit/src/compiler.rs (L260)). To avoid re-compiling `Trampoline`s for the same function signatures, `Trampoline`s are cached in the `SingleFunctionCompiler`.

Cargo.lock

@@ -409,6 +409,7 @@ version = "0.63.0"
 dependencies = [
  "byteorder",
  "cranelift-codegen",
+ "cranelift-frontend",
  "cranelift-native",
  "cranelift-preopt",
  "cranelift-reader",
@@ -420,6 +421,7 @@ dependencies = [
  "num_cpus",
  "region",
  "target-lexicon",
+ "thiserror",
 ]
 
 [[package]]

cranelift/filetests/Cargo.toml

@@ -11,9 +11,11 @@ edition = "2018"
 
 [dependencies]
 cranelift-codegen = { path = "../codegen", version = "0.63.0", features = ["testing_hooks"] }
+cranelift-frontend = { path = "../frontend", version = "0.63.0" }
 cranelift-native = { path = "../native", version = "0.63.0" }
 cranelift-reader = { path = "../reader", version = "0.63.0" }
 cranelift-preopt = { path = "../preopt", version = "0.63.0" }
+byteorder = { version = "1.3.2", default-features = false }
 file-per-thread-logger = "0.1.2"
 filecheck = "0.5.0"
 gimli = { version = "0.20.0", default-features = false, features = ["read"] }
@@ -21,5 +23,5 @@ log = "0.4.6"
 memmap = "0.7.0"
 num_cpus = "1.8.0"
 region = "2.1.2"
-byteorder = { version = "1.3.2", default-features = false }
 target-lexicon = "0.10"
+thiserror = "1.0.15"

cranelift/filetests/src/function_runner.rs

@@ -1,96 +1,392 @@
-use core::mem;
+//! Provides functionality for compiling and running CLIF IR for `run` tests.
+use core::{mem, ptr};
 use cranelift_codegen::binemit::{NullRelocSink, NullStackmapSink, NullTrapSink};
-use cranelift_codegen::ir::Function;
+use cranelift_codegen::ir::{condcodes::IntCC, Function, InstBuilder, Signature, Type};
 use cranelift_codegen::isa::TargetIsa;
-use cranelift_codegen::{settings, Context};
+use cranelift_codegen::{ir, settings, CodegenError, Context};
+use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext};
 use cranelift_native::builder as host_isa_builder;
-use memmap::MmapMut;
+use cranelift_reader::DataValue;
+use log::trace;
+use memmap::{Mmap, MmapMut};
+use std::cmp::max;
+use std::collections::HashMap;
+use thiserror::Error;
 
-/// Run a function on a host
-pub struct FunctionRunner {
-    function: Function,
+/// Compile a single function.
+///
+/// Several Cranelift functions need the ability to run Cranelift IR (e.g. `test_run`); this
+/// [SingleFunctionCompiler] provides a way for compiling Cranelift [Function]s to
+/// [CompiledFunction]s and subsequently calling them through the use of a [Trampoline]. As its
+/// name indicates, this compiler is limited: any functionality that requires knowledge of things
+/// outside the [Function] will likely not work (e.g. global values, calls). For an example of this
+/// "outside-of-function" functionality, see `cranelift_simplejit::backend::SimpleJITBackend`.
+///
+/// ```
+/// use cranelift_filetests::SingleFunctionCompiler;
+/// use cranelift_reader::parse_functions;
+///
+/// let code = "test run \n function %add(i32, i32) -> i32 {  block0(v0:i32, v1:i32):  v2 = iadd v0, v1  return v2 }".into();
+/// let func = parse_functions(code).unwrap().into_iter().nth(0).unwrap();
+/// let mut compiler = SingleFunctionCompiler::with_default_host_isa();
+/// let compiled_func = compiler.compile(func).unwrap();
+/// println!("Address of compiled function: {:p}", compiled_func.as_ptr());
+/// ```
+pub struct SingleFunctionCompiler {
     isa: Box<dyn TargetIsa>,
+    trampolines: HashMap<Signature, Trampoline>,
 }
 
-impl FunctionRunner {
-    /// Build a function runner from a function and the ISA to run on (must be the host machine's ISA)
-    pub fn new(function: Function, isa: Box<dyn TargetIsa>) -> Self {
-        Self { function, isa }
+impl SingleFunctionCompiler {
+    /// Build a [SingleFunctionCompiler] from a [TargetIsa]. For functions to be runnable on the
+    /// host machine, this [TargetISA] must match the host machine's ISA (see [with_host_isa]).
+    pub fn new(isa: Box<dyn TargetIsa>) -> Self {
+        let trampolines = HashMap::new();
+        Self { isa, trampolines }
     }
 
-    /// Build a function runner using the host machine's ISA and the passed flags
-    pub fn with_host_isa(function: Function, flags: settings::Flags) -> Self {
+    /// Build a [SingleFunctionCompiler] using the host machine's ISA and the passed flags.
+    pub fn with_host_isa(flags: settings::Flags) -> Self {
         let builder = host_isa_builder().expect("Unable to build a TargetIsa for the current host");
         let isa = builder.finish(flags);
-        Self::new(function, isa)
+        Self::new(isa)
     }
 
-    /// Build a function runner using the host machine's ISA and the default flags for this ISA
-    pub fn with_default_host_isa(function: Function) -> Self {
+    /// Build a [SingleFunctionCompiler] using the host machine's ISA and the default flags for this
+    /// ISA.
+    pub fn with_default_host_isa() -> Self {
         let flags = settings::Flags::new(settings::builder());
-        Self::with_host_isa(function, flags)
+        Self::with_host_isa(flags)
     }
 
-    /// Compile and execute a single function, expecting a boolean to be returned; a 'true' value is
-    /// interpreted as a successful test execution and mapped to Ok whereas a 'false' value is
-    /// interpreted as a failed test and mapped to Err.
-    pub fn run(&self) -> Result<(), String> {
-        let func = self.function.clone();
-        if !(func.signature.params.is_empty()
-            && func.signature.returns.len() == 1
-            && func.signature.returns.first().unwrap().value_type.is_bool())
-        {
-            return Err(String::from(
-                "Functions must have a signature like: () -> boolean",
-            ));
+    /// Compile the passed [Function] to a [CompiledFunction]. This function will:
+    ///  - check that the default ISA calling convention is used (to ensure it can be called)
+    ///  - compile the [Function]
+    ///  - compile a [Trampoline] for the [Function]'s signature (or used a cached [Trampoline];
+    ///    this makes it possible to call functions when the signature is not known until runtime.
+    pub fn compile(&mut self, function: Function) -> Result<CompiledFunction, CompilationError> {
+        let signature = function.signature.clone();
+        if signature.call_conv != self.isa.default_call_conv() {
+            return Err(CompilationError::InvalidTargetIsa);
         }
 
-        if func.signature.call_conv != self.isa.default_call_conv() {
-            return Err(String::from(
-                "Functions only run on the host's default calling convention; remove the specified calling convention in the function signature to use the host's default.",
-            ));
+        // Compile the function itself.
+        let code_page = compile(function, self.isa.as_ref())?;
+
+        // Compile the trampoline to call it, if necessary (it may be cached).
+        let isa = self.isa.as_ref();
+        let trampoline = self
+            .trampolines
+            .entry(signature.clone())
+            .or_insert_with(|| {
+                let ir = make_trampoline(&signature, isa);
+                let code = compile(ir, isa).expect("failed to compile trampoline");
+                Trampoline::new(code)
+            });
+
+        Ok(CompiledFunction::new(code_page, signature, trampoline))
+    }
+}
+
+#[derive(Error, Debug)]
+pub enum CompilationError {
+    #[error("Cross-compilation not currently supported; use the host's default calling convention \
+    or remove the specified calling convention in the function signature to use the host's default.")]
+    InvalidTargetIsa,
+    #[error("Cranelift codegen error")]
+    CodegenError(#[from] CodegenError),
+    #[error("Memory mapping error")]
+    IoError(#[from] std::io::Error),
+}
+
+/// Contains the compiled code to move memory-allocated [DataValue]s to the correct location (e.g.
+/// register, stack) dictated by the calling convention before calling a [CompiledFunction]. Without
+/// this, it would be quite difficult to correctly place [DataValue]s since both the calling
+/// convention and function signature are not known until runtime. See [make_trampoline] for the
+/// Cranelift IR used to build this.
+pub struct Trampoline {
+    page: Mmap,
+}
+
+impl Trampoline {
+    /// Build a new [Trampoline].
+    pub fn new(page: Mmap) -> Self {
+        Self { page }
+    }
+
+    /// Return a pointer to the compiled code.
+    fn as_ptr(&self) -> *const u8 {
+        self.page.as_ptr()
+    }
+}
+
+/// Container for the compiled code of a [Function]. This wrapper allows users to call the compiled
+/// function through the use of a [Trampoline].
+///
+/// ```
+/// use cranelift_filetests::SingleFunctionCompiler;
+/// use cranelift_reader::{parse_functions, DataValue};
+///
+/// let code = "test run \n function %add(i32, i32) -> i32 {  block0(v0:i32, v1:i32):  v2 = iadd v0, v1  return v2 }".into();
+/// let func = parse_functions(code).unwrap().into_iter().nth(0).unwrap();
+/// let mut compiler = SingleFunctionCompiler::with_default_host_isa();
+/// let compiled_func = compiler.compile(func).unwrap();
+///
+/// let returned = compiled_func.call(&vec![DataValue::I32(2), DataValue::I32(40)]);
+/// assert_eq!(vec![DataValue::I32(42)], returned);
+/// ```
+pub struct CompiledFunction<'a> {
+    page: Mmap,
+    signature: Signature,
+    trampoline: &'a Trampoline,
+}
+
+impl<'a> CompiledFunction<'a> {
+    /// Build a new [CompiledFunction].
+    pub fn new(page: Mmap, signature: Signature, trampoline: &'a Trampoline) -> Self {
+        Self {
+            page,
+            signature,
+            trampoline,
         }
+    }
 
-        // set up the context
-        let mut context = Context::new();
-        context.func = func;
+    /// Return a pointer to the compiled code.
+    pub fn as_ptr(&self) -> *const u8 {
+        self.page.as_ptr()
+    }
 
-        // compile and encode the result to machine code
-        let relocs = &mut NullRelocSink {};
-        let traps = &mut NullTrapSink {};
-        let stackmaps = &mut NullStackmapSink {};
-        let code_info = context
-            .compile(self.isa.as_ref())
-            .map_err(|e| e.to_string())?;
-        let mut code_page =
-            MmapMut::map_anon(code_info.total_size as usize).map_err(|e| e.to_string())?;
+    /// Call the [CompiledFunction], passing in [DataValue]s using a compiled [Trampoline].
+    pub fn call(&self, arguments: &[DataValue]) -> Vec<DataValue> {
+        let mut values = UnboxedValues::make_arguments(arguments, &self.signature);
+        let arguments_address = values.as_mut_ptr();
+        let function_address = self.as_ptr();
 
-        unsafe {
-            context.emit_to_memory(
-                self.isa.as_ref(),
-                code_page.as_mut_ptr(),
-                relocs,
-                traps,
-                stackmaps,
+        let callable_trampoline: fn(*const u8, *mut u128) -> () =
+            unsafe { mem::transmute(self.trampoline.as_ptr()) };
+        callable_trampoline(function_address, arguments_address);
+
+        values.collect_returns(&self.signature)
+    }
+}
+
+/// A container for laying out the [ValueData]s in memory in a way that the [Trampoline] can
+/// understand.
+struct UnboxedValues(Vec<u128>);
+
+impl UnboxedValues {
+    /// The size in bytes of each slot location in the allocated [DataValue]s. Though [DataValue]s
+    /// could be smaller than 16 bytes (e.g. `I16`), this simplifies the creation of the [DataValue]
+    /// array and could be used to align the slots to the largest used [DataValue] (i.e. 128-bit
+    /// vectors).
+    const SLOT_SIZE: usize = 16;
+
+    /// Build the arguments vector for passing the [DataValue]s into the [Trampoline]. The size of
+    /// `u128` used here must match [Trampoline::SLOT_SIZE].
+    pub fn make_arguments(arguments: &[DataValue], signature: &ir::Signature) -> Self {
+        assert_eq!(arguments.len(), signature.params.len());
+        let mut values_vec = vec![0; max(signature.params.len(), signature.returns.len())];
+
+        // Store the argument values into `values_vec`.
+        for ((arg, slot), param) in arguments.iter().zip(&mut values_vec).zip(&signature.params) {
+            assert_eq!(
+                arg.ty(),
+                param.value_type,
+                "argument type mismatch: {} != {}",
+                arg.ty(),
+                param.value_type
             );
-        };
+            unsafe {
+                Self::write_value_to(arg, slot);
+            }
+        }
 
-        let code_page = code_page.make_exec().map_err(|e| e.to_string())?;
-        let callable_fn: fn() -> bool = unsafe { mem::transmute(code_page.as_ptr()) };
+        Self(values_vec)
+    }
 
-        // execute
-        if callable_fn() {
-            Ok(())
-        } else {
-            Err(format!("Failed: {}", context.func.name.to_string()))
+    /// Return a pointer to the underlying memory for passing to the trampoline.
+    pub fn as_mut_ptr(&mut self) -> *mut u128 {
+        self.0.as_mut_ptr()
+    }
+
+    /// Collect the returned [DataValue]s into a [Vec]. The size of `u128` used here must match
+    /// [Trampoline::SLOT_SIZE].
+    pub fn collect_returns(&self, signature: &ir::Signature) -> Vec<DataValue> {
+        assert!(self.0.len() >= signature.returns.len());
+        let mut returns = Vec::with_capacity(signature.returns.len());
+
+        // Extract the returned values from this vector.
+        for (slot, param) in self.0.iter().zip(&signature.returns) {
+            let value = unsafe { Self::read_value_from(slot, param.value_type) };
+            returns.push(value);
+        }
+
+        returns
+    }
+
+    /// Write a [DataValue] to a memory location.
+    unsafe fn write_value_to(v: &DataValue, p: *mut u128) {
+        match v {
+            DataValue::B(b) => ptr::write(p as *mut bool, *b),
+            DataValue::I8(i) => ptr::write(p as *mut i8, *i),
+            DataValue::I16(i) => ptr::write(p as *mut i16, *i),
+            DataValue::I32(i) => ptr::write(p as *mut i32, *i),
+            DataValue::I64(i) => ptr::write(p as *mut i64, *i),
+            DataValue::F32(f) => ptr::write(p as *mut f32, *f),
+            DataValue::F64(f) => ptr::write(p as *mut f64, *f),
+            DataValue::V128(b) => ptr::write(p as *mut [u8; 16], *b),
         }
     }
+
+    /// Read a [DataValue] from a memory location using a given [Type].
+    unsafe fn read_value_from(p: *const u128, ty: Type) -> DataValue {
+        match ty {
+            ir::types::I8 => DataValue::I8(ptr::read(p as *const i8)),
+            ir::types::I16 => DataValue::I16(ptr::read(p as *const i16)),
+            ir::types::I32 => DataValue::I32(ptr::read(p as *const i32)),
+            ir::types::I64 => DataValue::I64(ptr::read(p as *const i64)),
+            ir::types::F32 => DataValue::F32(ptr::read(p as *const f32)),
+            ir::types::F64 => DataValue::F64(ptr::read(p as *const f64)),
+            _ if ty.is_bool() => DataValue::B(ptr::read(p as *const bool)),
+            _ if ty.is_vector() && ty.bytes() == 16 => {
+                DataValue::V128(ptr::read(p as *const [u8; 16]))
+            }
+            _ => unimplemented!(),
+        }
+    }
+}
+
+/// Compile a [Function] to its executable bytes in memory.
+///
+/// This currently returns a [Mmap], a type from an external crate, so we wrap this up before
+/// exposing it in public APIs.
+fn compile(function: Function, isa: &dyn TargetIsa) -> Result<Mmap, CompilationError> {
+    // Set up the context.
+    let mut context = Context::new();
+    context.func = function;
+
+    // Compile and encode the result to machine code.
+    let relocs = &mut NullRelocSink {};
+    let traps = &mut NullTrapSink {};
+    let stackmaps = &mut NullStackmapSink {};
+    let code_info = context.compile(isa)?;
+    let mut code_page = MmapMut::map_anon(code_info.total_size as usize)?;
+
+    unsafe {
+        context.emit_to_memory(isa, code_page.as_mut_ptr(), relocs, traps, stackmaps);
+    };
+
+    let code_page = code_page.make_exec()?;
+    trace!(
+        "Compiled function {} with signature {} at: {:p}",
+        context.func.name,
+        context.func.signature,
+        code_page.as_ptr()
+    );
+
+    Ok(code_page)
+}
+
+/// Build the Cranelift IR for moving the memory-allocated [DataValue]s to their correct location
+/// (e.g. register, stack) prior to calling a [CompiledFunction]. The [Function] returned by
+/// [make_trampoline] is compiled to a [Trampoline]. Note that this uses the [TargetIsa]'s default
+/// calling convention so we must also check that the [CompiledFunction] has the same calling
+/// convention (see [SingleFunctionCompiler::compile]).
+fn make_trampoline(signature: &ir::Signature, isa: &dyn TargetIsa) -> Function {
+    // Create the trampoline signature: (callee_address: pointer, values_vec: pointer) -> ()
+    let pointer_type = isa.pointer_type();
+    let mut wrapper_sig = ir::Signature::new(isa.frontend_config().default_call_conv);
+    wrapper_sig.params.push(ir::AbiParam::new(pointer_type)); // Add the `callee_address` parameter.
+    wrapper_sig.params.push(ir::AbiParam::new(pointer_type)); // Add the `values_vec` parameter.
+
+    let mut func = ir::Function::with_name_signature(ir::ExternalName::user(0, 0), wrapper_sig);
+
+    // The trampoline has a single block filled with loads, one call to callee_address, and some loads.
+    let mut builder_context = FunctionBuilderContext::new();
+    let mut builder = FunctionBuilder::new(&mut func, &mut builder_context);
+    let block0 = builder.create_block();
+    builder.append_block_params_for_function_params(block0);
+    builder.switch_to_block(block0);
+    builder.seal_block(block0);
+
+    // Extract the incoming SSA values.
+    let (callee_value, values_vec_ptr_val) = {
+        let params = builder.func.dfg.block_params(block0);
+        (params[0], params[1])
+    };
+
+    // Load the argument values out of `values_vec`.
+    let callee_args = signature
+        .params
+        .iter()
+        .enumerate()
+        .map(|(i, param)| {
+            // Calculate the type to load from memory, using integers for booleans (no encodings).
+            let ty = if param.value_type.is_bool() {
+                Type::int(max(param.value_type.bits(), 8)).expect(
+                    "to be able to convert any boolean type to its equal-width integer type",
+                )
+            } else {
+                param.value_type
+            };
+            // Load the value.
+            let loaded = builder.ins().load(
+                ty,
+                ir::MemFlags::trusted(),
+                values_vec_ptr_val,
+                (i * UnboxedValues::SLOT_SIZE) as i32,
+            );
+            // For booleans, we want to type-convert the loaded integer into a boolean and ensure
+            // that we are using the architecture's canonical boolean representation (presumably
+            // comparison will emit this).
+            if param.value_type.is_bool() {
+                builder.ins().icmp_imm(IntCC::NotEqual, loaded, 0)
+            } else {
+                loaded
+            }
+        })
+        .collect::<Vec<_>>();
+
+    // Call the passed function.
+    let new_sig = builder.import_signature(signature.clone());
+    let call = builder
+        .ins()
+        .call_indirect(new_sig, callee_value, &callee_args);
+
+    // Store the return values into `values_vec`.
+    let results = builder.func.dfg.inst_results(call).to_vec();
+    for ((i, value), param) in results.iter().enumerate().zip(&signature.returns) {
+        // Before storing return values, we convert booleans to their integer representation.
+        let value = if param.value_type.is_bool() {
+            let ty = Type::int(max(param.value_type.bits(), 8))
+                .expect("to be able to convert any boolean type to its equal-width integer type");
+            builder.ins().bint(ty, *value)
+        } else {
+            *value
+        };
+        // Store the value.
+        builder.ins().store(
+            ir::MemFlags::trusted(),
+            value,
+            values_vec_ptr_val,
+            (i * UnboxedValues::SLOT_SIZE) as i32,
+        );
+    }
+
+    builder.ins().return_(&[]);
+    builder.finalize();
+
+    func
 }
 
 #[cfg(test)]
 mod test {
     use super::*;
-    use cranelift_reader::{parse_test, ParseOptions};
+    use cranelift_reader::{parse_functions, parse_test, ParseOptions};
+
+    fn parse(code: &str) -> Function {
+        parse_functions(code).unwrap().into_iter().nth(0).unwrap()
+    }
 
     #[test]
     fn nop() {
@@ -111,7 +407,42 @@ mod test {
         let function = test_file.functions[0].0.clone();
 
         // execute function
-        let runner = FunctionRunner::with_default_host_isa(function);
-        runner.run().unwrap() // will panic if execution fails
+        let mut compiler = SingleFunctionCompiler::with_default_host_isa();
+        let compiled_function = compiler.compile(function).unwrap();
+        let returned = compiled_function.call(&[]);
+        assert_eq!(returned, vec![DataValue::B(true)])
+    }
+
+    #[test]
+    fn trampolines() {
+        let function = parse(
+            "
+            function %test(f32, i8, i64x2, b1) -> f32x4, b64 {
+            block0(v0: f32, v1: i8, v2: i64x2, v3: b1):
+                v4 = vconst.f32x4 [0x0.1 0x0.2 0x0.3 0x0.4]
+                v5 = bconst.b64 true
+                return v4, v5
+            }",
+        );
+
+        let compiler = SingleFunctionCompiler::with_default_host_isa();
+        let trampoline = make_trampoline(&function.signature, compiler.isa.as_ref());
+        assert!(format!("{}", trampoline).ends_with(
+            "sig0 = (f32, i8, i64x2, b1) -> f32x4, b64 fast
+
+block0(v0: i64, v1: i64):
+    v2 = load.f32 notrap aligned v1
+    v3 = load.i8 notrap aligned v1+16
+    v4 = load.i64x2 notrap aligned v1+32
+    v5 = load.i8 notrap aligned v1+48
+    v6 = icmp_imm ne v5, 0
+    v7, v8 = call_indirect sig0, v0(v2, v3, v4, v6)
+    store notrap aligned v7, v1
+    v9 = bint.i64 v8
+    store notrap aligned v9, v1+16
+    return
+}
+"
+        ));
     }
 }

cranelift/filetests/src/lib.rs

@@ -23,7 +23,7 @@
     )
 )]
 
-pub use crate::function_runner::FunctionRunner;
+pub use crate::function_runner::SingleFunctionCompiler;
 use crate::runner::TestRunner;
 use cranelift_codegen::timing;
 use cranelift_reader::TestCommand;

cranelift/filetests/src/test_run.rs

@@ -2,7 +2,7 @@
 //!
 //! The `run` test command compiles each function on the host machine and executes it
 
-use crate::function_runner::FunctionRunner;
+use crate::function_runner::SingleFunctionCompiler;
 use crate::subtest::{Context, SubTest, SubtestResult};
 use cranelift_codegen::ir;
 use cranelift_reader::parse_run_command;
@@ -36,11 +36,11 @@ impl SubTest for TestRun {
     }
 
     fn run(&self, func: Cow<ir::Function>, context: &Context) -> SubtestResult<()> {
+        let mut compiler = SingleFunctionCompiler::with_host_isa(context.flags.clone());
         for comment in context.details.comments.iter() {
-            if comment.text.contains("run") {
-                let trimmed_comment = comment.text.trim_start_matches(|c| c == ' ' || c == ';');
-                let command = parse_run_command(trimmed_comment, &func.signature)
-                    .map_err(|e| format!("{}", e))?;
+            if let Some(command) =
+                parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())?
+            {
                 trace!("Parsed run command: {}", command);
 
                 // If this test requests to run on a completely different
@@ -51,16 +51,15 @@ impl SubTest for TestRun {
                     return Ok(());
                 }
 
-                // TODO in following changes we will use the parsed command to alter FunctionRunner's behavior.
-                //
                 // Note that here we're also explicitly ignoring `context.isa`,
                 // regardless of what's requested. We want to use the native
                 // host ISA no matter what here, so the ISA listed in the file
                 // is only used as a filter to not run into situations like
                 // running x86_64 code on aarch64 platforms.
-                let runner =
-                    FunctionRunner::with_host_isa(func.clone().into_owned(), context.flags.clone());
-                runner.run()?
+                let compiled_fn = compiler
+                    .compile(func.clone().into_owned())
+                    .map_err(|e| e.to_string())?;
+                command.run(|args| compiled_fn.call(args))?;
             }
         }
         Ok(())

cranelift/reader/src/parser.rs

@@ -114,11 +114,25 @@ pub fn parse_test<'a>(text: &'a str, options: ParseOptions<'a>) -> ParseResult<T
     })
 }
 
-/// Parse the entire `text` as a run command.
-pub fn parse_run_command<'a>(text: &str, signature: &Signature) -> ParseResult<RunCommand> {
+/// Parse a CLIF comment `text` as a run command.
+///
+/// Return:
+///  - `Ok(None)` if the comment is not intended to be a `RunCommand` (i.e. does not start with `run`
+///    or `print`
+///  - `Ok(Some(command))` if the comment is intended as a `RunCommand` and can be parsed to one
+///  - `Err` otherwise.
+pub fn parse_run_command<'a>(text: &str, signature: &Signature) -> ParseResult<Option<RunCommand>> {
     let _tt = timing::parse_text();
-    let mut parser = Parser::new(text);
-    parser.parse_run_command(signature)
+    // We remove leading spaces and semi-colons for convenience here instead of at the call sites
+    // since this function will be attempting to parse a RunCommand from a CLIF comment.
+    let trimmed_text = text.trim_start_matches(|c| c == ' ' || c == ';');
+    let mut parser = Parser::new(trimmed_text);
+    match parser.token() {
+        Some(Token::Identifier("run")) | Some(Token::Identifier("print")) => {
+            parser.parse_run_command(signature).map(|c| Some(c))
+        }
+        Some(_) | None => Ok(None),
+    }
 }
 
 pub struct Parser<'a> {

cranelift/reader/src/run_command.rs

@@ -7,8 +7,8 @@
 //! - `; run: %fn(42, 4.2) == false`: this syntax specifies the parameters and return values.
 
 use cranelift_codegen::ir::immediates::{Ieee32, Ieee64};
-use cranelift_codegen::ir::ConstantData;
-use std::fmt::{Display, Formatter, Result};
+use cranelift_codegen::ir::{self, ConstantData, Type};
+use std::fmt::{self, Display, Formatter};
 
 /// A run command appearing in a test file.
 ///
@@ -22,8 +22,37 @@ pub enum RunCommand {
     Run(Invocation, Comparison, Vec<DataValue>),
 }
 
+impl RunCommand {
+    /// Run the [RunCommand]:
+    ///  - for [RunCommand::Print], print the returned values from invoking the function.
+    ///  - for [RunCommand::Run], compare the returned values from the invoked function and
+    ///    return an `Err` with a descriptive string if the comparison fails.
+    pub fn run<F>(&self, invoke_fn: F) -> Result<(), String>
+    where
+        F: FnOnce(&[DataValue]) -> Vec<DataValue>,
+    {
+        match self {
+            RunCommand::Print(invoke) => {
+                let actual = invoke_fn(&invoke.args);
+                println!("{:?} -> {:?}", invoke, actual)
+            }
+            RunCommand::Run(invoke, compare, expected) => {
+                let actual = invoke_fn(&invoke.args);
+                let matched = match compare {
+                    Comparison::Equals => *expected == actual,
+                    Comparison::NotEquals => *expected != actual,
+                };
+                if !matched {
+                    return Err(format!("Failed test: {:?}, actual: {:?}", self, actual));
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
 impl Display for RunCommand {
-    fn fmt(&self, f: &mut Formatter) -> Result {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match self {
             RunCommand::Print(invocation) => write!(f, "print: {}", invocation),
             RunCommand::Run(invocation, comparison, expected) => {
@@ -58,7 +87,7 @@ impl Invocation {
 }
 
 impl Display for Invocation {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "%{}(", self.func)?;
         write_data_value_list(f, &self.args)?;
         write!(f, ")")
@@ -82,6 +111,22 @@ pub enum DataValue {
     V128([u8; 16]),
 }
 
+impl DataValue {
+    /// Return the Cranelift IR [Type] for this [DataValue].
+    pub fn ty(&self) -> Type {
+        match self {
+            DataValue::B(_) => ir::types::B8,
+            DataValue::I8(_) => ir::types::I8,
+            DataValue::I16(_) => ir::types::I16,
+            DataValue::I32(_) => ir::types::I32,
+            DataValue::I64(_) => ir::types::I64,
+            DataValue::F32(_) => ir::types::F32,
+            DataValue::F64(_) => ir::types::F64,
+            DataValue::V128(_) => ir::types::I8X16,
+        }
+    }
+}
+
 /// Helper for creating [From] implementations for [DataValue]
 macro_rules! from_data {
     ( $ty:ty, $variant:ident ) => {
@@ -102,7 +147,7 @@ from_data!(f64, F64);
 from_data!([u8; 16], V128);
 
 impl Display for DataValue {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
             DataValue::B(dv) => write!(f, "{}", dv),
             DataValue::I8(dv) => write!(f, "{}", dv),
@@ -119,7 +164,7 @@ impl Display for DataValue {
 }
 
 /// Helper function for displaying `Vec<DataValue>`.
-fn write_data_value_list(f: &mut Formatter<'_>, list: &[DataValue]) -> Result {
+fn write_data_value_list(f: &mut Formatter<'_>, list: &[DataValue]) -> fmt::Result {
     match list.len() {
         0 => Ok(()),
         1 => write!(f, "{}", list[0]),
@@ -142,10 +187,30 @@ pub enum Comparison {
 }
 
 impl Display for Comparison {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         match self {
             Comparison::Equals => write!(f, "=="),
             Comparison::NotEquals => write!(f, "!="),
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::parse_run_command;
+    use cranelift_codegen::ir::{types, AbiParam, Signature};
+    use cranelift_codegen::isa::CallConv;
+
+    #[test]
+    fn run_a_command() {
+        let mut signature = Signature::new(CallConv::Fast);
+        signature.returns.push(AbiParam::new(types::I32));
+        let command = parse_run_command(";; run: %return42() == 42 ", &signature)
+            .unwrap()
+            .unwrap();
+
+        assert!(command.run(|_| vec![DataValue::I32(42)]).is_ok());
+        assert!(command.run(|_| vec![DataValue::I32(43)]).is_err());
+    }
+}

cranelift/src/run.rs

@@ -2,9 +2,9 @@
 
 use crate::utils::read_to_string;
 use cranelift_codegen::isa::{CallConv, TargetIsa};
-use cranelift_filetests::FunctionRunner;
+use cranelift_filetests::SingleFunctionCompiler;
 use cranelift_native::builder as host_isa_builder;
-use cranelift_reader::{parse_test, Details, IsaSpec, ParseOptions};
+use cranelift_reader::{parse_run_command, parse_test, Details, IsaSpec, ParseOptions};
 use std::path::PathBuf;
 use target_lexicon::Triple;
 use walkdir::WalkDir;
@@ -90,12 +90,16 @@ fn run_file_contents(file_contents: String) -> Result<(), String> {
         ..ParseOptions::default()
     };
     let test_file = parse_test(&file_contents, options).map_err(|e| e.to_string())?;
+    let isa = create_target_isa(&test_file.isa_spec)?;
+    let mut compiler = SingleFunctionCompiler::new(isa);
     for (func, Details { comments, .. }) in test_file.functions {
-        if comments.iter().any(|c| c.text.contains("run")) {
-            // TODO in following changes we will parse this comment to alter the FunctionRunner's behavior.
-            let isa = create_target_isa(&test_file.isa_spec)?;
-            // TODO the following no longer makes sense; use FunctionRunner::with_host_isa(...) instead
-            FunctionRunner::new(func, isa).run()?
+        for comment in comments {
+            if let Some(command) =
+                parse_run_command(comment.text, &func.signature).map_err(|e| e.to_string())?
+            {
+                let compiled_fn = compiler.compile(func.clone()).map_err(|e| e.to_string())?;
+                command.run(|args| compiled_fn.call(args))?;
+            }
         }
     }
     Ok(())