diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 76ab613ed1f..a9801c8904e 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -725,7 +725,7 @@ impl<'block> BrilligBlock<'block> { 1, ); } - Instruction::EnableSideEffects { .. } => { + Instruction::EnableSideEffectsIf { .. } => { todo!("enable_side_effects not supported by brillig") } Instruction::IfElse { .. } => { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index fca1f60544d..c17088a5d8c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -63,7 +63,7 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { /// /// This is equivalent to the Noir (pseudo)code /// -/// ```ignore +/// ```text /// fn quotient(a: T, b: T) -> (T,T) { /// (a/b, a-a/b*b) /// } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 88b7ff4fe58..2d138c13f7f 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -114,7 +114,7 @@ pub(crate) fn optimize_into_acir( .run_pass(Ssa::inline_functions_with_no_predicates, "After Inlining:") .run_pass(Ssa::remove_if_else, "After Remove IfElse:") .run_pass(Ssa::fold_constants, "After Constant Folding:") - .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:") + .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffectsIf removal:") .run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:") .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:") .run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:") diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index c2cea6e6a00..a6b962a45b2 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -492,7 +492,7 @@ impl AcirContext { self.sub_var(sum, mul) } else { // Implement OR in terms of AND - // (NOT a) NAND (NOT b) => a OR b + // (NOT a) AND (NOT b) => NOT (a OR b) let a = self.not_var(lhs, typ.clone())?; let b = self.not_var(rhs, typ.clone())?; let a_and_b = self.and_var(a, b, typ.clone())?; diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index c9b3d0f9c6a..0a81990ab10 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -706,7 +706,7 @@ impl<'a> Context<'a> { self.convert_ssa_truncate(*value, *bit_size, *max_bit_size, dfg)?; self.define_result_var(dfg, instruction_id, result_acir_var); } - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { let acir_var = self.convert_numeric_value(*condition, dfg)?; self.current_side_effects_enabled_var = acir_var; } diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 24fcb8f61df..79db4e645ee 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -255,7 +255,7 @@ impl Context { } Instruction::Allocate { .. } | Instruction::DecrementRc { .. } - | Instruction::EnableSideEffects { .. } + | Instruction::EnableSideEffectsIf { .. } | Instruction::IncrementRc { .. } | Instruction::RangeCheck { .. } => {} } diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 49184bf4c63..8cc42241d92 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -337,7 +337,7 @@ impl FunctionBuilder { /// Insert an enable_side_effects_if instruction. These are normally only automatically /// inserted during the flattening pass when branching is removed. pub(crate) fn insert_enable_side_effects_if(&mut self, condition: ValueId) { - self.insert_instruction(Instruction::EnableSideEffects { condition }, None); + self.insert_instruction(Instruction::EnableSideEffectsIf { condition }, None); } /// Terminates the current block with the given terminator instruction diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index e5c40652706..c3cd27bf179 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -218,13 +218,23 @@ pub(crate) enum Instruction { Store { address: ValueId, value: ValueId }, /// Provides a context for all instructions that follow up until the next - /// `EnableSideEffects` is encountered, for stating a condition that determines whether + /// `EnableSideEffectsIf` is encountered, for stating a condition that determines whether /// such instructions are allowed to have side-effects. /// + /// For example, + /// ```text + /// EnableSideEffectsIf condition0; + /// code0; + /// EnableSideEffectsIf condition1; + /// code1; + /// ``` + /// - `code0` will have side effects iff `condition0` evaluates to `true` + /// - `code1` will have side effects iff `condition1` evaluates to `true` + /// /// This instruction is only emitted after the cfg flattening pass, and is used to annotate /// instruction regions with an condition that corresponds to their position in the CFG's /// if-branching structure. - EnableSideEffects { condition: ValueId }, + EnableSideEffectsIf { condition: ValueId }, /// Retrieve a value from an array at the given index ArrayGet { array: ValueId, index: ValueId }, @@ -249,6 +259,17 @@ pub(crate) enum Instruction { DecrementRc { value: ValueId }, /// Merge two values returned from opposite branches of a conditional into one. + /// + /// ```text + /// if then_condition { + /// then_value + /// } else { // else_condition = !then_condition + /// else_value + /// } + /// ``` + /// + /// Where we save the result of !then_condition so that we have the same + /// ValueId for it each time. IfElse { then_condition: ValueId, then_value: ValueId, @@ -279,7 +300,7 @@ impl Instruction { | Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } | Instruction::RangeCheck { .. } - | Instruction::EnableSideEffects { .. } => InstructionResultType::None, + | Instruction::EnableSideEffectsIf { .. } => InstructionResultType::None, Instruction::Allocate { .. } | Instruction::Load { .. } | Instruction::ArrayGet { .. } @@ -306,7 +327,7 @@ impl Instruction { match self { // These either have side-effects or interact with memory - EnableSideEffects { .. } + EnableSideEffectsIf { .. } | Allocate | Load { .. } | Store { .. } @@ -362,7 +383,7 @@ impl Instruction { Constrain(..) | Store { .. } - | EnableSideEffects { .. } + | EnableSideEffectsIf { .. } | IncrementRc { .. } | DecrementRc { .. } | RangeCheck { .. } => false, @@ -401,7 +422,7 @@ impl Instruction { !dfg.is_safe_index(*index, *array) } - Instruction::EnableSideEffects { .. } | Instruction::ArraySet { .. } => true, + Instruction::EnableSideEffectsIf { .. } | Instruction::ArraySet { .. } => true, Instruction::Call { func, .. } => match dfg[*func] { Value::Function(_) => true, @@ -466,8 +487,8 @@ impl Instruction { Instruction::Store { address, value } => { Instruction::Store { address: f(*address), value: f(*value) } } - Instruction::EnableSideEffects { condition } => { - Instruction::EnableSideEffects { condition: f(*condition) } + Instruction::EnableSideEffectsIf { condition } => { + Instruction::EnableSideEffectsIf { condition: f(*condition) } } Instruction::ArrayGet { array, index } => { Instruction::ArrayGet { array: f(*array), index: f(*index) } @@ -541,7 +562,7 @@ impl Instruction { f(*index); f(*value); } - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { f(*condition); } Instruction::IncrementRc { value } @@ -678,11 +699,11 @@ impl Instruction { Instruction::Call { func, arguments } => { simplify_call(*func, arguments, dfg, block, ctrl_typevars, call_stack) } - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { if let Some(last) = dfg[block].instructions().last().copied() { let last = &mut dfg[last]; - if matches!(last, Instruction::EnableSideEffects { .. }) { - *last = Instruction::EnableSideEffects { condition: *condition }; + if matches!(last, Instruction::EnableSideEffectsIf { .. }) { + *last = Instruction::EnableSideEffectsIf { condition: *condition }; return Remove; } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 656bd26620e..e8c9d01988e 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -176,7 +176,7 @@ fn display_instruction_inner( Instruction::Store { address, value } => { writeln!(f, "store {} at {}", show(*value), show(*address)) } - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { writeln!(f, "enable_side_effects {}", show(*condition)) } Instruction::ArrayGet { array, index } => { diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 160105d27e6..c8f6d201d86 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -159,9 +159,9 @@ impl Context { *side_effects_enabled_var, ); - // If we just inserted an `Instruction::EnableSideEffects`, we need to update `side_effects_enabled_var` + // If we just inserted an `Instruction::EnableSideEffectsIf`, we need to update `side_effects_enabled_var` // so that we use the correct set of constrained values in future. - if let Instruction::EnableSideEffects { condition } = instruction { + if let Instruction::EnableSideEffectsIf { condition } = instruction { *side_effects_enabled_var = condition; }; } diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index 1aa0c2efbd0..b9804062118 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -245,7 +245,7 @@ impl Context { let instruction_id = *instruction_id; let instruction = &function.dfg[instruction_id]; - if let Instruction::EnableSideEffects { condition } = instruction { + if let Instruction::EnableSideEffectsIf { condition } = instruction { side_effects_condition = Some(*condition); // We still need to keep the EnableSideEffects instruction diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 288e41cb994..72ed02b00a8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -11,7 +11,7 @@ //! elimination (DIE) pass. //! //! Though CFG information is lost during this pass, some key information is retained in the form -//! of `EnableSideEffect` instructions. Each time the flattening pass enters and exits a branch of +//! of `EnableSideEffectsIf` instructions. Each time the flattening pass enters and exits a branch of //! a jmpif, an instruction is inserted to capture a condition that is analogous to the activeness //! of the program point. For example: //! @@ -573,7 +573,7 @@ impl<'f> Context<'f> { } /// Checks the branch condition on the top of the stack and uses it to build and insert an - /// `EnableSideEffects` instruction into the entry block. + /// `EnableSideEffectsIf` instruction into the entry block. /// /// If the stack is empty, a "true" u1 constant is taken to be the active condition. This is /// necessary for re-enabling side-effects when re-emerging to a branch depth of 0. @@ -584,7 +584,7 @@ impl<'f> Context<'f> { self.inserter.function.dfg.make_constant(FieldElement::one(), Type::unsigned(1)) } }; - let enable_side_effects = Instruction::EnableSideEffects { condition }; + let enable_side_effects = Instruction::EnableSideEffectsIf { condition }; let call_stack = self.inserter.function.dfg.get_value_call_stack(condition); self.insert_instruction_with_typevars(enable_side_effects, None, call_stack); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 90e24a1d5e3..7c2db62b0ea 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -374,7 +374,7 @@ impl<'a> ValueMerger<'a> { for (index, element_type, condition) in changed_indices { let typevars = Some(vec![element_type.clone()]); - let instruction = Instruction::EnableSideEffects { condition }; + let instruction = Instruction::EnableSideEffectsIf { condition }; self.insert_instruction(instruction); let mut get_element = |array, typevars| { @@ -398,7 +398,7 @@ impl<'a> ValueMerger<'a> { array = self.insert_array_set(array, index, value, Some(condition)).first(); } - let instruction = Instruction::EnableSideEffects { condition: current_condition }; + let instruction = Instruction::EnableSideEffectsIf { condition: current_condition }; self.insert_instruction(instruction); Some(array) } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index d78399a3e6b..1ff593a1531 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -502,7 +502,7 @@ impl<'function> PerFunctionContext<'function> { } None => self.push_instruction(*id), }, - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { side_effects_enabled = Some(self.translate_value(*condition)); self.push_instruction(*id); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 224060e131f..a56786b2603 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -1,13 +1,13 @@ -//! The goal of the "remove enable side effects" optimization pass is to delay any [Instruction::EnableSideEffects] +//! The goal of the "remove enable side effects" optimization pass is to delay any [Instruction::EnableSideEffectsIf] //! instructions such that they cover the minimum number of instructions possible. //! //! The pass works as follows: -//! - Insert instructions until an [Instruction::EnableSideEffects] is encountered, save this [InstructionId]. +//! - Insert instructions until an [Instruction::EnableSideEffectsIf] is encountered, save this [InstructionId]. //! - Continue inserting instructions until either -//! - Another [Instruction::EnableSideEffects] is encountered, if so then drop the previous [InstructionId] in favour +//! - Another [Instruction::EnableSideEffectsIf] is encountered, if so then drop the previous [InstructionId] in favour //! of this one. -//! - An [Instruction] with side-effects is encountered, if so then insert the currently saved [Instruction::EnableSideEffects] -//! before the [Instruction]. Continue inserting instructions until the next [Instruction::EnableSideEffects] is encountered. +//! - An [Instruction] with side-effects is encountered, if so then insert the currently saved [Instruction::EnableSideEffectsIf] +//! before the [Instruction]. Continue inserting instructions until the next [Instruction::EnableSideEffectsIf] is encountered. use std::collections::HashSet; use acvm::{acir::AcirField, FieldElement}; @@ -70,10 +70,10 @@ impl Context { for instruction_id in instructions { let instruction = &function.dfg[instruction_id]; - // If we run into another `Instruction::EnableSideEffects` before encountering any + // If we run into another `Instruction::EnableSideEffectsIf` before encountering any // instructions with side effects then we can drop the instruction we're holding and - // continue with the new `Instruction::EnableSideEffects`. - if let Instruction::EnableSideEffects { condition } = instruction { + // continue with the new `Instruction::EnableSideEffectsIf`. + if let Instruction::EnableSideEffectsIf { condition } = instruction { // If this instruction isn't changing the currently active condition then we can ignore it. if active_condition == *condition { continue; @@ -98,7 +98,7 @@ impl Context { } // If we hit an instruction which is affected by the side effects var then we must insert the - // `Instruction::EnableSideEffects` before we insert this new instruction. + // `Instruction::EnableSideEffectsIf` before we insert this new instruction. if Self::responds_to_side_effects_var(&function.dfg, instruction) { if let Some(enable_side_effects_instruction_id) = last_side_effects_enabled_instruction.take() @@ -140,7 +140,7 @@ impl Context { | IncrementRc { .. } | DecrementRc { .. } => false, - EnableSideEffects { .. } + EnableSideEffectsIf { .. } | ArrayGet { .. } | ArraySet { .. } | Allocate diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index b1ca5fa25a0..cc02faeb3df 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -128,7 +128,7 @@ impl Context { self.slice_sizes.insert(result, old_capacity); function.dfg[block].instructions_mut().push(instruction); } - Instruction::EnableSideEffects { condition } => { + Instruction::EnableSideEffectsIf { condition } => { current_conditional = *condition; function.dfg[block].instructions_mut().push(instruction); } diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index f7bcfd58b72..eb6b4bf7bd4 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -1,3 +1,5 @@ +use std::fmt::Display; + use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{ @@ -67,6 +69,12 @@ pub struct LocalId(pub u32); #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct FuncId(pub u32); +impl Display for FuncId { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + #[derive(Debug, Clone, Hash)] pub struct Ident { pub location: Option, @@ -357,16 +365,12 @@ impl Program { FuncId(0) } - pub fn take_main_body(&mut self) -> Expression { - self.take_function_body(FuncId(0)) - } - /// Takes a function body by replacing it with `false` and /// returning the previous value pub fn take_function_body(&mut self, function: FuncId) -> Expression { - let main = &mut self.functions[function.0 as usize]; - let replacement = Expression::Literal(Literal::Bool(false)); - std::mem::replace(&mut main.body, replacement) + let function_definition = &mut self[function]; + let replacement = Expression::Block(vec![]); + std::mem::replace(&mut function_definition.body, replacement) } } diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index 9b1eeecdc1a..b6421b26a03 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -19,7 +19,7 @@ impl AstPrinter { write!( f, "fn {}$f{}({}) -> {} {{", - function.name, function.id.0, params, function.return_type + function.name, function.id, params, function.return_type )?; self.indent_level += 1; self.print_expr_expect_block(&function.body, f)?; @@ -291,7 +291,7 @@ impl Display for Definition { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { match self { Definition::Local(id) => write!(f, "l{}", id.0), - Definition::Function(id) => write!(f, "f{}", id.0), + Definition::Function(id) => write!(f, "f{}", id), Definition::Builtin(name) => write!(f, "{name}"), Definition::LowLevel(name) => write!(f, "{name}"), Definition::Oracle(name) => write!(f, "{name}"),