fix(ssa): Slice mergers with multiple ifs

crates/nargo_cli/tests/execution_success/slice_dynamic_index/src/main.nr

@@ -20,6 +20,7 @@ fn regression_dynamic_slice_index(x: Field, y: Field) {
 
     dynamic_slice_merge_if(slice, x);
     dynamic_slice_merge_else(slice, x);
+    dynamic_slice_merge_two_ifs(slice, x);
 }
 
 fn dynamic_slice_index_set_if(mut slice: [Field], x: Field, y: Field) {
@@ -164,6 +165,35 @@ fn dynamic_slice_merge_else(mut slice: [Field], x: Field) {
     assert(slice[slice.len() - 1] == 20);
 }
 
+fn dynamic_slice_merge_two_ifs(mut slice: [Field], x: Field) {
+    if x as u32 > 10 {
+        assert(slice[x] == 0);
+        slice[x] = 2;
+    } else {
+        assert(slice[x] == 4);
+        slice[x] = slice[x] - 2;
+        slice = slice.push_back(10);
+    }
+
+    assert(slice.len() == 6);
+    assert(slice[slice.len() - 1] == 10);
+
+    if x == 20 {
+        slice = slice.push_back(20);
+    } else {
+        slice = slice.push_back(15);
+    }
+    // TODO(#2599): Breaks if the push back happens without the else case
+    // slice = slice.push_back(15);
+
+    assert(slice.len() == 7);
+    assert(slice[slice.len() - 1] == 15);
+
+    slice = slice.push_back(20);
+    assert(slice.len() == 8);
+    assert(slice[slice.len() - 1] == 20);
+}
+
 fn dynamic_slice_index_set_nested_if_else_else(mut slice: [Field], x: Field, y: Field) {
     assert(slice[x] == 4);
     assert(slice[y] == 1);

crates/nargo_cli/tests/execution_success/slices/src/main.nr

@@ -102,6 +102,18 @@ fn merge_slices_if(x: Field, y: Field) {
     let slice = merge_slices_mutate_in_loop(x, y);
     assert(slice[6] == 4);
     assert(slice.len() == 7);
+
+    let slice = merge_slices_mutate_two_ifs(x, y);
+    assert(slice.len() == 6);
+    assert(slice[3] == 5);
+    assert(slice[4] == 15);
+    assert(slice[5] == 30);
+
+    let slice = merge_slices_mutate_between_ifs(x, y);
+    assert(slice.len() == 6);
+    assert(slice[3] == 5);
+    assert(slice[4] == 30);
+    assert(slice[5] == 15);
 }
 
 fn merge_slices_else(x: Field) {
@@ -156,4 +168,46 @@ fn merge_slices_mutate_in_loop(x: Field, y: Field) -> [Field] {
         slice = slice.push_back(x);
     }
     slice
-}
+}
+
+fn merge_slices_mutate_two_ifs(x: Field, y: Field) -> [Field] {
+    let mut slice = [0; 2];
+    if x != y {
+        slice = slice.push_back(y);
+        slice = slice.push_back(x);
+    } else {
+        slice = slice.push_back(x);
+    }
+    if x == 20 {
+        slice = slice.push_back(20);
+    } else {
+        slice = slice.push_back(15);
+    }
+    // TODO(#2599): Breaks if the push back happens without the else case
+    // slice = slice.push_back(15);
+    slice = slice.push_back(30);
+
+    slice
+}
+
+fn merge_slices_mutate_between_ifs(x: Field, y: Field) -> [Field] {
+    let mut slice = [0; 2];
+    if x != y {
+        slice = slice.push_back(y);
+        slice = slice.push_back(x);
+    } else {
+        slice = slice.push_back(x);
+    }
+
+    slice = slice.push_back(30);
+
+    if x == 20 {
+        slice = slice.push_back(20);
+    } else {
+        slice = slice.push_back(15);
+    }
+    // TODO(#2599): Breaks if the push back happens without the else case
+    // slice = slice.push_back(15);
+
+    slice
+}

crates/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -171,7 +171,7 @@
     /// Creates a new array with the new value at the given index. All other elements are identical
     /// to those in the given array. This will not modify the original array.
     ///
-    /// An optional length can be provided to enabling handling of dynamic slice indices
+    /// An optional length can be provided to enable handling of dynamic slice indices.
     ArraySet { array: ValueId, index: ValueId, value: ValueId, length: Option<ValueId> },
 }
 
