Rewrite interpreter generically (#2323)

* Rewrite interpreter generically

This change re-implements the Cranelift interpreter to use generic values; this makes it possible to do abstract interpretation of Cranelift instructions. In doing so, the interpretation state is extracted from the `Interpreter` structure and is accessed via a `State` trait; this makes it possible to not only more clearly observe the interpreter's state but also to interpret using a dummy state (e.g. `ImmutableRegisterState`). This addition made it possible to implement more of the Cranelift instructions (~70%, ignoring the x86-specific instructions).

* Replace macros with closures

.github/workflows/main.yml

@@ -50,7 +50,7 @@ jobs:
     name: Doc - build the API documentation
     runs-on: ubuntu-latest
     env:
-      RUSTDOCFLAGS: -Dintra-doc-link-resolution-failure
+      RUSTDOCFLAGS: -Dbroken_intra_doc_links
     steps:
     - uses: actions/checkout@v2
       with:

cranelift/codegen/src/data_value.rs

@@ -1,48 +1,54 @@
 //! This module gives users to instantiate values that Cranelift understands. These values are used,
 //! for example, during interpretation and for wrapping immediates.
-use crate::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32};
+use crate::ir::immediates::{Ieee32, Ieee64, Offset32};
 use crate::ir::{types, ConstantData, Type};
 use core::convert::TryInto;
 use core::fmt::{self, Display, Formatter};
