diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f56dfaa7c65..081c1f3afdf 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -273,15 +273,7 @@ pub(crate) enum Instruction { /// 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, - else_condition: ValueId, - else_value: ValueId, - }, + IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId }, /// Creates a new array or slice. /// @@ -536,14 +528,11 @@ impl Instruction { assert_message: assert_message.clone(), } } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { - Instruction::IfElse { - then_condition: f(*then_condition), - then_value: f(*then_value), - else_condition: f(*else_condition), - else_value: f(*else_value), - } - } + Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse { + then_condition: f(*then_condition), + then_value: f(*then_value), + else_value: f(*else_value), + }, Instruction::MakeArray { elements, typ } => Instruction::MakeArray { elements: elements.iter().copied().map(f).collect(), typ: typ.clone(), @@ -602,10 +591,9 @@ impl Instruction { | Instruction::RangeCheck { value, .. } => { f(*value); } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { f(*then_condition); f(*then_value); - f(*else_condition); f(*else_value); } Instruction::MakeArray { elements, typ: _ } => { @@ -768,7 +756,7 @@ impl Instruction { None } } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let typ = dfg.type_of_value(*then_value); if let Some(constant) = dfg.get_numeric_constant(*then_condition) { @@ -787,13 +775,11 @@ impl Instruction { if matches!(&typ, Type::Numeric(_)) { let then_condition = *then_condition; - let else_condition = *else_condition; let result = ValueMerger::merge_numeric_values( dfg, block, then_condition, - else_condition, then_value, else_value, ); diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index e1e967b9a43..95447f941b0 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -443,12 +443,8 @@ fn simplify_slice_push_back( let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None, call_stack); - let new_slice = value_merger.merge_values( - len_not_equals_capacity, - len_equals_capacity, - set_last_slice_value, - new_slice, - ); + let new_slice = + value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice); SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]) } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index a34e9c132ec..1690d34b847 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -210,15 +210,11 @@ fn display_instruction_inner( Instruction::RangeCheck { value, max_bit_size, .. } => { writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,) } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = show(*then_condition); let then_value = show(*then_value); - let else_condition = show(*else_condition); let else_value = show(*else_value); - writeln!( - f, - "if {then_condition} then {then_value} else if {else_condition} then {else_value}" - ) + writeln!(f, "if {then_condition} then {then_value} else {else_value}") } Instruction::MakeArray { elements, typ } => { write!(f, "make_array [")?; diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 82f67a7c535..61a93aee58d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -519,7 +519,6 @@ impl<'f> Context<'f> { let instruction = Instruction::IfElse { then_condition: cond_context.then_branch.condition, then_value: then_arg, - else_condition: cond_context.else_branch.as_ref().unwrap().condition, else_value: else_arg, }; let call_stack = cond_context.call_stack.clone(); @@ -667,13 +666,10 @@ impl<'f> Context<'f> { ) .first(); - let not = Instruction::Not(condition); - let else_condition = self.insert_instruction(not, call_stack.clone()); - let instruction = Instruction::IfElse { then_condition: condition, then_value: value, - else_condition, + else_value: previous_value, }; @@ -907,13 +903,12 @@ mod test { b0(v0: u1, v1: &mut Field): enable_side_effects v0 v2 = load v1 -> Field - v3 = not v0 - v4 = cast v0 as Field - v6 = sub Field 5, v2 - v7 = mul v4, v6 - v8 = add v2, v7 - store v8 at v1 - v9 = not v0 + v3 = cast v0 as Field + v5 = sub Field 5, v2 + v6 = mul v3, v5 + v7 = add v2, v6 + store v7 at v1 + v8 = not v0 enable_side_effects u1 1 return } @@ -945,20 +940,19 @@ mod test { b0(v0: u1, v1: &mut Field): enable_side_effects v0 v2 = load v1 -> Field - v3 = not v0 - v4 = cast v0 as Field - v6 = sub Field 5, v2 - v7 = mul v4, v6 - v8 = add v2, v7 - store v8 at v1 - v9 = not v0 - enable_side_effects v9 - v10 = load v1 -> Field - v11 = cast v9 as Field - v13 = sub Field 6, v10 - v14 = mul v11, v13 - v15 = add v10, v14 - store v15 at v1 + v3 = cast v0 as Field + v5 = sub Field 5, v2 + v6 = mul v3, v5 + v7 = add v2, v6 + store v7 at v1 + v8 = not v0 + enable_side_effects v8 + v9 = load v1 -> Field + v10 = cast v8 as Field + v12 = sub Field 6, v9 + v13 = mul v10, v12 + v14 = add v9, v13 + store v14 at v1 enable_side_effects u1 1 return } @@ -1306,24 +1300,23 @@ mod test { v9 = add v7, Field 1 v10 = cast v9 as u8 v11 = load v6 -> u8 - v12 = not v5 - v13 = cast v4 as Field - v14 = cast v11 as Field - v15 = sub v9, v14 - v16 = mul v13, v15 - v17 = add v14, v16 - v18 = cast v17 as u8 - store v18 at v6 - v19 = not v5 - enable_side_effects v19 - v20 = load v6 -> u8 + v12 = cast v4 as Field + v13 = cast v11 as Field + v14 = sub v9, v13 + v15 = mul v12, v14 + v16 = add v13, v15 + v17 = cast v16 as u8 + store v17 at v6 + v18 = not v5 + enable_side_effects v18 + v19 = load v6 -> u8 + v20 = cast v18 as Field v21 = cast v19 as Field - v22 = cast v20 as Field - v24 = sub Field 0, v22 - v25 = mul v21, v24 - v26 = add v22, v25 - v27 = cast v26 as u8 - store v27 at v6 + v23 = sub Field 0, v21 + v24 = mul v20, v23 + v25 = add v21, v24 + v26 = cast v25 as u8 + store v26 at v6 enable_side_effects u1 1 constrain v5 == u1 1 return 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 3d45c43f587..8ea26d4e96d 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 @@ -45,7 +45,7 @@ impl<'a> ValueMerger<'a> { /// Merge two values a and b from separate basic blocks to a single value. /// If these two values are numeric, the result will be - /// `then_condition * then_value + else_condition * else_value`. + /// `then_condition * (then_value - else_value) + else_value`. /// Otherwise, if the values being merged are arrays, a new array will be made /// recursively from combining each element of both input arrays. /// @@ -54,7 +54,6 @@ impl<'a> ValueMerger<'a> { pub(crate) fn merge_values( &mut self, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -70,15 +69,14 @@ impl<'a> ValueMerger<'a> { self.dfg, self.block, then_condition, - else_condition, then_value, else_value, ), typ @ Type::Array(_, _) => { - self.merge_array_values(typ, then_condition, else_condition, then_value, else_value) + self.merge_array_values(typ, then_condition, then_value, else_value) } typ @ Type::Slice(_) => { - self.merge_slice_values(typ, then_condition, else_condition, then_value, else_value) + self.merge_slice_values(typ, then_condition, then_value, else_value) } Type::Reference(_) => panic!("Cannot return references from an if expression"), Type::Function => panic!("Cannot return functions from an if expression"), @@ -86,12 +84,11 @@ impl<'a> ValueMerger<'a> { } /// Merge two numeric values a and b from separate basic blocks to a single value. This - /// function would return the result of `if c { a } else { b }` as `c*a + (!c)*b`. + /// function would return the result of `if c { a } else { b }` as `c * (a-b) + b`. pub(crate) fn merge_numeric_values( dfg: &mut DataFlowGraph, block: BasicBlockId, then_condition: ValueId, - _else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -155,7 +152,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -170,7 +166,6 @@ impl<'a> ValueMerger<'a> { if let Some(result) = self.try_merge_only_changed_indices( then_condition, - else_condition, then_value, else_value, actual_length, @@ -200,12 +195,7 @@ impl<'a> ValueMerger<'a> { let then_element = get_element(then_value, typevars.clone()); let else_element = get_element(else_value, typevars); - merged.push_back(self.merge_values( - then_condition, - else_condition, - then_element, - else_element, - )); + merged.push_back(self.merge_values(then_condition, then_element, else_element)); } } @@ -218,7 +208,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value_id: ValueId, else_value_id: ValueId, ) -> ValueId { @@ -276,12 +265,7 @@ impl<'a> ValueMerger<'a> { let else_element = get_element(else_value_id, typevars, else_len * element_types.len()); - merged.push_back(self.merge_values( - then_condition, - else_condition, - then_element, - else_element, - )); + merged.push_back(self.merge_values(then_condition, then_element, else_element)); } } @@ -330,7 +314,6 @@ impl<'a> ValueMerger<'a> { fn try_merge_only_changed_indices( &mut self, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, array_length: usize, @@ -414,8 +397,7 @@ impl<'a> ValueMerger<'a> { let then_element = get_element(then_value, typevars.clone()); let else_element = get_element(else_value, typevars); - let value = - self.merge_values(then_condition, else_condition, then_element, else_element); + let value = self.merge_values(then_condition, then_element, else_element); array = self.insert_array_set(array, index, value, Some(condition)).first(); } 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 c387e0b6234..8076bc3cc99 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -66,10 +66,9 @@ impl Context { for instruction in instructions { match &function.dfg[instruction] { - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = *then_condition; let then_value = *then_value; - let else_condition = *else_condition; let else_value = *else_value; let typ = function.dfg.type_of_value(then_value); @@ -85,12 +84,7 @@ impl Context { call_stack, ); - let value = value_merger.merge_values( - then_condition, - else_condition, - then_value, - else_value, - ); + let value = value_merger.merge_values(then_condition, then_value, else_value); let _typ = function.dfg.type_of_value(value); let results = function.dfg.instruction_results(instruction);