diff --git a/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs b/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs index d3b7cbe9dec..48438bb9cbe 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs @@ -3,7 +3,7 @@ use crate::ssa::{ function::Function, instruction::{Instruction, Intrinsic}, types::{NumericType, Type}, - value::{Value, ValueMapping}, + value::Value, }, ssa_gen::Ssa, }; @@ -27,46 +27,30 @@ impl Ssa { impl Function { pub(crate) fn as_slice_optimization(&mut self) { - let mut values_to_replace = ValueMapping::default(); + self.simple_reachable_blocks_optimization(|context| { + let instruction_id = context.instruction_id; + let instruction = context.instruction(); - for block in self.reachable_blocks() { - let instruction_ids = self.dfg[block].take_instructions(); - for instruction_id in &instruction_ids { - let (target_func, first_argument) = { - let instruction = &mut self.dfg[*instruction_id]; - instruction.replace_values(&values_to_replace); + let (target_func, arguments) = match &instruction { + Instruction::Call { func, arguments } => (func, arguments), + _ => return, + }; - let (target_func, arguments) = match &instruction { - Instruction::Call { func, arguments } => (func, arguments), - _ => continue, - }; + let Value::Intrinsic(Intrinsic::AsSlice) = context.dfg[*target_func] else { + return; + }; - (*target_func, arguments.first().copied()) - }; + let first_argument = arguments.first().unwrap(); + let array_typ = context.dfg.type_of_value(*first_argument); + let Type::Array(_, length) = array_typ else { + unreachable!("AsSlice called with non-array {}", array_typ); + }; - match &self.dfg[target_func] { - Value::Intrinsic(Intrinsic::AsSlice) => { - let first_argument = first_argument.unwrap(); - let array_typ = self.dfg.type_of_value(first_argument); - if let Type::Array(_, length) = array_typ { - let call_returns = self.dfg.instruction_results(*instruction_id); - let original_slice_length = call_returns[0]; - let known_length = - self.dfg.make_constant(length.into(), NumericType::length_type()); - values_to_replace.insert(original_slice_length, known_length); - } else { - unreachable!("AsSlice called with non-array {}", array_typ); - } - } - _ => continue, - }; - } - - *self.dfg[block].instructions_mut() = instruction_ids; - self.dfg.replace_values_in_block_terminator(block, &values_to_replace); - } - - self.dfg.data_bus.replace_values(&values_to_replace); + let call_returns = context.dfg.instruction_results(instruction_id); + let original_slice_length = call_returns[0]; + let known_length = context.dfg.make_constant(length.into(), NumericType::length_type()); + context.replace_value(original_slice_length, known_length); + }); } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs b/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs index b79a7a7cbaa..c3d6f3ed0c3 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs @@ -10,8 +10,6 @@ //! is unnecessary as we already know the index. This pass looks for such array operations //! with constant indices and replaces their index with the appropriate offset. -use fxhash::FxHashMap as HashMap; - use crate::{ brillig::brillig_ir::BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, ssa::{ @@ -39,46 +37,34 @@ impl Ssa { impl Function { pub(super) fn brillig_array_gets(&mut self) { - let reachable_blocks = self.reachable_blocks(); - - let mut instructions_to_update = HashMap::default(); - for block_id in reachable_blocks.into_iter() { - for instruction_id in self.dfg[block_id].instructions() { - if let Instruction::ArrayGet { array, index } = self.dfg[*instruction_id] { - if self.dfg.is_constant(index) { - instructions_to_update.insert( - *instruction_id, - (Instruction::ArrayGet { array, index }, block_id), - ); - } - } + self.simple_reachable_blocks_optimization(|context| { + let instruction = context.instruction(); + let Instruction::ArrayGet { array, index } = instruction else { + return; + }; + + let array = *array; + let index = *index; + if !context.dfg.is_constant(index) { + return; } - } - for (instruction_id, _) in instructions_to_update { - let new_instruction = match self.dfg[instruction_id] { - Instruction::ArrayGet { array, index } => { - let index_constant = - self.dfg.get_numeric_constant(index).expect("ICE: Expected constant index"); - let offset = if matches!(self.dfg.type_of_value(array), Type::Array(..)) { - // Brillig arrays are [RC, ...items] - 1u128 - } else { - // Brillig vectors are [RC, Size, Capacity, ...items] - 3u128 - }; - let index = self.dfg.make_constant( - index_constant + offset.into(), - NumericType::unsigned(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), - ); - Instruction::ArrayGet { array, index } - } - _ => { - continue; - } + let index_constant = + context.dfg.get_numeric_constant(index).expect("ICE: Expected constant index"); + let offset = if matches!(context.dfg.type_of_value(array), Type::Array(..)) { + // Brillig arrays are [RC, ...items] + 1u128 + } else { + // Brillig vectors are [RC, Size, Capacity, ...items] + 3u128 }; - self.dfg[instruction_id] = new_instruction; - } + let index = context.dfg.make_constant( + index_constant + offset.into(), + NumericType::unsigned(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE), + ); + let new_instruction = Instruction::ArrayGet { array, index }; + context.replace_current_instruction_with(new_instruction); + }); } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/check_u128_mul_overflow.rs b/compiler/noirc_evaluator/src/ssa/opt/check_u128_mul_overflow.rs index 067f1e7a353..4cbab001188 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/check_u128_mul_overflow.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/check_u128_mul_overflow.rs @@ -4,7 +4,6 @@ use crate::ssa::{ ir::{ basic_block::BasicBlockId, call_stack::CallStackId, - dfg::DataFlowGraph, function::Function, instruction::{Binary, BinaryOp, ConstrainError, Instruction}, types::NumericType, @@ -13,6 +12,8 @@ use crate::ssa::{ ssa_gen::Ssa, }; +use super::simple_optimization::SimpleOptimizationContext; + impl Ssa { /// An SSA pass that checks that multiplying two u128 doesn't overflow because /// both operands are greater or equal than 2^64. @@ -34,30 +35,29 @@ impl Function { return; } - for block in self.reachable_blocks() { - let instructions = self.dfg[block].take_instructions(); - - for instruction in instructions { - self.dfg[block].insert_instruction(instruction); - - let Instruction::Binary(Binary { - lhs, - rhs, - operator: BinaryOp::Mul { unchecked: false }, - }) = &self.dfg[instruction] - else { - continue; - }; - - let binary_type = self.dfg.type_of_value(*lhs).unwrap_numeric(); - let NumericType::Unsigned { bit_size: 128 } = binary_type else { - continue; - }; - - let call_stack = self.dfg.get_instruction_call_stack_id(instruction); - check_u128_mul_overflow(*lhs, *rhs, block, &mut self.dfg, call_stack); - } - } + self.simple_reachable_blocks_optimization(|context| { + context.insert_current_instruction(); + + let block_id = context.block_id; + let instruction_id = context.instruction_id; + let instruction = context.instruction(); + let Instruction::Binary(Binary { + lhs, + rhs, + operator: BinaryOp::Mul { unchecked: false }, + }) = instruction + else { + return; + }; + + let binary_type = context.dfg.type_of_value(*lhs).unwrap_numeric(); + let NumericType::Unsigned { bit_size: 128 } = binary_type else { + return; + }; + + let call_stack = context.dfg.get_instruction_call_stack_id(instruction_id); + check_u128_mul_overflow(*lhs, *rhs, block_id, context, call_stack); + }); } } @@ -65,9 +65,10 @@ fn check_u128_mul_overflow( lhs: ValueId, rhs: ValueId, block: BasicBlockId, - dfg: &mut DataFlowGraph, + context: &mut SimpleOptimizationContext<'_, '_>, call_stack: CallStackId, ) { + let dfg = &mut context.dfg; let lhs_value = dfg.get_numeric_constant(lhs); let rhs_value = dfg.get_numeric_constant(rhs); diff --git a/compiler/noirc_evaluator/src/ssa/opt/make_constrain_not_equal.rs b/compiler/noirc_evaluator/src/ssa/opt/make_constrain_not_equal.rs index 95545fbd0f3..aab02cef2c0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/make_constrain_not_equal.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/make_constrain_not_equal.rs @@ -30,41 +30,56 @@ impl Function { return; } - for block in self.reachable_blocks() { - let instructions = self.dfg[block].instructions().to_vec(); + self.simple_reachable_blocks_optimization(|context| { + let instruction = context.instruction(); - for instruction in instructions { - let constrain_ne: Instruction = match &self.dfg[instruction] { - Instruction::Constrain(lhs, rhs, msg) => { - if self - .dfg - .get_numeric_constant(*rhs) - .is_some_and(|constant| constant.is_zero()) - { - if let Value::Instruction { instruction, .. } = &self.dfg[*lhs] { - if let Instruction::Binary(Binary { - lhs, - rhs, - operator: BinaryOp::Eq, - .. - }) = self.dfg[*instruction] - { - Instruction::ConstrainNotEqual(lhs, rhs, msg.clone()) - } else { - continue; - } - } else { - continue; - } - } else { - continue; - } - } - _ => continue, - }; + let Instruction::Constrain(lhs, rhs, msg) = instruction else { + return; + }; - self.dfg[instruction] = constrain_ne; + if !context.dfg.get_numeric_constant(*rhs).is_some_and(|constant| constant.is_zero()) { + return; } + + let Value::Instruction { instruction, .. } = &context.dfg[*lhs] else { + return; + }; + + let Instruction::Binary(Binary { lhs, rhs, operator: BinaryOp::Eq, .. }) = + context.dfg[*instruction] + else { + return; + }; + + let new_instruction = Instruction::ConstrainNotEqual(lhs, rhs, msg.clone()); + context.replace_current_instruction_with(new_instruction); + }); + } +} + +#[cfg(test)] +mod tests { + use crate::{assert_ssa_snapshot, ssa::ssa_gen::Ssa}; + + #[test] + fn test_make_constrain_not_equals() { + let src = " + acir(inline) fn main f1 { + b0(v0: Field, v1: Field): + v2 = eq v0, v1 + constrain v2 == u1 0 + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.make_constrain_not_equal_instructions(); + assert_ssa_snapshot!(ssa, @r" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = eq v0, v1 + constrain v0 != v1 + return } + "); } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/mod.rs index f00724b82be..e91b2da134b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -29,6 +29,7 @@ mod remove_enable_side_effects; mod remove_if_else; mod remove_truncate_after_range_check; mod remove_unreachable; +mod simple_optimization; mod simplify_cfg; mod unrolling; diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 1d67a16f526..6abb6267335 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -4,17 +4,18 @@ use acvm::{FieldElement, acir::AcirField}; use crate::ssa::{ ir::{ - basic_block::BasicBlockId, call_stack::CallStackId, dfg::InsertInstructionResult, function::Function, - instruction::{Binary, BinaryOp, Endian, Instruction, InstructionId, Intrinsic}, + instruction::{Binary, BinaryOp, Endian, Instruction, Intrinsic}, types::{NumericType, Type}, - value::{ValueId, ValueMapping}, + value::ValueId, }, ssa_gen::Ssa, }; +use super::simple_optimization::SimpleOptimizationContext; + impl Ssa { /// Performs constant folding on each instruction. /// @@ -37,70 +38,56 @@ impl Function { } let block = self.entry_block(); - let mut context = Context { - function: self, - new_instructions: Vec::new(), - block, - call_stack: CallStackId::root(), - }; - context.remove_bit_shifts(); - } -} + // Make sure this optimization runs when there's only one block + assert_eq!(self.dfg[block].successors().count(), 0); -struct Context<'f> { - function: &'f mut Function, - new_instructions: Vec, - - block: BasicBlockId, - call_stack: CallStackId, -} + self.simple_reachable_blocks_optimization(|context| { + let instruction_id = context.instruction_id; + let instruction = context.instruction(); -impl Context<'_> { - fn remove_bit_shifts(&mut self) { - let mut values_to_replace = ValueMapping::default(); - let instructions = self.function.dfg[self.block].take_instructions(); + let Instruction::Binary(Binary { lhs, rhs, operator }) = instruction else { + return; + }; - for instruction_id in instructions { - if !values_to_replace.is_empty() { - let instruction = &mut self.function.dfg[instruction_id]; - instruction.replace_values(&values_to_replace); + if !matches!(operator, BinaryOp::Shl | BinaryOp::Shr) { + return; } - match self.function.dfg[instruction_id] { - Instruction::Binary(Binary { lhs, rhs, operator }) - if matches!(operator, BinaryOp::Shl | BinaryOp::Shr) => - { - self.call_stack = - self.function.dfg.get_instruction_call_stack_id(instruction_id); - let old_result = - *self.function.dfg.instruction_results(instruction_id).first().unwrap(); - - let bit_size = match self.function.dfg.type_of_value(lhs) { - Type::Numeric(NumericType::Signed { bit_size }) - | Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size, - _ => unreachable!("ICE: right-shift attempted on non-integer"), - }; - let new_result = if operator == BinaryOp::Shl { - self.insert_wrapping_shift_left(lhs, rhs, bit_size) - } else { - self.insert_shift_right(lhs, rhs, bit_size) - }; - - values_to_replace.insert(old_result, new_result); - } - _ => { - self.new_instructions.push(instruction_id); - } + let lhs = *lhs; + let rhs = *rhs; + let operator = *operator; + + context.remove_current_instruction(); + + let call_stack = context.dfg.get_instruction_call_stack_id(instruction_id); + let old_result = *context.dfg.instruction_results(instruction_id).first().unwrap(); + + let bit_size = match context.dfg.type_of_value(lhs) { + Type::Numeric(NumericType::Signed { bit_size }) + | Type::Numeric(NumericType::Unsigned { bit_size }) => bit_size, + _ => unreachable!("ICE: right-shift attempted on non-integer"), + }; + + let new_result = if operator == BinaryOp::Shl { + let mut context = Context { context, call_stack }; + context.insert_wrapping_shift_left(lhs, rhs, bit_size) + } else { + let mut context = Context { context, call_stack }; + context.insert_shift_right(lhs, rhs, bit_size) }; - } - *self.function.dfg[self.block].instructions_mut() = - std::mem::take(&mut self.new_instructions); - self.function.dfg.replace_values_in_block_terminator(self.block, &values_to_replace); - self.function.dfg.data_bus.replace_values(&values_to_replace); + context.replace_value(old_result, new_result); + }); } +} + +struct Context<'m, 'dfg, 'mapping> { + context: &'m mut SimpleOptimizationContext<'dfg, 'mapping>, + call_stack: CallStackId, +} +impl Context<'_, '_, '_> { /// Insert ssa instructions which computes lhs << rhs by doing lhs*2^rhs /// and truncate the result to bit_size pub(crate) fn insert_wrapping_shift_left( @@ -110,8 +97,8 @@ impl Context<'_> { bit_size: u32, ) -> ValueId { let base = self.field_constant(FieldElement::from(2_u128)); - let typ = self.function.dfg.type_of_value(lhs).unwrap_numeric(); - let (max_bit, pow) = if let Some(rhs_constant) = self.function.dfg.get_numeric_constant(rhs) + let typ = self.context.dfg.type_of_value(lhs).unwrap_numeric(); + let (max_bit, pow) = if let Some(rhs_constant) = self.context.dfg.get_numeric_constant(rhs) { // Happy case is that we know precisely by how many bits the integer will // increase: lhs_bit_size + rhs @@ -127,7 +114,7 @@ impl Context<'_> { } let pow = self.numeric_constant(FieldElement::from(rhs_bit_size_pow_2), typ); - let max_lhs_bits = self.function.dfg.get_value_max_num_bits(lhs); + let max_lhs_bits = self.context.dfg.get_value_max_num_bits(lhs); let max_bit_size = max_lhs_bits + bit_shift_size; // There is no point trying to truncate to more than the Field size. // A higher `max_lhs_bits` input can come from trying to left-shift a Field. @@ -172,7 +159,7 @@ impl Context<'_> { rhs: ValueId, bit_size: u32, ) -> ValueId { - let lhs_typ = self.function.dfg.type_of_value(lhs).unwrap_numeric(); + let lhs_typ = self.context.dfg.type_of_value(lhs).unwrap_numeric(); let base = self.field_constant(FieldElement::from(2_u128)); let pow = self.pow(base, rhs); let pow = self.pow_or_max_for_bit_size(pow, rhs, bit_size, lhs_typ); @@ -239,7 +226,7 @@ impl Context<'_> { // pow = add pow_when_is_less_than_bit_size, pow_when_is_not_less_than_bit_size // // All operations here are unchecked because they work on field types. - let rhs_typ = self.function.dfg.type_of_value(rhs).unwrap_numeric(); + let rhs_typ = self.context.dfg.type_of_value(rhs).unwrap_numeric(); let bit_size = self.numeric_constant(bit_size as u128, rhs_typ); let rhs_is_less_than_bit_size = self.insert_binary(rhs, BinaryOp::Lt, bit_size); let rhs_is_not_less_than_bit_size = self.insert_not(rhs_is_less_than_bit_size); @@ -271,9 +258,9 @@ impl Context<'_> { /// r = (r_squared * lhs * b) + (1 - b) * r_squared; /// } fn pow(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId { - let typ = self.function.dfg.type_of_value(rhs); + let typ = self.context.dfg.type_of_value(rhs); if let Type::Numeric(NumericType::Unsigned { bit_size }) = typ { - let to_bits = self.function.dfg.import_intrinsic(Intrinsic::ToBits(Endian::Little)); + let to_bits = self.context.dfg.import_intrinsic(Intrinsic::ToBits(Endian::Little)); let result_types = vec![Type::Array(Arc::new(vec![Type::bool()]), bit_size)]; let rhs_bits = self.insert_call(to_bits, vec![rhs], result_types); @@ -300,7 +287,7 @@ impl Context<'_> { } pub(crate) fn field_constant(&mut self, constant: FieldElement) -> ValueId { - self.function.dfg.make_constant(constant, NumericType::NativeField) + self.context.dfg.make_constant(constant, NumericType::NativeField) } /// Insert a numeric constant into the current function @@ -309,7 +296,7 @@ impl Context<'_> { value: impl Into, typ: NumericType, ) -> ValueId { - self.function.dfg.make_constant(value.into(), typ) + self.context.dfg.make_constant(value.into(), typ) } /// Insert a binary instruction at the end of the current block. @@ -375,18 +362,12 @@ impl Context<'_> { instruction: Instruction, ctrl_typevars: Option>, ) -> InsertInstructionResult { - let result = self.function.dfg.insert_instruction_and_results( + self.context.dfg.insert_instruction_and_results( instruction, - self.block, + self.context.block_id, ctrl_typevars, self.call_stack, - ); - - if let InsertInstructionResult::Results(instruction_id, _) = result { - self.new_instructions.push(instruction_id); - } - - result + ) } } 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 3dcc54360ad..0dfe436692e 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 @@ -9,13 +9,11 @@ //! - 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::{FieldElement, acir::AcirField}; use crate::ssa::{ ir::{ - basic_block::BasicBlockId, dfg::DataFlowGraph, function::{Function, RuntimeType}, instruction::{BinaryOp, Hint, Instruction, Intrinsic}, @@ -43,41 +41,25 @@ impl Function { return; } - let mut context = Context::default(); - context.block_queue.push(self.entry_block()); + // Make sure this optimization runs when there's only one block + let block = self.entry_block(); + assert_eq!(self.dfg[block].successors().count(), 0); - while let Some(block) = context.block_queue.pop() { - if context.visited_blocks.contains(&block) { - continue; - } - - context.visited_blocks.insert(block); - context.remove_enable_side_effects_in_block(self, block); - } - } -} - -#[derive(Default)] -struct Context { - visited_blocks: HashSet, - block_queue: Vec, -} + let one = self.dfg.make_constant(FieldElement::one(), NumericType::bool()); + let mut active_condition = one; + let mut last_side_effects_enabled_instruction = None; -impl Context { - fn remove_enable_side_effects_in_block( - &mut self, - function: &mut Function, - block: BasicBlockId, - ) { - let instructions = function.dfg[block].take_instructions(); + let mut previous_block = None; - let one = FieldElement::one(); - let mut active_condition = function.dfg.make_constant(one, NumericType::bool()); - let mut last_side_effects_enabled_instruction = None; + self.simple_reachable_blocks_optimization(|context| { + if Some(context.block_id) != previous_block { + active_condition = one; + last_side_effects_enabled_instruction = None; + } + previous_block = Some(context.block_id); - let mut new_instructions = Vec::with_capacity(instructions.len()); - for instruction_id in instructions { - let instruction = &function.dfg[instruction_id]; + let instruction_id = context.instruction_id; + let instruction = context.instruction(); // If we run into another `Instruction::EnableSideEffectsIf` before encountering any // instructions with side effects then we can drop the instruction we're holding and @@ -85,202 +67,183 @@ impl Context { 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; + context.remove_current_instruction(); + return; } // If we're seeing an `enable_side_effects u1 1` then we want to insert it immediately. // This is because we want to maximize the effect it will have. - let condition_is_one = function + let condition_is_one = context .dfg .get_numeric_constant(*condition) .is_some_and(|condition| condition.is_one()); if condition_is_one { - new_instructions.push(instruction_id); last_side_effects_enabled_instruction = None; active_condition = *condition; - continue; + return; } last_side_effects_enabled_instruction = Some(instruction_id); active_condition = *condition; - continue; + context.remove_current_instruction(); + return; } // If we hit an instruction which is affected by the side effects var then we must insert the // `Instruction::EnableSideEffectsIf` before we insert this new instruction. - if Self::responds_to_side_effects_var(&function.dfg, instruction) { + if responds_to_side_effects_var(context.dfg, instruction) { if let Some(enable_side_effects_instruction_id) = last_side_effects_enabled_instruction.take() { - new_instructions.push(enable_side_effects_instruction_id); + context.insert_instruction(enable_side_effects_instruction_id); } } - new_instructions.push(instruction_id); - } - - *function.dfg[block].instructions_mut() = new_instructions; - - self.block_queue.extend(function.dfg[block].successors()); + }); } +} - fn responds_to_side_effects_var(dfg: &DataFlowGraph, instruction: &Instruction) -> bool { - use Instruction::*; - match instruction { - Binary(binary) => match binary.operator { - BinaryOp::Add { .. } | BinaryOp::Sub { .. } | BinaryOp::Mul { .. } => { - dfg.type_of_value(binary.lhs).is_unsigned() - } - BinaryOp::Div | BinaryOp::Mod => { - if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) { - rhs == FieldElement::zero() - } else { - true - } +fn responds_to_side_effects_var(dfg: &DataFlowGraph, instruction: &Instruction) -> bool { + use Instruction::*; + match instruction { + Binary(binary) => match binary.operator { + BinaryOp::Add { .. } | BinaryOp::Sub { .. } | BinaryOp::Mul { .. } => { + dfg.type_of_value(binary.lhs).is_unsigned() + } + BinaryOp::Div | BinaryOp::Mod => { + if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) { + rhs == FieldElement::zero() + } else { + true } - _ => false, - }, - - Cast(_, _) - | Not(_) - | Truncate { .. } - | Constrain(..) - | ConstrainNotEqual(..) - | RangeCheck { .. } - | IfElse { .. } - | IncrementRc { .. } - | DecrementRc { .. } - | Noop - | MakeArray { .. } => false, - - EnableSideEffectsIf { .. } - | ArrayGet { .. } - | ArraySet { .. } - | Allocate - | Store { .. } - | Load { .. } => true, - - // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is. - Call { func, .. } => match dfg[*func] { - Value::Intrinsic(intrinsic) => match intrinsic { - Intrinsic::SlicePushBack - | Intrinsic::SlicePushFront - | Intrinsic::SlicePopBack - | Intrinsic::SlicePopFront - | Intrinsic::SliceInsert - | Intrinsic::SliceRemove => true, + } + _ => false, + }, - Intrinsic::ArrayLen - | Intrinsic::ArrayAsStrUnchecked - | Intrinsic::AssertConstant - | Intrinsic::StaticAssert - | Intrinsic::ApplyRangeConstraint - | Intrinsic::StrAsBytes - | Intrinsic::ToBits(_) - | Intrinsic::ToRadix(_) - | Intrinsic::BlackBox(_) - | Intrinsic::Hint(Hint::BlackBox) - | Intrinsic::AsSlice - | Intrinsic::AsWitness - | Intrinsic::IsUnconstrained - | Intrinsic::DerivePedersenGenerators - | Intrinsic::ArrayRefCount - | Intrinsic::SliceRefCount - | Intrinsic::FieldLessThan => false, - }, + Cast(_, _) + | Not(_) + | Truncate { .. } + | Constrain(..) + | ConstrainNotEqual(..) + | RangeCheck { .. } + | IfElse { .. } + | IncrementRc { .. } + | DecrementRc { .. } + | Noop + | MakeArray { .. } => false, + + EnableSideEffectsIf { .. } + | ArrayGet { .. } + | ArraySet { .. } + | Allocate + | Store { .. } + | Load { .. } => true, + + // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is. + Call { func, .. } => match dfg[*func] { + Value::Intrinsic(intrinsic) => match intrinsic { + Intrinsic::SlicePushBack + | Intrinsic::SlicePushFront + | Intrinsic::SlicePopBack + | Intrinsic::SlicePopFront + | Intrinsic::SliceInsert + | Intrinsic::SliceRemove => true, + + Intrinsic::ArrayLen + | Intrinsic::ArrayAsStrUnchecked + | Intrinsic::AssertConstant + | Intrinsic::StaticAssert + | Intrinsic::ApplyRangeConstraint + | Intrinsic::StrAsBytes + | Intrinsic::ToBits(_) + | Intrinsic::ToRadix(_) + | Intrinsic::BlackBox(_) + | Intrinsic::Hint(Hint::BlackBox) + | Intrinsic::AsSlice + | Intrinsic::AsWitness + | Intrinsic::IsUnconstrained + | Intrinsic::DerivePedersenGenerators + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount + | Intrinsic::FieldLessThan => false, + }, - // We must assume that functions contain a side effect as we cannot inspect more deeply. - Value::Function(_) => true, + // We must assume that functions contain a side effect as we cannot inspect more deeply. + Value::Function(_) => true, - _ => false, - }, - } + _ => false, + }, } } #[cfg(test)] mod test { - - use crate::ssa::{ - function_builder::FunctionBuilder, - ir::{ - instruction::{BinaryOp, Instruction}, - map::Id, - types::{NumericType, Type}, - }, - }; + use crate::{assert_ssa_snapshot, ssa::ssa_gen::Ssa}; #[test] fn remove_chains_of_same_condition() { - // acir(inline) fn main f0 { - // b0(v0: Field): - // enable_side_effects u1 1 - // v4 = mul v0, Field 2 - // enable_side_effects u1 1 - // v5 = mul v0, Field 2 - // enable_side_effects u1 1 - // v6 = mul v0, Field 2 - // enable_side_effects u1 1 - // v7 = mul v0, Field 2 - // enable_side_effects u1 1 - // (no terminator instruction) - // } - // - // After constructing this IR, we run constant folding which should replace the second cast - // with a reference to the results to the first. This then allows us to optimize away - // the constrain instruction as both inputs are known to be equal. - // - // The first cast instruction is retained and will be removed in the dead instruction elimination pass. - let main_id = Id::test_new(0); - - // Compiling main - let mut builder = FunctionBuilder::new("main".into(), main_id); - let v0 = builder.add_parameter(Type::field()); - - let two = builder.field_constant(2u128); - - let one = builder.numeric_constant(1u128, NumericType::bool()); - - builder.insert_enable_side_effects_if(one); - builder.insert_binary(v0, BinaryOp::Mul { unchecked: false }, two); - builder.insert_enable_side_effects_if(one); - builder.insert_binary(v0, BinaryOp::Mul { unchecked: false }, two); - builder.insert_enable_side_effects_if(one); - builder.insert_binary(v0, BinaryOp::Mul { unchecked: false }, two); - builder.insert_enable_side_effects_if(one); - builder.insert_binary(v0, BinaryOp::Mul { unchecked: false }, two); - builder.insert_enable_side_effects_if(one); - - let ssa = builder.finish(); - - println!("{ssa}"); - - let main = ssa.main(); - let instructions = main.dfg[main.entry_block()].instructions(); - assert_eq!(instructions.len(), 9); + let src = " + acir(inline) fn main f0 { + b0(v0: Field): + enable_side_effects u1 1 + v3 = mul v0, Field 2 + enable_side_effects u1 1 + v4 = mul v0, Field 2 + enable_side_effects u1 1 + v5 = mul v0, Field 2 + enable_side_effects u1 1 + v6 = mul v0, Field 2 + enable_side_effects u1 1 + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); - // Expected output: - // - // acir(inline) fn main f0 { - // b0(v0: Field): - // v3 = mul v0, Field 2 - // v4 = mul v0, Field 2 - // v5 = mul v0, Field 2 - // v6 = mul v0, Field 2 - // (no terminator instruction) - // } let ssa = ssa.remove_enable_side_effects(); + assert_ssa_snapshot!(ssa, @r" + acir(inline) fn main f0 { + b0(v0: Field): + v2 = mul v0, Field 2 + v3 = mul v0, Field 2 + v4 = mul v0, Field 2 + v5 = mul v0, Field 2 + return + } + " + ); + } - println!("{ssa}"); - - let main = ssa.main(); - let instructions = main.dfg[main.entry_block()].instructions(); + #[test] + fn keeps_enable_side_effects_for_instructions_that_have_side_effects() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + enable_side_effects v0 + v3 = allocate -> &mut Field + enable_side_effects v0 + v4 = allocate -> &mut Field + enable_side_effects v1 + v5 = allocate -> &mut Field + enable_side_effects u1 1 + v6 = allocate -> &mut Field + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); - assert_eq!(instructions.len(), 4); - for instruction in instructions.iter().take(4) { - assert_eq!( - &main.dfg[*instruction], - &Instruction::binary(BinaryOp::Mul { unchecked: false }, v0, two) - ); + let ssa = ssa.remove_enable_side_effects(); + assert_ssa_snapshot!(ssa, @r" + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + enable_side_effects v0 + v2 = allocate -> &mut Field + v3 = allocate -> &mut Field + enable_side_effects v1 + v4 = allocate -> &mut Field + enable_side_effects u1 1 + v6 = allocate -> &mut Field + return } + " + ); } } 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 9c79e07b33a..d349fe9299c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -6,7 +6,7 @@ use fxhash::FxHashMap as HashMap; use crate::ssa::ir::function::RuntimeType; use crate::ssa::ir::instruction::Hint; use crate::ssa::ir::types::NumericType; -use crate::ssa::ir::value::{ValueId, ValueMapping}; +use crate::ssa::ir::value::ValueId; use crate::ssa::{ Ssa, ir::{ @@ -59,31 +59,30 @@ struct Context { impl Context { fn remove_if_else(&mut self, function: &mut Function) { let block = function.entry_block(); - let instructions = function.dfg[block].take_instructions(); + + // Make sure this optimization runs when there's only one block + assert_eq!(function.dfg[block].successors().count(), 0); + let one = FieldElement::one(); let mut current_conditional = function.dfg.make_constant(one, NumericType::bool()); - let mut values_to_replace = ValueMapping::default(); - for instruction in instructions { - // Before we process instructions, replace any values we previously determined we need to replace - if !values_to_replace.is_empty() { - let instruction = &mut function.dfg[instruction]; - instruction.replace_values(&values_to_replace); - } + function.simple_reachable_blocks_optimization(|context| { + let instruction_id = context.instruction_id; + let instruction = context.instruction(); - match &function.dfg[instruction] { + match instruction { Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { let then_condition = *then_condition; let else_condition = *else_condition; let then_value = *then_value; let else_value = *else_value; - let typ = function.dfg.type_of_value(then_value); + let typ = context.dfg.type_of_value(then_value); assert!(!matches!(typ, Type::Numeric(_))); - let call_stack = function.dfg.get_instruction_call_stack_id(instruction); + let call_stack = context.dfg.get_instruction_call_stack_id(instruction_id); let mut value_merger = ValueMerger::new( - &mut function.dfg, + context.dfg, block, &mut self.slice_sizes, &mut self.array_set_conditionals, @@ -98,8 +97,8 @@ impl Context { else_value, ); - let _typ = function.dfg.type_of_value(value); - let results = function.dfg.instruction_results(instruction); + let _typ = context.dfg.type_of_value(value); + let results = context.dfg.instruction_results(instruction_id); let result = results[0]; // let result = match typ { // Type::Array(..) => results[0], @@ -107,54 +106,47 @@ impl Context { // other => unreachable!("IfElse instructions should only have arrays or slices at this point. Found {other:?}"), // }; - values_to_replace.insert(result, value); + context.remove_current_instruction(); + context.replace_value(result, value); self.array_set_conditionals.insert(value, current_conditional); } Instruction::Call { func, arguments } => { - if let Value::Intrinsic(intrinsic) = function.dfg[*func] { - let results = function.dfg.instruction_results(instruction); + if let Value::Intrinsic(intrinsic) = context.dfg[*func] { + let results = context.dfg.instruction_results(instruction_id); - match slice_capacity_change(&function.dfg, intrinsic, arguments, results) { + match slice_capacity_change(context.dfg, intrinsic, arguments, results) { SizeChange::None => (), SizeChange::SetTo(value, new_capacity) => { self.slice_sizes.insert(value, new_capacity); } SizeChange::Inc { old, new } => { - let old_capacity = self.get_or_find_capacity(&function.dfg, old); + let old_capacity = self.get_or_find_capacity(context.dfg, old); self.slice_sizes.insert(new, old_capacity + 1); } SizeChange::Dec { old, new } => { - let old_capacity = self.get_or_find_capacity(&function.dfg, old); + let old_capacity = self.get_or_find_capacity(context.dfg, old); // We use a saturating sub here as calling `pop_front` or `pop_back` on a zero-length slice // would otherwise underflow. self.slice_sizes.insert(new, old_capacity.saturating_sub(1)); } } } - function.dfg[block].instructions_mut().push(instruction); } Instruction::ArraySet { array, .. } => { - let results = function.dfg.instruction_results(instruction); + let results = context.dfg.instruction_results(instruction_id); let result = if results.len() == 2 { results[1] } else { results[0] }; self.array_set_conditionals.insert(result, current_conditional); - let old_capacity = self.get_or_find_capacity(&function.dfg, *array); + let old_capacity = self.get_or_find_capacity(context.dfg, *array); self.slice_sizes.insert(result, old_capacity); - function.dfg[block].instructions_mut().push(instruction); } Instruction::EnableSideEffectsIf { condition } => { current_conditional = *condition; - function.dfg[block].instructions_mut().push(instruction); - } - _ => { - function.dfg[block].instructions_mut().push(instruction); } + _ => (), } - } - - function.dfg.replace_values_in_block_terminator(block, &values_to_replace); - function.dfg.data_bus.replace_values(&values_to_replace); + }); } fn get_or_find_capacity(&mut self, dfg: &DataFlowGraph, value: ValueId) -> u32 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs index 0d6ecb85e42..0a0b709a494 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_truncate_after_range_check.rs @@ -1,7 +1,5 @@ use fxhash::FxHashMap as HashMap; -use fxhash::FxHashSet as HashSet; -use crate::ssa::ir::value::ValueMapping; use crate::ssa::{ ir::{function::Function, instruction::Instruction, value::ValueId}, ssa_gen::Ssa, @@ -22,64 +20,45 @@ impl Ssa { impl Function { pub(crate) fn remove_truncate_after_range_check(&mut self) { - let mut values_to_replace = ValueMapping::default(); // Keeps the minimum bit size a value was range-checked against let mut range_checks: HashMap = HashMap::default(); - let blocks = self.reachable_blocks(); - for block in &blocks { - let block = *block; - let mut instructions_to_remove = HashSet::default(); - let mut instruction_ids = self.dfg[block].take_instructions(); + self.simple_reachable_blocks_optimization(|context| { + let instruction_id = context.instruction_id; + let instruction = context.instruction(); - for instruction_id in &instruction_ids { - if !values_to_replace.is_empty() { - let instruction = &mut self.dfg[*instruction_id]; - instruction.replace_values(&values_to_replace); - } - - let instruction = &self.dfg[*instruction_id]; - match instruction { - // If this is a range_check instruction, associate the max bit size with the value - Instruction::RangeCheck { value, max_bit_size, .. } => { - range_checks - .entry(*value) - .and_modify(|current_max| { - if *max_bit_size < *current_max { - *current_max = *max_bit_size; - } - }) - .or_insert(*max_bit_size); - } - // If this is a truncate instruction, check if there's a range check for that same value - Instruction::Truncate { value, bit_size, .. } => { - if let Some(range_check_bit_size) = range_checks.get(value) { - if range_check_bit_size <= bit_size { - // We need to replace the truncated value with the original one. That is, in: - // - // range_check v0 to 32 bits - // v1 = truncate v0 to 32 bits, max_bit_size: 254 - // - // we need to remove the `truncate` and all references to `v1` should now be `v0`. - let result = - self.dfg.instruction_results(*instruction_id).first().unwrap(); - values_to_replace.insert(*result, *value); - instructions_to_remove.insert(*instruction_id); + match instruction { + // If this is a range_check instruction, associate the max bit size with the value + Instruction::RangeCheck { value, max_bit_size, .. } => { + range_checks + .entry(*value) + .and_modify(|current_max| { + if *max_bit_size < *current_max { + *current_max = *max_bit_size; } + }) + .or_insert(*max_bit_size); + } + // If this is a truncate instruction, check if there's a range check for that same value + Instruction::Truncate { value, bit_size, .. } => { + if let Some(range_check_bit_size) = range_checks.get(value) { + if range_check_bit_size <= bit_size { + // We need to replace the truncated value with the original one. That is, in: + // + // range_check v0 to 32 bits + // v1 = truncate v0 to 32 bits, max_bit_size: 254 + // + // we need to remove the `truncate` and all references to `v1` should now be `v0`. + let result = + context.dfg.instruction_results(instruction_id).first().unwrap(); + context.replace_value(*result, *value); + context.remove_current_instruction(); } } - _ => (), } + _ => (), } - - if !instructions_to_remove.is_empty() { - instruction_ids.retain(|instruction| !instructions_to_remove.contains(instruction)); - } - *self.dfg[block].instructions_mut() = instruction_ids; - self.dfg.replace_values_in_block_terminator(block, &values_to_replace); - } - - self.dfg.data_bus.replace_values(&values_to_replace); + }); } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs b/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs new file mode 100644 index 00000000000..15877deab53 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/opt/simple_optimization.rs @@ -0,0 +1,103 @@ +use crate::ssa::ir::{ + basic_block::BasicBlockId, + dfg::DataFlowGraph, + function::Function, + instruction::{Instruction, InstructionId}, + value::{ValueId, ValueMapping}, +}; + +impl Function { + /// Performs a simple optimization according to the given callback. + /// + /// The function's [`Function::reachable_blocks`] are traversed in turn, and instructions in those blocks + /// are then traversed in turn. For each one, `f` will be called with a context. + /// + /// The current instruction will be inserted at the end of the callback given to `mutate` unless + /// `remove_current_instruction` or `insert_current_instruction` are called. + /// + /// `insert_current_instruction` is useful if you need to insert new instructions after the current + /// one, so this can be done before the callback ends. + /// + /// `replace_value` can be used to replace a value with another one. This substitution will be + /// performed in all subsequent instructions. + pub(crate) fn simple_reachable_blocks_optimization(&mut self, mut f: F) + where + F: FnMut(&mut SimpleOptimizationContext<'_, '_>), + { + let mut values_to_replace = ValueMapping::default(); + + for block_id in self.reachable_blocks() { + let instruction_ids = self.dfg[block_id].take_instructions(); + self.dfg[block_id].instructions_mut().reserve(instruction_ids.len()); + for instruction_id in &instruction_ids { + let instruction_id = *instruction_id; + + if !values_to_replace.is_empty() { + let instruction = &mut self.dfg[instruction_id]; + instruction.replace_values(&values_to_replace); + } + + let mut context = SimpleOptimizationContext { + block_id, + instruction_id, + dfg: &mut self.dfg, + values_to_replace: &mut values_to_replace, + insert_current_instruction_at_callback_end: true, + }; + f(&mut context); + + if context.insert_current_instruction_at_callback_end { + self.dfg[block_id].insert_instruction(instruction_id); + } + } + + self.dfg.replace_values_in_block_terminator(block_id, &values_to_replace); + } + + self.dfg.data_bus.replace_values(&values_to_replace); + } +} + +pub(crate) struct SimpleOptimizationContext<'dfg, 'mapping> { + #[allow(unused)] + pub(crate) block_id: BasicBlockId, + pub(crate) instruction_id: InstructionId, + pub(crate) dfg: &'dfg mut DataFlowGraph, + values_to_replace: &'mapping mut ValueMapping, + insert_current_instruction_at_callback_end: bool, +} + +impl SimpleOptimizationContext<'_, '_> { + /// Returns the current instruction being visited. + pub(crate) fn instruction(&self) -> &Instruction { + &self.dfg[self.instruction_id] + } + + /// Instructs this context to replace a value with another value. The value will be replaced + /// in all subsequent instructions. + pub(crate) fn replace_value(&mut self, from: ValueId, to: ValueId) { + self.values_to_replace.insert(from, to); + } + + /// Instructs this context to insert the current instruction right away, as opposed + /// to doing this at the end of `mutate`'s block (unless `remove_current_instruction is called`). + pub(crate) fn insert_current_instruction(&mut self) { + self.dfg[self.block_id].insert_instruction(self.instruction_id); + self.insert_current_instruction_at_callback_end = false; + } + + /// Instructs this context to remove the current instruction from its block. + pub(crate) fn remove_current_instruction(&mut self) { + self.insert_current_instruction_at_callback_end = false; + } + + /// Inserts an instruction in the current block right away. + pub(crate) fn insert_instruction(&mut self, instruction_id: InstructionId) { + self.dfg[self.block_id].insert_instruction(instruction_id); + } + + /// Replaces the current instruction with another one. + pub(crate) fn replace_current_instruction_with(&mut self, instruction: Instruction) { + self.dfg[self.instruction_id] = instruction; + } +}