Merge branch 'mv/fix-signed-cast-simplification' into mv/promote-ConstantDoesNotFitInType-err

Author: vezenovm

Cargo.lock

@@ -74,6 +74,10 @@ pub enum RuntimeError {
         "Cannot return a function from an if or match expression, or assignment within these expressions"
     )]
     ReturnedFunctionFromDynamicIf { call_stack: CallStack },
+    /// This case is not an error. It's used during codegen to prevent inserting instructions after
+    /// code when a break or continue is generated.
+    #[error("Break or continue")]
+    BreakOrContinue { call_stack: CallStack },
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Hash)]
@@ -186,6 +190,7 @@ impl RuntimeError {
             | RuntimeError::UnknownReference { call_stack } => call_stack,
             RuntimeError::ReturnedReferenceFromDynamicIf { call_stack } => call_stack,
             RuntimeError::ReturnedFunctionFromDynamicIf { call_stack } => call_stack,
+            RuntimeError::BreakOrContinue { call_stack } => call_stack,
         }
     }
 }

compiler/noirc_evaluator/src/errors.rs

@@ -3,7 +3,6 @@ pub mod data_bus;
 use std::{borrow::Cow, collections::BTreeMap, sync::Arc};
 
 use acvm::{FieldElement, acir::circuit::ErrorSelector};
