cranelift: Register all functions in test file for interpreter (#4817)

* cranelift: Implement `bnot` in interpreter

* cranelift: Register all functions in test file for interpreter

* cranelift: Relax signature checking for bools and vectors

cranelift/filetests/filetests/runtests/bnot.clif

@@ -0,0 +1,45 @@
+test interpret
+test run
+target x86_64
+target aarch64
+target s390x
+
+function %bnot_b1(b1) -> b1 {
+block0(v0: b1):
+    v1 = bnot.b1 v0
+    return v1
+}
+; run: %bnot_b1(false) == true
+; run: %bnot_b1(true) == false
+
+function %bnot_b8(b8) -> b8 {
+block0(v0: b8):
+    v1 = bnot.b8 v0
+    return v1
+}
+; run: %bnot_b8(false) == true
+; run: %bnot_b8(true) == false
+
+function %bnot_b16(b16) -> b16 {
+block0(v0: b16):
+    v1 = bnot.b16 v0
+    return v1
+}
+; run: %bnot_b16(false) == true
+; run: %bnot_b16(true) == false
+
+function %bnot_b32(b32) -> b32 {
+block0(v0: b32):
+    v1 = bnot.b32 v0
+    return v1
+}
+; run: %bnot_b32(false) == true
+; run: %bnot_b32(true) == false
+
+function %bnot_b64(b64) -> b64 {
+block0(v0: b64):
+    v1 = bnot.b64 v0
+    return v1
+}
+; run: %bnot_b64(false) == true
+; run: %bnot_b64(true) == false

cranelift/filetests/filetests/runtests/call.clif

@@ -1,3 +1,4 @@
+test interpret
 test run
 target x86_64
 target aarch64

cranelift/filetests/filetests/runtests/i128-bnot.clif

@@ -0,0 +1,11 @@
+test interpret
+test run
+target s390x
+
+function %bnot_b128(b128) -> b128 {
+block0(v0: b128):
+    v1 = bnot.b128 v0
+    return v1
+}
+; run: %bnot_b128(false) == true
+; run: %bnot_b128(true) == false

cranelift/filetests/filetests/runtests/i128-call.clif

@@ -1,3 +1,4 @@
+test interpret
 test run
 set enable_llvm_abi_extensions=true
 target x86_64

cranelift/filetests/src/test_interpret.rs

@@ -3,16 +3,21 @@
 //! The `interpret` test command interprets each function on the host machine
 //! using [RunCommand](cranelift_reader::RunCommand)s.
 
+use crate::runone::FileUpdate;
 use crate::runtest_environment::RuntestEnvironment;
-use crate::subtest::{Context, SubTest};
+use crate::subtest::SubTest;
+use anyhow::{anyhow, Context};
 use cranelift_codegen::data_value::DataValue;
 use cranelift_codegen::ir::types::I64;
+use cranelift_codegen::ir::Function;
+use cranelift_codegen::isa::TargetIsa;
+use cranelift_codegen::settings::Flags;
 use cranelift_codegen::{self, ir};
 use cranelift_interpreter::environment::FunctionStore;
 use cranelift_interpreter::interpreter::{HeapInit, Interpreter, InterpreterState};
 use cranelift_interpreter::step::ControlFlow;
-use cranelift_reader::{parse_run_command, TestCommand};
+use cranelift_reader::{parse_run_command, Details, TestCommand, TestFile};
-use log::trace;
+use log::{info, trace};
 use std::borrow::Cow;
 
 struct TestInterpret;
@@ -38,24 +43,63 @@ impl SubTest for TestInterpret {
         false
     }
 
-    fn run(&self, func: Cow<ir::Function>, context: &Context) -> anyhow::Result<()> {
+    /// Runs the entire subtest for a given target, invokes [Self::run] for running
-        let test_env = RuntestEnvironment::parse(&context.details.comments[..])?;
+    /// individual tests.
-        for comment in context.details.comments.iter() {
+    fn run_target<'a>(
+        &self,
+        testfile: &TestFile,
+        _: &mut FileUpdate,
+        _: &'a str,
+        _: &'a Flags,
+        _: Option<&'a dyn TargetIsa>,
+    ) -> anyhow::Result<()> {
+        // We can build the FunctionStore once and reuse it
+        let mut func_store = FunctionStore::default();
+        for (func, _) in &testfile.functions {
+            func_store.add(func.name.to_string(), &func);
+        }
+
+        for (func, details) in &testfile.functions {
+            info!("Test: {}({}) interpreter", self.name(), func.name);
+
+            let test_env = RuntestEnvironment::parse(&details.comments[..])?;
+            test_env.validate_signature(&func).map_err(|s| anyhow!(s))?;
+
+            run_test(&test_env, &func_store, func, details).context(self.name())?;
+        }
+
+        Ok(())
+    }
+
+    fn run(
+        &self,
+        _func: Cow<ir::Function>,
+        _context: &crate::subtest::Context,
+    ) -> anyhow::Result<()> {
+        unreachable!()
+    }
+}
+
+fn run_test(
+    test_env: &RuntestEnvironment,
+    func_store: &FunctionStore,
+    func: &Function,
+    details: &Details,
+) -> anyhow::Result<()> {
+    for comment in details.comments.iter() {
         if let Some(command) = parse_run_command(comment.text, &func.signature)? {
             trace!("Parsed run command: {}", command);
 
-                let mut env = FunctionStore::default();
-                env.add(func.name.to_string(), &func);
-
             command
                 .run(|func_name, run_args| {
-                        test_env.validate_signature(&func)?;
+                    // Rebuild the interpreter state on every run to ensure that we don't accidentally depend on
-
+                    // some leftover state
-                        let mut state = InterpreterState::default().with_function_store(env);
+                    let mut state =
+                        InterpreterState::default().with_function_store(func_store.clone());
 
                     let mut args = Vec::with_capacity(run_args.len());
                     if test_env.is_active() {
-                            let vmctx_addr = register_heaps(&mut state, &test_env);
+                        let vmctx_addr = register_heaps(&mut state, test_env);
                         args.push(vmctx_addr);
                     }
                     args.extend_from_slice(run_args);
@@ -64,8 +108,8 @@ impl SubTest for TestInterpret {
                     let func_name = &format!("%{}", func_name);
                     match Interpreter::new(state).call_by_name(func_name, &args) {
                         Ok(ControlFlow::Return(results)) => Ok(results.to_vec()),
-                            Ok(_) => {
+                        Ok(e) => {
-                                panic!("Unexpected returned control flow--this is likely a bug.")
+                            panic!("Unexpected returned control flow: {:?}", e)
                         }
                         Err(t) => Err(format!("unexpected trap: {:?}", t)),
                     }
@@ -74,7 +118,6 @@ impl SubTest for TestInterpret {
         }
     }
     Ok(())
-    }
 }
 
 /// Build a VMContext struct with the layout described in docs/testing.md.

cranelift/interpreter/src/step.rs

@@ -22,7 +22,17 @@ fn validate_signature_params(sig: &[AbiParam], args: &[impl Value]) -> bool {
     args.iter()
         .map(|r| r.ty())
         .zip(sig.iter().map(|r| r.value_type))
-        .all(|(a, b)| a == b)
+        .all(|(a, b)| match (a, b) {
+            // For these two cases we don't have precise type information for `a`.
+            // We don't distinguish between different bool types, or different vector types
+            // The actual error is in `Value::ty` that returns default types for some values
+            // but we don't have enough information there either.
+            //
+            // Ideally the user has run the verifier and caught this properly...
+            (a, b) if a.is_bool() && b.is_bool() => true,
+            (a, b) if a.is_vector() && b.is_vector() => true,
+            (a, b) => a == b,
+        })
 }
 
 /// Interpret a single Cranelift instruction. Note that program traps and interpreter errors are

cranelift/interpreter/src/value.rs

@@ -679,7 +679,7 @@ impl Value for DataValue {
     }
 
     fn not(self) -> ValueResult<Self> {
-        unary_match!(!(&self); [I8, I16, I32, I64])
+        unary_match!(!(&self); [B, I8, I16, I32, I64])
     }
 
     fn count_ones(self) -> ValueResult<Self> {