From a9cda5af19ee0dc8fda2debe5ab0e1fe11caca0b Mon Sep 17 00:00:00 2001 From: Afonso Bordado Date: Wed, 5 Apr 2023 23:33:10 +0100 Subject: [PATCH] cranelift: Implement PartialEq in `Function` (#6157) --- cranelift/codegen/src/ir/function.rs | 4 +- cranelift/interpreter/src/interpreter.rs | 139 ++++++++++++----------- cranelift/interpreter/src/step.rs | 26 +---- 3 files changed, 75 insertions(+), 94 deletions(-) diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index e277aa46d4..f9dd4b7d1e 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -64,7 +64,7 @@ impl<'de> Deserialize<'de> for VersionMarker { /// Function parameters used when creating this function, and that will become applied after /// compilation to materialize the final `CompiledCode`. -#[derive(Clone)] +#[derive(Clone, PartialEq)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] pub struct FunctionParameters { /// The first `SourceLoc` appearing in the function, serving as a base for every relative @@ -351,7 +351,7 @@ impl FunctionStencil { /// Functions can be cloned, but it is not a very fast operation. /// The clone will have all the same entity numbers as the original. -#[derive(Clone)] +#[derive(Clone, PartialEq)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] pub struct Function { /// Name of this function. diff --git a/cranelift/interpreter/src/interpreter.rs b/cranelift/interpreter/src/interpreter.rs index c5c6d8b509..06b7e5f5a2 100644 --- a/cranelift/interpreter/src/interpreter.rs +++ b/cranelift/interpreter/src/interpreter.rs @@ -604,12 +604,9 @@ mod tests { let mut env = FunctionStore::default(); env.add(func.name.to_string(), &func); let state = InterpreterState::default().with_function_store(env); - let result = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_return(); + let result = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(result, vec![DataValue::I8(1)]) + assert_eq!(result, ControlFlow::Return(smallvec![DataValue::I8(1)])); } // We don't have a way to check for traps with the current filetest infrastructure @@ -626,12 +623,12 @@ mod tests { let mut env = FunctionStore::default(); env.add(func.name.to_string(), &func); let state = InterpreterState::default().with_function_store(env); - let trap = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_trap(); + let trap = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::IntegerDivisionByZero)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::IntegerDivisionByZero)) + ); } #[test] @@ -683,10 +680,9 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let result = Interpreter::new(state) .call_by_name("%parent", &[DataValue::I32(0)]) - .unwrap() - .unwrap_return(); + .unwrap(); - assert_eq!(result, vec![DataValue::I32(0)]) + assert_eq!(result, ControlFlow::Return(smallvec![DataValue::I32(0)])); } #[test] @@ -704,11 +700,9 @@ mod tests { // The default interpreter should not enable the fuel mechanism let state = InterpreterState::default().with_function_store(env.clone()); - let result = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_return(); - assert_eq!(result, vec![DataValue::I32(2)]); + let result = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); + + assert_eq!(result, ControlFlow::Return(smallvec![DataValue::I32(2)])); // With 2 fuel, we should execute the iconst and iadd, but not the return thus giving a // fuel exhausted error @@ -726,9 +720,9 @@ mod tests { let result = Interpreter::new(state) .with_fuel(Some(3)) .call_by_name("%test", &[]) - .unwrap() - .unwrap_return(); - assert_eq!(result, vec![DataValue::I32(2)]); + .unwrap(); + + assert_eq!(result, ControlFlow::Return(smallvec![DataValue::I32(2)])); } // Verifies that writing to the stack on a called function does not overwrite the parents @@ -784,10 +778,9 @@ mod tests { DataValue::I64(11), ], ) - .unwrap() - .unwrap_return(); + .unwrap(); - assert_eq!(result, vec![DataValue::I64(26)]) + assert_eq!(result, ControlFlow::Return(smallvec![DataValue::I64(26)])) } #[test] @@ -808,10 +801,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_write", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -832,10 +827,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_write", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -855,10 +852,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_load", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -878,10 +877,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_load", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -904,10 +905,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_load", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -930,10 +933,12 @@ mod tests { let state = InterpreterState::default().with_function_store(env); let trap = Interpreter::new(state) .call_by_name("%stack_store", &[]) - .unwrap() - .unwrap_trap(); + .unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapOutOfBounds)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapOutOfBounds)) + ); } #[test] @@ -950,12 +955,12 @@ mod tests { let mut env = FunctionStore::default(); env.add(func.name.to_string(), &func); let state = InterpreterState::default().with_function_store(env); - let trap = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_trap(); + let trap = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::IntegerOverflow)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::IntegerOverflow)) + ); } #[test] @@ -980,12 +985,12 @@ mod tests { }]) }); - let result = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_return(); + let result = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(result, vec![DataValue::F32(Ieee32::with_float(1.0))]) + assert_eq!( + result, + ControlFlow::Return(smallvec![DataValue::F32(Ieee32::with_float(1.0))]) + ) } #[test] @@ -1005,12 +1010,12 @@ mod tests { let mut env = FunctionStore::default(); env.add(func.name.to_string(), &func); let state = InterpreterState::default().with_function_store(env); - let trap = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_trap(); + let trap = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapMisaligned)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapMisaligned)) + ); } #[test] @@ -1031,12 +1036,12 @@ mod tests { let mut env = FunctionStore::default(); env.add(func.name.to_string(), &func); let state = InterpreterState::default().with_function_store(env); - let trap = Interpreter::new(state) - .call_by_name("%test", &[]) - .unwrap() - .unwrap_trap(); + let trap = Interpreter::new(state).call_by_name("%test", &[]).unwrap(); - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapMisaligned)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapMisaligned)) + ); } // When a trap occurs in a function called by another function, the trap was not being propagated @@ -1074,12 +1079,12 @@ mod tests { } let state = InterpreterState::default().with_function_store(env); - let trap = Interpreter::new(state) - .call_by_name("%u1", &[]) - .unwrap() - .unwrap_trap(); + let trap = Interpreter::new(state).call_by_name("%u1", &[]).unwrap(); // Ensure that the correct trap was propagated. - assert_eq!(trap, CraneliftTrap::User(TrapCode::HeapMisaligned)); + assert_eq!( + trap, + ControlFlow::Trap(CraneliftTrap::User(TrapCode::HeapMisaligned)) + ); } } diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 191e3488e3..2233c457e9 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -1370,7 +1370,7 @@ pub enum StepError { /// Enumerate the ways in which the control flow can change based on a single step in a Cranelift /// interpreter. -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum ControlFlow<'a, V> { /// Return one or more values from an instruction to be assigned to a left-hand side, e.g.: /// in `v0 = iadd v1, v2`, the sum of `v1` and `v2` is assigned to `v0`. @@ -1394,30 +1394,6 @@ pub enum ControlFlow<'a, V> { Trap(CraneliftTrap), } -impl<'a, V> ControlFlow<'a, V> { - /// For convenience, we can unwrap the [ControlFlow] state assuming that it is a - /// [ControlFlow::Return], panicking otherwise. - #[cfg(test)] - pub fn unwrap_return(self) -> Vec { - if let ControlFlow::Return(values) = self { - values.into_vec() - } else { - panic!("expected the control flow to be in the return state") - } - } - - /// For convenience, we can unwrap the [ControlFlow] state assuming that it is a - /// [ControlFlow::Trap], panicking otherwise. - #[cfg(test)] - pub fn unwrap_trap(self) -> CraneliftTrap { - if let ControlFlow::Trap(trap) = self { - trap - } else { - panic!("expected the control flow to be a trap") - } - } -} - #[derive(Error, Debug, PartialEq, Eq, Hash)] pub enum CraneliftTrap { #[error("user code: {0}")]