fix(ssa): synchronisation for functions (#764)

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
 {

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))
     }
 }
 

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

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;