+use core::ptr;
 use thiserror::Error;
 
 /// Represent a data value. Where [Value] is an SSA reference, [DataValue] is the type + value
 /// that would be referred to by a [Value].
 ///
 /// [Value]: crate::ir::Value
 #[allow(missing_docs)]
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq, PartialOrd)]
 pub enum DataValue {
     B(bool),
     I8(i8),
     I16(i16),
     I32(i32),
     I64(i64),
+    U8(u8),
+    U16(u16),
+    U32(u32),
+    U64(u64),
     F32(Ieee32),
     F64(Ieee64),
     V128([u8; 16]),
 }
 
 impl DataValue {
-    /// Try to cast an immediate integer ([Imm64]) to the given Cranelift [Type].
-    pub fn from_integer(imm: Imm64, ty: Type) -> Result<DataValue, DataValueCastFailure> {
+    /// Try to cast an immediate integer (a wrapped `i64` on most Cranelift instructions) to the
+    /// given Cranelift [Type].
+    pub fn from_integer(imm: i64, ty: Type) -> Result<DataValue, DataValueCastFailure> {
         match ty {
-            types::I8 => Ok(DataValue::I8(imm.bits() as i8)),
-            types::I16 => Ok(DataValue::I16(imm.bits() as i16)),
-            types::I32 => Ok(DataValue::I32(imm.bits() as i32)),
-            types::I64 => Ok(DataValue::I64(imm.bits())),
-            _ => Err(DataValueCastFailure::FromImm64(imm, ty)),
+            types::I8 => Ok(DataValue::I8(imm as i8)),
+            types::I16 => Ok(DataValue::I16(imm as i16)),
+            types::I32 => Ok(DataValue::I32(imm as i32)),
+            types::I64 => Ok(DataValue::I64(imm)),
+            _ => Err(DataValueCastFailure::FromInteger(imm, ty)),
         }
     }
 
     /// Return the Cranelift IR [Type] for this [DataValue].
     pub fn ty(&self) -> Type {
         match self {
             DataValue::B(_) => types::B8, // A default type.
-            DataValue::I8(_) => types::I8,
-            DataValue::I16(_) => types::I16,
-            DataValue::I32(_) => types::I32,
-            DataValue::I64(_) => types::I64,
+            DataValue::I8(_) | DataValue::U8(_) => types::I8,
+            DataValue::I16(_) | DataValue::U16(_) => types::I16,
+            DataValue::I32(_) | DataValue::U32(_) => types::I32,
+            DataValue::I64(_) | DataValue::U64(_) => types::I64,
             DataValue::F32(_) => types::F32,
             DataValue::F64(_) => types::F64,
             DataValue::V128(_) => types::I8X16, // A default type.
@@ -56,6 +62,38 @@ impl DataValue {
             _ => false,
         }
     }
+
+    /// Write a [DataValue] to a memory location.
+    pub unsafe fn write_value_to(&self, p: *mut u128) {
+        match self {
+            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 Ieee32, *f),
+            DataValue::F64(f) => ptr::write(p as *mut Ieee64, *f),
+            DataValue::V128(b) => ptr::write(p as *mut [u8; 16], *b),
+            _ => unimplemented!(),
+        }
+    }
+
+    /// Read a [DataValue] from a memory location using a given [Type].
+    pub unsafe fn read_value_from(p: *const u128, ty: Type) -> Self {
+        match ty {
+            types::I8 => DataValue::I8(ptr::read(p as *const i8)),
+            types::I16 => DataValue::I16(ptr::read(p as *const i16)),
+            types::I32 => DataValue::I32(ptr::read(p as *const i32)),
+            types::I64 => DataValue::I64(ptr::read(p as *const i64)),
+            types::F32 => DataValue::F32(ptr::read(p as *const Ieee32)),
+            types::F64 => DataValue::F64(ptr::read(p as *const Ieee64)),
+            _ 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!(),
+        }
+    }
 }
 
 /// Record failures to cast [DataValue].
@@ -64,8 +102,8 @@ impl DataValue {
 pub enum DataValueCastFailure {
     #[error("unable to cast data value of type {0} to type {1}")]
     TryInto(Type, Type),
-    #[error("unable to cast Imm64({0}) to a data value of type {1}")]
-    FromImm64(Imm64, Type),
+    #[error("unable to cast i64({0}) to a data value of type {1}")]
+    FromInteger(i64, Type),
 }
 
 /// Helper for creating conversion implementations for [DataValue].
@@ -97,6 +135,10 @@ build_conversion_impl!(i8, I8, I8);
 build_conversion_impl!(i16, I16, I16);
 build_conversion_impl!(i32, I32, I32);
 build_conversion_impl!(i64, I64, I64);
+build_conversion_impl!(u8, U8, I8);
+build_conversion_impl!(u16, U16, I16);
+build_conversion_impl!(u32, U32, I32);
+build_conversion_impl!(u64, U64, I64);
 build_conversion_impl!(Ieee32, F32, F32);
 build_conversion_impl!(Ieee64, F64, F64);
 build_conversion_impl!([u8; 16], V128, I8X16);
@@ -114,6 +156,10 @@ impl Display for DataValue {
             DataValue::I16(dv) => write!(f, "{}", dv),
             DataValue::I32(dv) => write!(f, "{}", dv),
             DataValue::I64(dv) => write!(f, "{}", dv),
+            DataValue::U8(dv) => write!(f, "{}", dv),
+            DataValue::U16(dv) => write!(f, "{}", dv),
+            DataValue::U32(dv) => write!(f, "{}", dv),
+            DataValue::U64(dv) => write!(f, "{}", dv),
             // The Ieee* wrappers here print the expected syntax.
             DataValue::F32(dv) => write!(f, "{}", dv),
             DataValue::F64(dv) => write!(f, "{}", dv),

cranelift/codegen/src/ir/immediates.rs

@@ -5,6 +5,7 @@
 //! `cranelift-codegen/meta/src/shared/immediates` crate in the meta language.
 
 use alloc::vec::Vec;
+use core::cmp::Ordering;
 use core::fmt::{self, Display, Formatter};
 use core::str::FromStr;
 use core::{i32, u32};
@@ -739,6 +740,17 @@ impl Ieee32 {
     pub fn bits(self) -> u32 {
         self.0
     }
+
+    /// Check if the value is a NaN.
+    pub fn is_nan(&self) -> bool {
+        f32::from_bits(self.0).is_nan()
+    }
+}
+
+impl PartialOrd for Ieee32 {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        f32::from_bits(self.0).partial_cmp(&f32::from_bits(other.0))
+    }
 }
 
 impl Display for Ieee32 {
@@ -812,6 +824,18 @@ impl Ieee64 {
     pub fn bits(self) -> u64 {
         self.0
     }
+
+    /// Check if the value is a NaN. For [Ieee64], this means checking that the 11 exponent bits are
+    /// all set.
+    pub fn is_nan(&self) -> bool {
+        f64::from_bits(self.0).is_nan()
+    }
+}
+
+impl PartialOrd for Ieee64 {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        f64::from_bits(self.0).partial_cmp(&f64::from_bits(other.0))
+    }
 }
 
 impl Display for Ieee64 {

cranelift/filetests/src/function_runner.rs

@@ -237,6 +237,7 @@ impl UnboxedValues {
             DataValue::F32(f) => ptr::write(p as *mut Ieee32, *f),
             DataValue::F64(f) => ptr::write(p as *mut Ieee64, *f),
             DataValue::V128(b) => ptr::write(p as *mut [u8; 16], *b),
+            _ => unimplemented!(),
         }
     }
 

cranelift/filetests/src/test_interpret.rs

@@ -5,8 +5,9 @@
 
 use crate::subtest::{Context, SubTest};
 use cranelift_codegen::{self, ir};
-use cranelift_interpreter::environment::Environment;
-use cranelift_interpreter::interpreter::{ControlFlow, Interpreter};
+use cranelift_interpreter::environment::FunctionStore;
+use cranelift_interpreter::interpreter::{Interpreter, InterpreterState};
+use cranelift_interpreter::step::ControlFlow;
 use cranelift_reader::{parse_run_command, TestCommand};
 use log::trace;
 use std::borrow::Cow;
@@ -39,16 +40,16 @@ impl SubTest for TestInterpret {
             if let Some(command) = parse_run_command(comment.text, &func.signature)? {
                 trace!("Parsed run command: {}", command);
 
-                let mut env = Environment::default();
-                env.add(func.name.to_string(), func.clone().into_owned());
-                let interpreter = Interpreter::new(env);
+                let mut env = FunctionStore::default();
+                env.add(func.name.to_string(), &func);
 
                 command
                     .run(|func_name, args| {
                         // Because we have stored function names with a leading %, we need to re-add it.
                         let func_name = &format!("%{}", func_name);
-                        match interpreter.call_by_name(func_name, args) {
-                            Ok(ControlFlow::Return(results)) => Ok(results),
+                        let state = InterpreterState::default().with_function_store(env);
+                        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.")
                             }

cranelift/interpreter/Cargo.toml

@@ -11,10 +11,11 @@ readme = "README.md"
 edition = "2018"
 
 [dependencies]
-cranelift-codegen = { path = "../codegen", version = "0.67.0", default-features = false }
+cranelift-codegen = { path = "../codegen", version = "0.67.0", features = ["all-arch"] }
 cranelift-entity = { path = "../entity", version = "0.67.0" }
 cranelift-reader = { path = "../reader", version = "0.67.0" }
 log = { version = "0.4.8", default-features = false }
+smallvec = "1.4.2"
 thiserror = "1.0.15"
 
 [dev-dependencies]

cranelift/interpreter/src/environment.rs

@@ -3,14 +3,14 @@
 use cranelift_codegen::ir::{FuncRef, Function};
 use std::collections::HashMap;
 
-#[derive(Default)]
-pub struct Environment {
-    functions: HashMap<FuncRef, Function>,
+#[derive(Default, Clone)]
+pub struct FunctionStore<'a> {
+    functions: HashMap<FuncRef, &'a Function>,
     function_name_to_func_ref: HashMap<String, FuncRef>,
 }
 
-impl From<Function> for Environment {
-    fn from(f: Function) -> Self {
+impl<'a> From<&'a Function> for FunctionStore<'a> {
+    fn from(f: &'a Function) -> Self {
         let func_ref = FuncRef::from_u32(0);
         let mut function_name_to_func_ref = HashMap::new();
         function_name_to_func_ref.insert(f.name.to_string(), func_ref);
@@ -23,9 +23,9 @@ impl From<Function> for Environment {
     }
 }
 
-impl Environment {
+impl<'a> FunctionStore<'a> {
     /// Add a function by name.
-    pub fn add(&mut self, name: String, function: Function) {
+    pub fn add(&mut self, name: String, function: &'a Function) {
         let func_ref = FuncRef::with_number(self.function_name_to_func_ref.len() as u32)
             .expect("a valid function reference");
         self.function_name_to_func_ref.insert(name, func_ref);
@@ -38,12 +38,12 @@ impl Environment {
     }
 
     /// Retrieve a function by its function reference.
-    pub fn get_by_func_ref(&self, func_ref: FuncRef) -> Option<&Function> {
-        self.functions.get(&func_ref)
+    pub fn get_by_func_ref(&self, func_ref: FuncRef) -> Option<&'a Function> {
+        self.functions.get(&func_ref).cloned()
     }
 
     /// Retrieve a function by its name.
-    pub fn get_by_name(&self, name: &str) -> Option<&Function> {
+    pub fn get_by_name(&self, name: &str) -> Option<&'a Function> {
         let func_ref = self.index_of(name)?;
         self.get_by_func_ref(func_ref)
     }
@@ -57,26 +57,26 @@ mod tests {
 
     #[test]
     fn addition() {
-        let mut env = Environment::default();
+        let mut env = FunctionStore::default();
         let a = "a";
         let f = Function::new();
 
-        env.add(a.to_string(), f);
+        env.add(a.to_string(), &f);
         assert!(env.get_by_name(a).is_some());
     }
 
     #[test]
     fn nonexistence() {
-        let env = Environment::default();
+        let env = FunctionStore::default();
         assert!(env.get_by_name("a").is_none());
     }
 
     #[test]
     fn from() {
         let name = ExternalName::testcase("test");
         let signature = Signature::new(CallConv::Fast);
-        let func = Function::with_name_signature(name, signature);
-        let env: Environment = func.into();
+        let func = &Function::with_name_signature(name, signature);
+        let env: FunctionStore = func.into();
         assert_eq!(env.index_of("%test"), FuncRef::with_number(0));
     }
 }

cranelift/interpreter/src/frame.rs

@@ -32,16 +32,16 @@ impl<'a> Frame<'a> {
 
     /// Retrieve the actual value associated with an SSA reference.
     #[inline]
-    pub fn get(&self, name: &ValueRef) -> &DataValue {
+    pub fn get(&self, name: ValueRef) -> &DataValue {
         trace!("Get {}", name);
         self.registers
-            .get(name)
+            .get(&name)
             .unwrap_or_else(|| panic!("unknown value: {}", name))
     }
 
     /// Retrieve multiple SSA references; see `get`.
     pub fn get_all(&self, names: &[ValueRef]) -> Vec<DataValue> {
-        names.iter().map(|r| self.get(r)).cloned().collect()
+        names.iter().map(|r| self.get(*r)).cloned().collect()
     }
 
     /// Assign `value` to the SSA reference `name`.
@@ -108,7 +108,7 @@ mod tests {
         let a = ValueRef::with_number(1).unwrap();
         let fortytwo = DataValue::I32(42);
         frame.set(a, fortytwo.clone());
-        assert_eq!(frame.get(&a), &fortytwo);
+        assert_eq!(frame.get(a), &fortytwo);
     }
 
     #[test]
@@ -118,6 +118,6 @@ mod tests {
         let frame = Frame::new(&func);
 
         let a = ValueRef::with_number(1).unwrap();
-        frame.get(&a);
+        frame.get(a);
     }
 }