-use fxhash::FxHashSet as HashSet;
 use noirc_errors::{
     Location,
     call_stack::{CallStack, CallStackId},
@@ -42,8 +41,6 @@ use super::{
 pub struct FunctionBuilder {
     pub current_function: Function,
     current_block: BasicBlockId,
-    current_block_closed: bool,
-    closed_blocked: HashSet<(FunctionId, BasicBlockId)>,
     finished_functions: Vec<Function>,
     call_stack: CallStackId,
     error_types: BTreeMap<ErrorSelector, HirType>,
@@ -65,8 +62,6 @@ impl FunctionBuilder {
         let new_function = Function::new(function_name, function_id);
         Self {
             current_block: new_function.entry_block(),
-            current_block_closed: false,
-            closed_blocked: HashSet::default(),
             current_function: new_function,
             finished_functions: Vec::new(),
             call_stack: CallStackId::root(),
@@ -207,17 +202,6 @@ impl FunctionBuilder {
         self.current_function.dfg.make_block()
     }
 
-    /// Prevents any further instructions from being added to the current block.
-    /// That is, calls to add instructions can be called, but they will have no effect.
-    pub fn close_block(&mut self) {
-        self.closed_blocked.insert((self.current_function.id(), self.current_block));
-        self.current_block_closed = true;
-    }
-
-    pub fn current_block_is_closed(&self) -> bool {
-        self.current_block_closed
-    }
-
     /// Adds a parameter with the given type to the given block.
     /// Returns the newly-added parameter.
     pub fn add_block_parameter(&mut self, block: BasicBlockId, typ: Type) -> ValueId {
@@ -235,10 +219,6 @@ impl FunctionBuilder {
         instruction: Instruction,
         ctrl_typevars: Option<Vec<Type>>,
     ) -> InsertInstructionResult {
-        if self.current_block_closed {
-            return InsertInstructionResult::InstructionRemoved;
-        }
-
         let block = self.current_block();
 
         if self.simplify {
@@ -263,8 +243,6 @@ impl FunctionBuilder {
     /// instructions into a new function, call new_function instead.
     pub fn switch_to_block(&mut self, block: BasicBlockId) {
         self.current_block = block;
-        self.current_block_closed =
-            self.closed_blocked.contains(&(self.current_function.id(), block));
     }
 
     /// Returns the block currently being inserted into

compiler/noirc_evaluator/src/ssa/function_builder/mod.rs

@@ -39,6 +39,14 @@ struct Interpreter<'ssa> {
     /// expected to have no effect if there are no such instructions or if the code
     /// being executed is an unconstrained function.
     side_effects_enabled: bool,
+
+    options: InterpreterOptions,
+}
+
+#[derive(Copy, Clone, Default)]
+pub struct InterpreterOptions {
+    /// If true, the interpreter will print each value definition to stdout.
+    pub print_definitions: bool,
 }
 
 struct CallContext {
@@ -66,20 +74,33 @@ type IResults = IResult<Vec<Value>>;
 #[allow(unused)]
 impl Ssa {
     pub fn interpret(&self, args: Vec<Value>) -> IResults {
-        self.interpret_function(self.main_id, args)
+        self.interpret_with_options(args, InterpreterOptions::default())
+    }
+
+    pub fn interpret_with_options(
+        &self,
+        args: Vec<Value>,
+        options: InterpreterOptions,
+    ) -> IResults {
+        self.interpret_function(self.main_id, args, options)
     }
 
-    pub(crate) fn interpret_function(&self, function: FunctionId, args: Vec<Value>) -> IResults {
-        let mut interpreter = Interpreter::new(self);
+    fn interpret_function(
+        &self,
+        function: FunctionId,
+        args: Vec<Value>,
+        options: InterpreterOptions,
+    ) -> IResults {
+        let mut interpreter = Interpreter::new(self, options);
         interpreter.interpret_globals()?;
         interpreter.call_function(function, args)
     }
 }
 
 impl<'ssa> Interpreter<'ssa> {
-    fn new(ssa: &'ssa Ssa) -> Self {
+    fn new(ssa: &'ssa Ssa, options: InterpreterOptions) -> Self {
         let call_stack = vec![CallContext::global_context()];
-        Self { ssa, call_stack, side_effects_enabled: true }
+        Self { ssa, call_stack, side_effects_enabled: true, options }
     }
 
     fn call_context(&self) -> &CallContext {
@@ -94,12 +115,15 @@ impl<'ssa> Interpreter<'ssa> {
         &self.call_stack.first().expect("call_stack should always be non-empty").scope
     }
 
-    fn current_function(&self) -> &'ssa Function {
+    fn try_current_function(&self) -> Option<&'ssa Function> {
         let current_function_id = self.call_context().called_function;
-        let current_function_id = current_function_id.expect(
+        current_function_id.map(|current_function_id| &self.ssa.functions[&current_function_id])
+    }
+
+    fn current_function(&self) -> &'ssa Function {
+        self.try_current_function().expect(
             "Tried calling `Interpreter::current_function` while evaluating global instructions",
-        );
-        &self.ssa.functions[&current_function_id]
+        )
     }
 
     fn dfg(&self) -> &'ssa DataFlowGraph {
@@ -113,6 +137,9 @@ impl<'ssa> Interpreter<'ssa> {
     /// Define or redefine a value.
     /// Redefinitions are expected in the case of loops.
     fn define(&mut self, id: ValueId, value: Value) {
+        if self.options.print_definitions {
+            println!("{id} = {value}");
+        }
         self.call_context_mut().scope.insert(id, value);
     }
 
@@ -376,9 +403,17 @@ impl<'ssa> Interpreter<'ssa> {
         try_vecmap(ids, |id| self.lookup(*id))
     }
 
-    fn side_effects_enabled(&self) -> bool {
-        match self.current_function().runtime() {
-            RuntimeType::Acir(_) => self.side_effects_enabled,
+    fn side_effects_enabled(&self, instruction: &Instruction) -> bool {
+        let Some(current_function) = self.try_current_function() else {
+            // If there's no current function it means we are evaluating global instructions
+            return true;
+        };
+
+        match current_function.runtime() {
+            RuntimeType::Acir(_) => {
+                self.side_effects_enabled
+                    || !instruction.requires_acir_gen_predicate(&current_function.dfg)
+            }
             RuntimeType::Brillig(_) => true,
         }
     }
@@ -389,9 +424,11 @@ impl<'ssa> Interpreter<'ssa> {
         instruction: &Instruction,
         results: &[ValueId],
     ) -> IResult<()> {
+        let side_effects_enabled = self.side_effects_enabled(instruction);
+
         match instruction {
             Instruction::Binary(binary) => {
-                let result = self.interpret_binary(binary)?;
+                let result = self.interpret_binary(binary, side_effects_enabled)?;
                 self.define(results[0], result);
                 Ok(())
             }
@@ -410,7 +447,7 @@ impl<'ssa> Interpreter<'ssa> {
             Instruction::Constrain(lhs_id, rhs_id, constrain_error) => {
                 let lhs = self.lookup(*lhs_id)?;
                 let rhs = self.lookup(*rhs_id)?;
-                if self.side_effects_enabled() && lhs != rhs {
+                if side_effects_enabled && lhs != rhs {
                     let lhs = lhs.to_string();
                     let rhs = rhs.to_string();
                     let lhs_id = *lhs_id;
@@ -433,7 +470,7 @@ impl<'ssa> Interpreter<'ssa> {
             Instruction::ConstrainNotEqual(lhs_id, rhs_id, constrain_error) => {
                 let lhs = self.lookup(*lhs_id)?;
                 let rhs = self.lookup(*rhs_id)?;
-                if self.side_effects_enabled() && lhs == rhs {
+                if side_effects_enabled && lhs == rhs {
                     let lhs = lhs.to_string();
                     let rhs = rhs.to_string();
                     let lhs_id = *lhs_id;
@@ -453,10 +490,16 @@ impl<'ssa> Interpreter<'ssa> {
                 }
                 Ok(())
             }
-            Instruction::RangeCheck { value, max_bit_size, assert_message } => {
-                self.interpret_range_check(*value, *max_bit_size, assert_message.as_ref())
+            Instruction::RangeCheck { value, max_bit_size, assert_message } => self
+                .interpret_range_check(
+                    *value,
+                    *max_bit_size,
+                    assert_message.as_ref(),
+                    side_effects_enabled,
+                ),
+            Instruction::Call { func, arguments } => {
+                self.interpret_call(*func, arguments, results, side_effects_enabled)
             }
-            Instruction::Call { func, arguments } => self.interpret_call(*func, arguments, results),
             Instruction::Allocate => {
                 self.interpret_allocate(results[0]);
                 Ok(())
@@ -468,11 +511,18 @@ impl<'ssa> Interpreter<'ssa> {
                 Ok(())
             }
             Instruction::ArrayGet { array, index, offset } => {
-                self.interpret_array_get(*array, *index, *offset, results[0])
-            }
-            Instruction::ArraySet { array, index, value, mutable, offset } => {
-                self.interpret_array_set(*array, *index, *value, *mutable, *offset, results[0])
+                self.interpret_array_get(*array, *index, *offset, results[0], side_effects_enabled)
             }
+            Instruction::ArraySet { array, index, value, mutable, offset } => self
+                .interpret_array_set(
+                    *array,
+                    *index,
+                    *value,
+                    *mutable,
+                    *offset,
+                    results[0],
+                    side_effects_enabled,
+                ),
             Instruction::IncrementRc { value } => self.interpret_inc_rc(*value),
             Instruction::DecrementRc { value } => self.interpret_dec_rc(*value),
             Instruction::IfElse { then_condition, then_value, else_condition, else_value } => self
@@ -556,8 +606,9 @@ impl<'ssa> Interpreter<'ssa> {
         value_id: ValueId,
         max_bit_size: u32,
         error_message: Option<&String>,
+        side_effects_enabled: bool,
     ) -> IResult<()> {
-        if !self.side_effects_enabled() {
+        if !side_effects_enabled {
             return Ok(());
         }
 
@@ -628,11 +679,12 @@ impl<'ssa> Interpreter<'ssa> {
         function_id: ValueId,
         argument_ids: &[ValueId],
         results: &[ValueId],
+        side_effects_enabled: bool,
     ) -> IResult<()> {
         let function = self.lookup(function_id)?;
         let mut arguments = try_vecmap(argument_ids, |argument| self.lookup(*argument))?;
 
-        let new_results = if self.side_effects_enabled() {
+        let new_results = if side_effects_enabled {
             match function {
                 Value::Function(id) => {
                     // If we're crossing a constrained -> unconstrained boundary we have to wipe
@@ -762,8 +814,9 @@ impl<'ssa> Interpreter<'ssa> {
         index: ValueId,
         offset: ArrayOffset,
         result: ValueId,
+        side_effects_enabled: bool,
     ) -> IResult<()> {
-        let element = if self.side_effects_enabled() {
+        let element = if side_effects_enabled {
             let array = self.lookup_array_or_slice(array, "array get")?;
             let index = self.lookup_u32(index, "array get index")?;
             let index = index - offset.to_u32();
@@ -776,6 +829,7 @@ impl<'ssa> Interpreter<'ssa> {
         Ok(())
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn interpret_array_set(
         &mut self,
         array: ValueId,
@@ -784,10 +838,11 @@ impl<'ssa> Interpreter<'ssa> {
         mutable: bool,
         offset: ArrayOffset,
         result: ValueId,
+        side_effects_enabled: bool,
     ) -> IResult<()> {
         let array = self.lookup_array_or_slice(array, "array set")?;
 
-        let result_array = if self.side_effects_enabled() {
+        let result_array = if side_effects_enabled {
             let index = self.lookup_u32(index, "array set index")?;
             let index = index - offset.to_u32();
             let value = self.lookup(value)?;
@@ -1047,7 +1102,7 @@ macro_rules! apply_int_comparison_op {
 }
 
 impl Interpreter<'_> {
-    fn interpret_binary(&mut self, binary: &Binary) -> IResult<Value> {
+    fn interpret_binary(&mut self, binary: &Binary, side_effects_enabled: bool) -> IResult<Value> {
         let lhs_id = binary.lhs;
         let rhs_id = binary.rhs;
         let lhs = self.lookup_numeric(lhs_id, "binary op lhs")?;
@@ -1066,7 +1121,7 @@ impl Interpreter<'_> {
         }
 
         // Disable this instruction if it is side-effectful and side effects are disabled.
-        if !self.side_effects_enabled() && binary.requires_acir_gen_predicate(self.dfg()) {
+        if !side_effects_enabled {
             let zero = NumericValue::zero(lhs.get_type());
             return Ok(Value::Numeric(zero));
         }