From 54cc3c147764dfe11e2d734860d09722411f66d7 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 20 Jun 2023 13:36:09 -0500 Subject: [PATCH 1/3] Merge array values --- .../src/ssa_refactor/acir_gen/mod.rs | 3 +- .../src/ssa_refactor/ir/instruction.rs | 19 ++++--- .../src/ssa_refactor/opt/flatten_cfg.rs | 57 ++++++++++++++++++- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index b2c675c16a..ace4d054a2 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -483,7 +483,8 @@ impl Context { (Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => { assert_eq!( lhs_type, rhs_type, - "lhs and rhs types in a binary operation are always the same" + "lhs and rhs types in {:?} are not the same", + binary ); Type::Numeric(lhs_type) } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index bb238f9a82..d2699059ef 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -277,11 +277,12 @@ impl Instruction { if let (Some((array, _)), Some(index)) = (array, index) { let index = index.try_to_u64().expect("Expected array index to fit in u64") as usize; - assert!(index < array.len()); - SimplifiedTo(array[index]) - } else { - None + + if index < array.len() { + return SimplifiedTo(array[index]); + } } + None } Instruction::ArraySet { array, index, value } => { let array = dfg.get_array_constant(*array); @@ -290,11 +291,13 @@ impl Instruction { if let (Some((array, element_type)), Some(index)) = (array, index) { let index = index.try_to_u64().expect("Expected array index to fit in u64") as usize; - assert!(index < array.len()); - SimplifiedTo(dfg.make_array(array.update(index, *value), element_type)) - } else { - None + + if index < array.len() { + let new_array = dfg.make_array(array.update(index, *value), element_type); + return SimplifiedTo(new_array); + } } + None } Instruction::Truncate { value, bit_size, .. } => { if let Some((numeric_constant, typ)) = dfg.get_numeric_constant_with_type(*value) { diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 73f411a0ef..d6278d1570 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -396,14 +396,67 @@ impl<'f> Context<'f> { self.insert_instruction_with_typevars(enable_side_effects, None); } - /// Merge two 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`. + /// 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`. + /// Otherwise, if the values being merged are arrays, a new array will be made + /// recursively from combining each element of both input arrays. + /// + /// It is currently an error to call this function on reference or function values + /// as it is less clear how to merge these. fn merge_values( &mut self, then_condition: ValueId, else_condition: ValueId, then_value: ValueId, else_value: ValueId, + ) -> ValueId { + match self.inserter.function.dfg.type_of_value(then_value) { + Type::Numeric(_) => { + self.merge_numeric_values(then_condition, else_condition, then_value, else_value) + } + Type::Array(element_types, len) => { + let mut merged = im::Vector::new(); + + for i in 0..len { + for (element_index, element_type) in element_types.iter().enumerate() { + let index = ((i * element_types.len() + element_index) as u128).into(); + let index = self.inserter.function.dfg.make_constant(index, Type::field()); + + let typevars = Some(vec![element_type.clone()]); + + let mut get_element = |array, typevars| { + let get = Instruction::ArrayGet { array, index }; + self.insert_instruction_with_typevars(get, typevars).first() + }; + + 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, + )); + } + } + + self.inserter.function.dfg.make_array(merged, element_types.clone()) + } + Type::Reference => panic!("Cannot return references from an if expression"), + Type::Function => panic!("Cannot return functions from an if expression"), + } + } + + /// 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`. + fn merge_numeric_values( + &mut self, + then_condition: ValueId, + else_condition: ValueId, + then_value: ValueId, + else_value: ValueId, ) -> ValueId { let block = self.inserter.function.entry_block(); let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); From 04979a91a47f95801ec890925cff2302d3b13222 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 20 Jun 2023 13:36:44 -0500 Subject: [PATCH 2/3] Remove redundant clone --- crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index d6278d1570..ee7aecff7e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -442,7 +442,7 @@ impl<'f> Context<'f> { } } - self.inserter.function.dfg.make_array(merged, element_types.clone()) + self.inserter.function.dfg.make_array(merged, element_types) } Type::Reference => panic!("Cannot return references from an if expression"), Type::Function => panic!("Cannot return functions from an if expression"), From f51409a43b0eca3d11d7c3d85fc0f625857451a6 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Tue, 20 Jun 2023 13:43:40 -0500 Subject: [PATCH 3/3] Fix merge values of different types bug --- .../src/ssa_refactor/opt/flatten_cfg.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index ee7aecff7e..c9dac0f55d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -459,6 +459,17 @@ impl<'f> Context<'f> { else_value: ValueId, ) -> ValueId { let block = self.inserter.function.entry_block(); + let then_type = self.inserter.function.dfg.type_of_value(then_value); + let else_type = self.inserter.function.dfg.type_of_value(else_value); + assert_eq!( + then_type, else_type, + "Expected values merged to be of the same type but found {then_type} and {else_type}" + ); + + // We must cast the bool conditions to the actual numeric type used by each value. + let then_condition = self.insert_instruction(Instruction::Cast(then_condition, then_type)); + let else_condition = self.insert_instruction(Instruction::Cast(else_condition, else_type)); + let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); let then_value = self.inserter.function.dfg.insert_instruction_and_results(mul, block, None).first();