From 79044af10ed2a4103eb408355c6411a9fbe170c7 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 20 Nov 2024 19:00:34 +0000 Subject: [PATCH 01/10] chore: pull value merger code from sync --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 11 +-- .../src/ssa/ir/instruction/call.rs | 1 - .../noirc_evaluator/src/ssa/ir/printer.rs | 5 +- .../src/ssa/opt/flatten_cfg.rs | 7 -- .../src/ssa/opt/flatten_cfg/value_merger.rs | 67 +++++++++---------- .../src/ssa/opt/remove_if_else.rs | 4 +- 6 files changed, 36 insertions(+), 59 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 2b40bccca7b6..f4c8286158d0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -275,7 +275,6 @@ pub(crate) enum Instruction { IfElse { then_condition: ValueId, then_value: ValueId, - else_condition: ValueId, else_value: ValueId, }, } @@ -511,11 +510,10 @@ impl Instruction { assert_message: assert_message.clone(), } } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { Instruction::IfElse { then_condition: f(*then_condition), then_value: f(*then_value), - else_condition: f(*else_condition), else_value: f(*else_value), } } @@ -573,10 +571,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); } } @@ -726,7 +723,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) { @@ -745,13 +742,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/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 9dbd2c569936..c469f5491105 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -442,7 +442,6 @@ fn simplify_slice_push_back( let new_slice = value_merger.merge_values( len_not_equals_capacity, - len_equals_capacity, set_last_slice_value, new_slice, ); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 3bbe14f866a1..625d7bd776a6 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -220,14 +220,13 @@ 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}" + "if {then_condition} then {then_value} else {else_value}" ) } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index db2d96aac818..1a11786ed819 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -560,7 +560,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(); @@ -656,11 +655,6 @@ impl<'f> Context<'f> { } let then_condition = then_branch.condition; - let else_condition = if let Some(branch) = else_branch { - branch.condition - } else { - self.inserter.function.dfg.make_constant(FieldElement::zero(), Type::bool()) - }; let block = self.inserter.function.entry_block(); // Merging must occur in a separate loop as we cannot borrow `self` as mutable while `value_merger` does @@ -669,7 +663,6 @@ impl<'f> Context<'f> { let instruction = Instruction::IfElse { then_condition, then_value: *then_case, - else_condition, else_value: *else_case, }; let dfg = &mut self.inserter.function.dfg; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 75ee57dd4fab..6fb17f634f0c 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/noir/noir-repo/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 { @@ -114,31 +111,38 @@ impl<'a> ValueMerger<'a> { // We must cast the bool conditions to the actual numeric type used by each value. let then_condition = dfg .insert_instruction_and_results( - Instruction::Cast(then_condition, then_type), - block, - None, - call_stack.clone(), - ) - .first(); - let else_condition = dfg - .insert_instruction_and_results( - Instruction::Cast(else_condition, else_type), + Instruction::Cast(then_condition, Type::field()), block, None, call_stack.clone(), ) .first(); - let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); - let then_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let then_field = Instruction::Cast(then_value, Type::field()); + let then_field_value = + dfg.insert_instruction_and_results(then_field, block, None, call_stack.clone()).first(); + + let else_field = Instruction::Cast(else_value, Type::field()); + let else_field_value = + dfg.insert_instruction_and_results(else_field, block, None, call_stack.clone()).first(); + + let diff = Instruction::binary(BinaryOp::Sub, then_field_value, else_field_value); + let diff_value = + dfg.insert_instruction_and_results(diff, block, None, call_stack.clone()).first(); - let mul = Instruction::binary(BinaryOp::Mul, else_condition, else_value); - let else_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let conditional_diff = Instruction::binary(BinaryOp::Mul, then_condition, diff_value); + let conditional_diff_value = dfg + .insert_instruction_and_results(conditional_diff, block, None, call_stack.clone()) + .first(); + + let merged_field = + Instruction::binary(BinaryOp::Add, else_field_value, conditional_diff_value); + let merged_field_value = dfg + .insert_instruction_and_results(merged_field, block, None, call_stack.clone()) + .first(); - let add = Instruction::binary(BinaryOp::Add, then_value, else_value); - dfg.insert_instruction_and_results(add, block, None, call_stack).first() + let merged = Instruction::Cast(merged_field_value, then_type); + dfg.insert_instruction_and_results(merged, block, None, call_stack).first() } /// Given an if expression that returns an array: `if c { array1 } else { array2 }`, @@ -148,7 +152,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -163,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, @@ -195,7 +197,6 @@ impl<'a> ValueMerger<'a> { merged.push_back(self.merge_values( then_condition, - else_condition, then_element, else_element, )); @@ -209,7 +210,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value_id: ValueId, else_value_id: ValueId, ) -> ValueId { @@ -267,12 +267,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)); } } @@ -315,7 +310,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, @@ -399,8 +393,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/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index c387e0b62342..0d5fb3c914c3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/noir/noir-repo/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); @@ -87,7 +86,6 @@ impl Context { let value = value_merger.merge_values( then_condition, - else_condition, then_value, else_value, ); From b4daa6a74e9a5e74f0cc58736075441e84c6bd52 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 20 Nov 2024 19:02:45 +0000 Subject: [PATCH 02/10] . --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 20 +++++++------------ .../src/ssa/ir/instruction/call.rs | 7 ++----- .../noirc_evaluator/src/ssa/ir/printer.rs | 7 ++----- .../src/ssa/opt/flatten_cfg/value_merger.rs | 6 +----- .../src/ssa/opt/remove_if_else.rs | 8 ++------ 5 files changed, 14 insertions(+), 34 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f4c8286158d0..940f7e769416 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -272,11 +272,7 @@ pub(crate) enum Instruction { /// /// 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_value: ValueId, - }, + IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId }, } impl Instruction { @@ -510,13 +506,11 @@ impl Instruction { assert_message: assert_message.clone(), } } - 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::IfElse { then_condition, then_value, else_value } => Instruction::IfElse { + then_condition: f(*then_condition), + then_value: f(*then_value), + else_value: f(*else_value), + }, } } @@ -723,7 +717,7 @@ impl Instruction { None } } - Instruction::IfElse { then_condition, then_value, 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) { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index c469f5491105..476ec0d0f52b 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -440,11 +440,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, - 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/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 625d7bd776a6..5802c7542bc2 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -220,14 +220,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_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = show(*then_condition); let then_value = show(*then_value); let else_value = show(*else_value); - writeln!( - f, - "if {then_condition} then {then_value} else {else_value}" - ) + writeln!(f, "if {then_condition} then {then_value} else {else_value}") } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 6fb17f634f0c..84baaac77781 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -195,11 +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, - then_element, - else_element, - )); + merged.push_back(self.merge_values(then_condition, then_element, else_element)); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index 0d5fb3c914c3..8076bc3cc997 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -66,7 +66,7 @@ impl Context { for instruction in instructions { match &function.dfg[instruction] { - Instruction::IfElse { then_condition, then_value, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = *then_condition; let then_value = *then_value; let else_value = *else_value; @@ -84,11 +84,7 @@ impl Context { call_stack, ); - let value = value_merger.merge_values( - then_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); From d2fc76225899eec53a5b300a8bf3c075e9afd66f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 20 Nov 2024 19:44:37 +0000 Subject: [PATCH 03/10] . --- .../src/ssa/opt/flatten_cfg.rs | 225 +++++++----------- .../src/ssa/parser/into_ssa.rs | 8 +- 2 files changed, 99 insertions(+), 134 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 1a11786ed819..179655dc6ec4 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -911,7 +911,7 @@ impl<'f> Context<'f> { mod test { use std::sync::Arc; - use acvm::acir::AcirField; + use acvm::{acir::AcirField, FieldElement}; use crate::ssa::{ function_builder::FunctionBuilder, @@ -951,11 +951,9 @@ mod test { v1 = not v0 enable_side_effects u1 1 v3 = cast v0 as Field - v4 = cast v1 as Field - v6 = mul v3, Field 3 - v8 = mul v4, Field 4 - v9 = add v6, v8 - return v9 + v5 = mul v3, Field -1 + v7 = add Field 4, v5 + return v7 } "; @@ -1020,11 +1018,10 @@ mod test { store v2 at v1 enable_side_effects u1 1 v6 = cast v0 as Field - v7 = cast v4 as Field - v8 = mul v6, Field 5 - v9 = mul v7, v2 - v10 = add v8, v9 - store v10 at v1 + v7 = sub Field 5, v2 + v8 = mul v6, v7 + v9 = add v2, v8 + store v9 at v1 return } "; @@ -1063,11 +1060,9 @@ mod test { store Field 6 at v1 enable_side_effects u1 1 v8 = cast v0 as Field - v9 = cast v4 as Field - v10 = mul v8, Field 5 - v11 = mul v9, Field 6 - v12 = add v10, v11 - store v12 at v1 + v10 = mul v8, Field -1 + v11 = add Field 6, v10 + store v11 at v1 return } "; @@ -1110,123 +1105,84 @@ mod test { // b7 b8 // ↘ ↙ // b9 - let main_id = Id::test_new(0); - let mut builder = FunctionBuilder::new("main".into(), main_id); - - let b1 = builder.insert_block(); - let b2 = builder.insert_block(); - let b3 = builder.insert_block(); - let b4 = builder.insert_block(); - let b5 = builder.insert_block(); - let b6 = builder.insert_block(); - let b7 = builder.insert_block(); - let b8 = builder.insert_block(); - let b9 = builder.insert_block(); - - let c1 = builder.add_parameter(Type::bool()); - let c4 = builder.add_parameter(Type::bool()); - - let r1 = builder.insert_allocate(Type::field()); - - let store_value = |builder: &mut FunctionBuilder, value: u128| { - let value = builder.field_constant(value); - builder.insert_store(r1, value); - }; - - let test_function = Id::test_new(1); - - let call_test_function = |builder: &mut FunctionBuilder, block: u128| { - let block = builder.field_constant(block); - let load = builder.insert_load(r1, Type::field()); - builder.insert_call(test_function, vec![block, load], Vec::new()); - }; - - let switch_store_and_test_function = - |builder: &mut FunctionBuilder, block, block_number: u128| { - builder.switch_to_block(block); - store_value(builder, block_number); - call_test_function(builder, block_number); - }; - - let switch_and_test_function = - |builder: &mut FunctionBuilder, block, block_number: u128| { - builder.switch_to_block(block); - call_test_function(builder, block_number); - }; - - store_value(&mut builder, 0); - call_test_function(&mut builder, 0); - builder.terminate_with_jmp(b1, vec![]); - - switch_store_and_test_function(&mut builder, b1, 1); - builder.terminate_with_jmpif(c1, b2, b3); - - switch_store_and_test_function(&mut builder, b2, 2); - builder.terminate_with_jmp(b4, vec![]); - - switch_store_and_test_function(&mut builder, b3, 3); - builder.terminate_with_jmp(b8, vec![]); - - switch_and_test_function(&mut builder, b4, 4); - builder.terminate_with_jmpif(c4, b5, b6); - - switch_store_and_test_function(&mut builder, b5, 5); - builder.terminate_with_jmp(b7, vec![]); - - switch_store_and_test_function(&mut builder, b6, 6); - builder.terminate_with_jmp(b7, vec![]); - - switch_and_test_function(&mut builder, b7, 7); - builder.terminate_with_jmp(b9, vec![]); - - switch_and_test_function(&mut builder, b8, 8); - builder.terminate_with_jmp(b9, vec![]); - - switch_and_test_function(&mut builder, b9, 9); - let load = builder.insert_load(r1, Type::field()); - builder.terminate_with_return(vec![load]); + let src = " + acir(inline) fn main f0 { + b0(v0: u1, v1: u1): + v2 = allocate -> &mut Field + store Field 0 at v2 + v4 = load v2 -> Field + // call v1(Field 0, v4) + jmp b1() + b1(): + store Field 1 at v2 + v6 = load v2 -> Field + // call v1(Field 1, v6) + jmpif v0 then: b2, else: b3 + b2(): + store Field 2 at v2 + v8 = load v2 -> Field + // call v1(Field 2, v8) + jmp b4() + b4(): + v12 = load v2 -> Field + // call v1(Field 4, v12) + jmpif v1 then: b5, else: b6 + b5(): + store Field 5 at v2 + v14 = load v2 -> Field + // call v1(Field 5, v14) + jmp b7() + b7(): + v18 = load v2 -> Field + // call v1(Field 7, v18) + jmp b9() + b9(): + v22 = load v2 -> Field + // call v1(Field 9, v22) + v23 = load v2 -> Field + return v23 + b6(): + store Field 6 at v2 + v16 = load v2 -> Field + // call v1(Field 6, v16) + jmp b7() + b3(): + store Field 3 at v2 + v10 = load v2 -> Field + // call v1(Field 3, v10) + jmp b8() + b8(): + v20 = load v2 -> Field + // call v1(Field 8, v20) + jmp b9() + } + "; - let ssa = builder.finish().flatten_cfg().mem2reg(); + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.flatten_cfg().mem2reg(); // Expected results after mem2reg removes the allocation and each load and store: - // - // fn main f0 { - // b0(v0: u1, v1: u1): - // call test_function(Field 0, Field 0) - // call test_function(Field 1, Field 1) - // enable_side_effects v0 - // call test_function(Field 2, Field 2) - // call test_function(Field 4, Field 2) - // v29 = and v0, v1 - // enable_side_effects v29 - // call test_function(Field 5, Field 5) - // v32 = not v1 - // v33 = and v0, v32 - // enable_side_effects v33 - // call test_function(Field 6, Field 6) - // enable_side_effects v0 - // v36 = mul v1, Field 5 - // v37 = mul v32, Field 2 - // v38 = add v36, v37 - // v39 = mul v1, Field 5 - // v40 = mul v32, Field 6 - // v41 = add v39, v40 - // call test_function(Field 7, v42) - // v43 = not v0 - // enable_side_effects v43 - // store Field 3 at v2 - // call test_function(Field 3, Field 3) - // call test_function(Field 8, Field 3) - // enable_side_effects Field 1 - // v47 = mul v0, v41 - // v48 = mul v43, Field 1 - // v49 = add v47, v48 - // v50 = mul v0, v44 - // v51 = mul v43, Field 3 - // v52 = add v50, v51 - // call test_function(Field 9, v53) - // return v54 - // } + let expected = " + acir(inline) fn main f0 { + b0(v0: u1, v1: u1): + v2 = allocate -> &mut Field + enable_side_effects v0 + v3 = mul v0, v1 + enable_side_effects v3 + v4 = not v1 + v5 = mul v0, v4 + enable_side_effects v0 + v6 = cast v3 as Field + v8 = mul v6, Field -1 + v10 = add Field 6, v8 + v11 = not v0 + enable_side_effects u1 1 + v13 = cast v0 as Field + v15 = sub v10, Field 3 + v16 = mul v13, v15 + v17 = add Field 3, v16 + return v17 + }"; let main = ssa.main(); let ret = match main.dfg[main.entry_block()].terminator() { @@ -1235,7 +1191,10 @@ mod test { }; let merged_values = get_all_constants_reachable_from_instruction(&main.dfg, ret); - assert_eq!(merged_values, vec![3, 5, 6]); + assert_eq!(merged_values, vec![FieldElement::from(3u128), FieldElement::from(6u128), -FieldElement::from(1u128)]); + + assert_normalized_ssa_equals(ssa, expected); + } #[test] @@ -1329,7 +1288,7 @@ mod test { fn get_all_constants_reachable_from_instruction( dfg: &DataFlowGraph, value: ValueId, - ) -> Vec { + ) -> Vec { match dfg[value] { Value::Instruction { instruction, .. } => { let mut values = vec![]; @@ -1347,7 +1306,7 @@ mod test { values.dedup(); values } - Value::NumericConstant { constant, .. } => vec![constant.to_u128()], + Value::NumericConstant { constant, .. } => vec![constant], _ => Vec::new(), } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 2a94a4fd1eb9..a0ea9ef1c75a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -308,7 +308,13 @@ impl Translator { } fn finish(self) -> Ssa { - self.builder.finish() + let mut ssa = self.builder.finish(); + // Normalize the IDs so we have a better chance of matching the SSA we parsed + // after the step-by-step reconstruction done during translation. This assumes + // that the SSA we parsed was printed by the `SsaBuilder`, which normalizes + // before each print. + ssa.normalize_ids(); + ssa } fn current_function_id(&self) -> FunctionId { From e34f1b16887eb56dc4c9276a0a45bc79758dc062 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 20 Nov 2024 19:47:50 +0000 Subject: [PATCH 04/10] . --- .../compiler/noirc_evaluator/src/ssa/parser/tests.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 9205353151e3..5011ad885eb5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -103,8 +103,8 @@ fn test_multiple_blocks_and_jmp() { acir(inline) fn main f0 { b0(): jmp b1(Field 1) - b1(v1: Field): - return v1 + b1(v0: Field): + return v0 } "; assert_ssa_roundtrip(src); @@ -115,11 +115,11 @@ fn test_jmpif() { let src = " acir(inline) fn main f0 { b0(v0: Field): - jmpif v0 then: b1, else: b2 - b1(): - return + jmpif v0 then: b2, else: b1 b2(): return + b1(): + return } "; assert_ssa_roundtrip(src); From 23a2b09f15d3b0f1ff62ac606648074966f2359c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 20 Nov 2024 20:05:31 +0000 Subject: [PATCH 05/10] . --- .../compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 179655dc6ec4..32deb7a800df 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1191,10 +1191,12 @@ mod test { }; let merged_values = get_all_constants_reachable_from_instruction(&main.dfg, ret); - assert_eq!(merged_values, vec![FieldElement::from(3u128), FieldElement::from(6u128), -FieldElement::from(1u128)]); + assert_eq!( + merged_values, + vec![FieldElement::from(3u128), FieldElement::from(6u128), -FieldElement::from(1u128)] + ); assert_normalized_ssa_equals(ssa, expected); - } #[test] From d620021d7a3f6c75301b78a6e9d3b7251989beb7 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 21 Nov 2024 13:11:10 +0000 Subject: [PATCH 06/10] . --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index aeb6978737e9..3b3fd73a1fd7 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -519,13 +519,11 @@ impl Instruction { assert_message: assert_message.clone(), } } - 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::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(), From 2c1e8ea2f0606d0d75ad972794fb1f77264a6344 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 25 Nov 2024 20:43:06 +0000 Subject: [PATCH 07/10] chore: ignore flakey test --- noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index f91487fd73ea..6cf7070e65e0 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1089,6 +1089,7 @@ mod test { } #[test] + #[ignore] #[should_panic( expected = "Attempted to recur more than 1000 times during inlining function 'main': acir(inline) fn main f0 {" )] From 06d3a271d0ce0067b70ad81ab9a2e67dd3646386 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 27 Nov 2024 19:20:04 +0000 Subject: [PATCH 08/10] . --- noir/scripts/test_native.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/noir/scripts/test_native.sh b/noir/scripts/test_native.sh index 1f6d633935db..e0b3618f8364 100755 --- a/noir/scripts/test_native.sh +++ b/noir/scripts/test_native.sh @@ -14,4 +14,6 @@ RUSTFLAGS=-Dwarnings cargo clippy --workspace --locked --release ./.github/scripts/cargo-binstall-install.sh cargo-binstall cargo-nextest --version 0.9.67 -y --secure +# See https://github.com/AztecProtocol/aztec-packages/pull/10080 +RUST_MIN_STACK=8388608 cargo nextest run --workspace --locked --release -E '!test(hello_world_example) & !test(simple_verifier_codegen)' From e1f23982c0ef107bdb52c51c7f41de78bc198198 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 27 Nov 2024 21:58:24 +0000 Subject: [PATCH 09/10] poke ci From ba424123e9924d171e659726709d41326caf194d Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 27 Nov 2024 22:03:25 +0000 Subject: [PATCH 10/10] invalidate cache --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 8f5a9809fdde..72c6c44fff00 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -136,7 +136,7 @@ std::string honk_vk_to_json(std::vector& data) } /** - * @brief Proves and Verifies an ACIR circuit + * @brief Proves and verifies an ACIR circuit * * Communication: * - proc_exit: A boolean value is returned indicating whether the proof is valid.