@@ -637,8 +637,8 @@
         let operand_type = dfg.type_of_value(self.lhs);
 
         if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
             // If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
             // Thus, we do not evaluate this divison as we want to avoid triggering a panic,
             // and division by zero should be handled by laying down constraints during ACIR generation.
             if matches!(self.operator, BinaryOp::Div | BinaryOp::Mod) && rhs == FieldElement::zero()
             {
@@ -788,7 +788,7 @@
 
                 // The divisor is being truncated into the type of the operand, which can potentially
                 // lead to the rhs being zero.
                 // If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
                 // Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
                 // and the operation should be handled by ACIR generation.
                 if matches!(self.operator, BinaryOp::Div) && rhs == 0 {

crates/noirc_evaluator/src/ssa/ir/instruction/call.rs

@@ -5,18 +5,19 @@
 use num_bigint::BigUint;
 
 use crate::ssa::ir::{
+    basic_block::BasicBlockId,
     dfg::DataFlowGraph,
     instruction::Intrinsic,
     map::Id,
     types::Type,
-    value::{Value, ValueId}, basic_block::BasicBlockId,
+    value::{Value, ValueId},
 };
 
 use super::{Binary, BinaryOp, Endian, Instruction, SimplifyResult};
 
 /// Try to simplify this call instruction. If the instruction can be simplified to a known value,
 /// that value is returned. Otherwise None is returned.
-/// 
+///
 /// The `block` parameter indicates the block any new instructions that are part of a call's
 /// simplification will be inserted into. For example, all slice intrinsics require updates
 /// to the slice length, which requires inserting a binary instruction. This update instruction
@@ -227,12 +228,17 @@
 
 /// Slices have a tuple structure (slice length, slice contents) to enable logic
 /// that uses dynamic slice lengths (such as with merging slices in the flattening pass).
 /// This method codegens an update to the slice length.
 ///
 /// The binary operation performed on the slice length is always an addition or subtraction of `1`.
 /// This is because the slice length holds the user length (length as displayed by a `.len()` call),
 /// and not a flattened length used internally to represent arrays of tuples.
-fn update_slice_length(slice_len: ValueId, dfg: &mut DataFlowGraph, operator: BinaryOp, block: BasicBlockId) -> ValueId {
+fn update_slice_length(
+    slice_len: ValueId,
+    dfg: &mut DataFlowGraph,
+    operator: BinaryOp,
+    block: BasicBlockId,
+) -> ValueId {
     let one = dfg.make_constant(FieldElement::one(), Type::field());
     let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one });
     let call_stack = dfg.get_value_call_stack(slice_len);

crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -131,7 +131,7 @@
 //!   v11 = mul v4, Field 12
 //!   v12 = add v10, v11
 //!   store v12 at v5         (new store)
 use fxhash::FxHashMap as HashMap;
 use std::collections::{BTreeMap, HashSet};
 
 use acvm::FieldElement;
@@ -489,33 +489,36 @@
         let value = &self.inserter.function.dfg[value_id];
         match value {
             Value::Array { array, .. } => array.len(),
-            Value::NumericConstant { constant, .. } => constant.to_u128() as usize,
             Value::Instruction { instruction: instruction_id, .. } => {
                 let instruction = &self.inserter.function.dfg[*instruction_id];
                 match instruction {
-                    Instruction::ArraySet { length, .. } => {
-                        let length = length.expect("ICE: array set on a slice must have a length");
-                        self.get_slice_length(length)
-                    }
+                    Instruction::ArraySet { array, .. } => self.get_slice_length(*array),
                     Instruction::Load { address } => {
-                        let context_store = self
-                            .outer_block_stores
-                            .get(address)
-                            .expect("ICE: load in merger should have store from outer block");
+                        let context_store = if let Some(store) = self.store_values.get(address) {
+                            store
+                        } else {
+                            self.outer_block_stores
+                                .get(address)
+                                .expect("ICE: load in merger should have store from outer block")
+                        };
 
                         self.get_slice_length(context_store.new_value)
                     }
                     Instruction::Call { func, arguments } => {
                         let func = &self.inserter.function.dfg[*func];
-                        let length = arguments[0];
+                        let slice_contents = arguments[1];
                         match func {
                             Value::Intrinsic(intrinsic) => match intrinsic {
                                 Intrinsic::SlicePushBack
                                 | Intrinsic::SlicePushFront
-                                | Intrinsic::SliceInsert => self.get_slice_length(length) + 1,
+                                | Intrinsic::SliceInsert => {
+                                    self.get_slice_length(slice_contents) + 1
+                                }
                                 Intrinsic::SlicePopBack
                                 | Intrinsic::SlicePopFront
-                                | Intrinsic::SliceRemove => self.get_slice_length(length) - 1,
+                                | Intrinsic::SliceRemove => {
+                                    self.get_slice_length(slice_contents) - 1
+                                }
                                 _ => {
                                     unreachable!("ICE: Intrinsic not supported, got {intrinsic:?}")
                                 }