diff --git a/crates/nargo/tests/test_data/9_conditional/src/main.nr b/crates/nargo/tests/test_data/9_conditional/src/main.nr index bdae37c8fab..80577b2a12e 100644 --- a/crates/nargo/tests/test_data/9_conditional/src/main.nr +++ b/crates/nargo/tests/test_data/9_conditional/src/main.nr @@ -198,6 +198,15 @@ fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]){ } constrain c_661[0] < 20000; + // Test case for function synchronisation + let mut c_sync = 0; + if a == 42 { + c_sync = foo2(); + } else { + c_sync = foo2() + foo2(); + } + constrain c_sync == 6; + // Regression for predicate simplification safe_inverse(0); } @@ -254,6 +263,9 @@ fn issue_661_bar(a : [u32;4]) ->[u32;4] { b } +fn foo2() -> Field { + 3 +} fn safe_inverse(n: Field) -> Field { diff --git a/crates/noirc_evaluator/src/ssa/conditional.rs b/crates/noirc_evaluator/src/ssa/conditional.rs index 757b4f4a27b..1dcd2a43847 100644 --- a/crates/noirc_evaluator/src/ssa/conditional.rs +++ b/crates/noirc_evaluator/src/ssa/conditional.rs @@ -375,7 +375,12 @@ impl DecisionTree { } let mut modified = false; super::optimizations::cse_block(ctx, left, &mut merged_ins, &mut modified)?; - + if modified { + // a second round is necessary when the synchronisation optimise function calls between the two branches + // in that case, the first cse update the result instructions to the same call + // the second cse can (and must) then simplify identical result instructions. + super::optimizations::cse_block(ctx, left, &mut merged_ins, &mut modified)?; + } //housekeeping... let if_block = &mut ctx[if_block_id]; if_block.dominated = vec![left]; @@ -977,8 +982,8 @@ impl Segment { Segment { left: (left_node.0, *left_node.1), right: (right_node.0, *right_node.1) } } pub fn intersect(&self, other: &Segment) -> bool { - (self.right.0 < other.right.0 && self.left.0 < other.left.0) - || (self.right.0 > other.right.0 && self.left.0 > other.left.0) + !((self.right.0 < other.right.0 && self.left.0 < other.left.0) + || (self.right.0 > other.right.0 && self.left.0 > other.left.0)) } } diff --git a/crates/noirc_evaluator/src/ssa/context.rs b/crates/noirc_evaluator/src/ssa/context.rs index d08662f4556..ff28f2d591d 100644 --- a/crates/noirc_evaluator/src/ssa/context.rs +++ b/crates/noirc_evaluator/src/ssa/context.rs @@ -702,7 +702,9 @@ impl SsaContext { //Optimization block::compute_dom(self); optimizations::full_cse(self, self.first_block, false)?; - + // the second cse is recommended because of opportunities occuring from the first one + // we could use an optimisation level that will run more cse pass + optimizations::full_cse(self, self.first_block, false)?; //flattening self.log(enable_logging, "\nCSE:", "\nunrolling:"); //Unrolling diff --git a/crates/noirc_evaluator/src/ssa/optimizations.rs b/crates/noirc_evaluator/src/ssa/optimizations.rs index 83b1217feaa..f002fd9337e 100644 --- a/crates/noirc_evaluator/src/ssa/optimizations.rs +++ b/crates/noirc_evaluator/src/ssa/optimizations.rs @@ -233,7 +233,7 @@ fn cse_block_with_anchor( new_list.push(*ins_id); } else if let Some(similar) = anchor.find_similar_instruction(&operator) { - debug_assert!(similar != ins.id); + assert_ne!(similar, ins.id); *modified = true; new_mark = Mark::ReplaceWith(similar); } else if binary.operator == BinaryOp::Assign { @@ -244,6 +244,16 @@ fn cse_block_with_anchor( anchor.push_front(&ins.operation, *ins_id); } } + Operation::Result { .. } => { + if let Some(similar) = anchor.find_similar_instruction(&operator) { + assert_ne!(similar, ins.id); + *modified = true; + new_mark = Mark::ReplaceWith(similar); + } else { + new_list.push(*ins_id); + anchor.push_front(&ins.operation, *ins_id); + } + } Operation::Load { array_id: x, .. } | Operation::Store { array_id: x, .. } => { if !is_join && ins.operation.is_dummy_store() { continue;