Revert "cranelift: Register all functions in test file for interpreter (

#4800)" (#4810)

This reverts commit 500a9f1.

cranelift/filetests/filetests/runtests/call.clif

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

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

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

cranelift/filetests/src/test_interpret.rs

@@ -3,21 +3,16 @@
 //! 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::SubTest;
-use anyhow::{anyhow, Context};
+use crate::subtest::{Context, SubTest};
 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, Details, TestCommand, TestFile};
-use log::{info, trace};
+use cranelift_reader::{parse_run_command, TestCommand};
+use log::trace;
 use std::borrow::Cow;
 
 struct TestInterpret;
@@ -43,81 +38,43 @@ impl SubTest for TestInterpret {
         false
     }
 
-    /// Runs the entire subtest for a given target, invokes [Self::run] for running
-    /// individual tests.
-    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: &Context) -> anyhow::Result<()> {
+        let test_env = RuntestEnvironment::parse(&context.details.comments[..])?;
+        for comment in context.details.comments.iter() {
+            if let Some(command) = parse_run_command(comment.text, &func.signature)? {
+                trace!("Parsed run command: {}", command);
 
-    fn run(
-        &self,
-        _func: Cow<ir::Function>,
-        _context: &crate::subtest::Context,
-    ) -> anyhow::Result<()> {
-        unreachable!()
-    }
-}
+                let mut env = FunctionStore::default();
+                env.add(func.name.to_string(), &func);
 
-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);
+                command
+                    .run(|func_name, run_args| {
+                        test_env.validate_signature(&func)?;
 
-            command
-                .run(|func_name, run_args| {
-                    // 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(func_store.clone());
+                        let mut state = InterpreterState::default().with_function_store(env);
 
-                    let mut args = Vec::with_capacity(run_args.len());
-                    if test_env.is_active() {
-                        let vmctx_addr = register_heaps(&mut state, test_env);
-                        args.push(vmctx_addr);
-                    }
-                    args.extend_from_slice(run_args);
-
-                    // Because we have stored function names with a leading %, we need to re-add it.
-                    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(e) => {
-                            panic!("Unexpected returned control flow: {:?}", e)
+                        let mut args = Vec::with_capacity(run_args.len());
+                        if test_env.is_active() {
+                            let vmctx_addr = register_heaps(&mut state, &test_env);
+                            args.push(vmctx_addr);
+                        }
+                        args.extend_from_slice(run_args);
+
+                        // Because we have stored function names with a leading %, we need to re-add it.
+                        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(_) => {
+                                panic!("Unexpected returned control flow--this is likely a bug.")
+                            }
+                            Err(t) => Err(format!("unexpected trap: {:?}", t)),
                         }
-                        Err(t) => Err(format!("unexpected trap: {:?}", t)),
-                    }
-                })
-                .map_err(|e| anyhow::anyhow!("{}", e))?;
+                    })
+                    .map_err(|e| anyhow::anyhow!("{}", e))?;
+            }
         }
+        Ok(())
     }
-    Ok(())
 }
 
 /// Build a VMContext struct with the layout described in docs/testing.md.

cranelift/interpreter/src/value.rs

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