fix(ssa): Put some default in Value::uninitialized for references in the SSA interpreter

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

@@ -835,14 +835,15 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
                 "Result of allocate should always be a reference type, but found {other}"
             ),
         };
-        self.define(result, Value::reference(result, element_type))
+        let value = Value::reference(result, element_type);
+        self.define(result, value)
     }
 
     fn interpret_load(&mut self, address: ValueId, result: ValueId) -> IResult<()> {
         let address = self.lookup_reference(address, "load")?;
 
         let element = address.element.borrow();
-        let Some(value) = &*element else {
+        let Some(value) = element.as_ref() else {
             let value = address.to_string();
             return Err(internal(InternalError::UninitializedReferenceValueLoaded { value }));
         };

compiler/noirc_evaluator/src/ssa/interpreter/value.rs

@@ -88,6 +88,7 @@ impl Value {
         }
     }
 
+    /// Create an empty reference value.
     pub(crate) fn reference(original_id: ValueId, element_type: Arc<Type>) -> Self {
         Value::Reference(ReferenceValue { original_id, element_type, element: Shared::new(None) })
     }
@@ -184,7 +185,16 @@ impl Value {
     pub(crate) fn uninitialized(typ: &Type, id: ValueId) -> Value {
         match typ {
             Type::Numeric(typ) => Value::Numeric(NumericValue::zero(*typ)),
-            Type::Reference(element_type) => Self::reference(id, element_type.clone()),
+            Type::Reference(element_type) => {
+                // Initialize the reference to a default value, so that if we execute a
+                // Load instruction when side effects are disabled, we don't get an error.
+                let value = Self::uninitialized(element_type, id);
+                Self::Reference(ReferenceValue {
+                    original_id: id,
+                    element_type: element_type.clone(),
+                    element: Shared::new(Some(value)),
+                })
+            }
             Type::Array(element_types, length) => {
                 let first_elements =
                     vecmap(element_types.iter(), |typ| Self::uninitialized(typ, id));

compiler/noirc_evaluator/src/ssa/ir/function.rs

@@ -212,6 +212,7 @@ impl Function {
         unreachable!("SSA Function {} has no reachable return instruction!", self.id())
     }
 
+    /// Total number of instructions in the reachable blocks of this function.
     pub(crate) fn num_instructions(&self) -> usize {
         self.reachable_blocks()
             .iter()

compiler/noirc_evaluator/src/ssa/mod.rs

@@ -126,6 +126,11 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass> {
         ),
         // Run mem2reg with the CFG separated into blocks
         SsaPass::new(Ssa::mem2reg, "Mem2Reg"),
+        // Running DIE here might remove some unused instructions mem2reg could not eliminate.
+        SsaPass::new(
+            Ssa::dead_instruction_elimination_pre_flattening,
+            "Dead Instruction Elimination",
+        ),
         SsaPass::new(Ssa::simplify_cfg, "Simplifying"),
         SsaPass::new(Ssa::as_slice_optimization, "`as_slice` optimization")
             .and_then(Ssa::remove_unreachable_functions),

compiler/noirc_evaluator/src/ssa/opt/die.rs

@@ -55,6 +55,9 @@ impl Ssa {
         flattened: bool,
         skip_brillig: bool,
     ) -> Ssa {
+        #[cfg(debug_assertions)]
+        self.functions.values().for_each(|func| die_pre_check(func, flattened));
+
         let mut previous_unused_params = None;
         loop {
             let (new_ssa, result) =
@@ -609,7 +612,7 @@ fn can_be_eliminated_if_unused(
         | Load { .. }
         | IfElse { .. }
         // Arrays are not side-effectual in Brillig where OOB checks are laid down explicitly in SSA.
-        // However, arrays are side-effectual in ACIR (array OOB checks). 
+        // However, arrays are side-effectual in ACIR (array OOB checks).
         // We mark them available for deletion, but it is expected that this pass will insert
         // back the relevant side effects for array access in ACIR that can possible fail (e.g., index OOB or dynamic index).
         | ArrayGet { .. }
@@ -965,6 +968,15 @@ fn should_remove_store(func: &Function, flattened: bool) -> bool {
     flattened && func.runtime().is_acir() && func.reachable_blocks().len() == 1
 }
 
+/// Check pre-execution properties:
+/// * Passing `flattened = true` will confirm the CFG has already been flattened into a single block for ACIR functions
+#[cfg(debug_assertions)]
+fn die_pre_check(func: &Function, flattened: bool) {
+    if flattened {
+        super::flatten_cfg::flatten_cfg_post_check(func);
+    }
+}
+
 /// Check post-execution properties:
 /// * Store and Load instructions should be removed from ACIR after flattening.
 #[cfg(debug_assertions)]
@@ -1494,7 +1506,7 @@ mod test {
             v1 = make_array [u1 1] : [u1; 1]
             v2 = make_array [v1] : [[u1; 1]; 1]
             inc_rc v1
-            inc_rc v2 
+            inc_rc v2
             return v2
         }
         "#;
@@ -1564,8 +1576,8 @@ mod test {
     fn does_not_replace_valid_array_set() {
         let src = r"
         acir(inline) fn main f0 {
-          b0(v0: [u8; 32]):                 	
-            v3 = array_set v0, index u32 0, value u8 5    	                     	
+          b0(v0: [u8; 32]):
+            v3 = array_set v0, index u32 0, value u8 5
             return v3
         }
         ";

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

@@ -193,8 +193,10 @@ impl Ssa {
 /// Panics if:
 ///   - Any acir function has at least 1 loop
 #[cfg(debug_assertions)]
-#[allow(dead_code)]
 fn flatten_cfg_pre_check(function: &Function) {
+    if !function.runtime().is_acir() {
+        return;
+    }
     let loops = super::unrolling::Loops::find_all(function);
     assert_eq!(loops.yet_to_unroll.len(), 0);
 }
@@ -204,10 +206,12 @@ fn flatten_cfg_pre_check(function: &Function) {
 /// Panics if:
 ///   - Any acir function contains > 1 block
 #[cfg(debug_assertions)]
-#[allow(dead_code)]
 pub(super) fn flatten_cfg_post_check(function: &Function) {
+    if !function.runtime().is_acir() {
+        return;
+    }
     let blocks = function.reachable_blocks();
-    assert_eq!(blocks.len(), 1);
+    assert_eq!(blocks.len(), 1, "CFG contains more than 1 block");
 }
 
 pub(crate) struct Context<'f> {

compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs

@@ -356,16 +356,26 @@ fn zeroed_value(
             dfg.insert_instruction_and_results(instruction, block_id, None, stack).first()
         }
         Type::Reference(element_type) => {
+            // The result of the instruction is a reference; Allocate creates a reference,
+            // but if we tried to Load from it we would get an error, so follow it with a
+            // Store of a default value.
             let instruction = Instruction::Allocate;
             let reference_type = Type::Reference(Arc::new((**element_type).clone()));
 
-            dfg.insert_instruction_and_results(
-                instruction,
-                block_id,
-                Some(vec![reference_type]),
-                CallStackId::root(),
-            )
-            .first()
+            let reference_id = dfg
+                .insert_instruction_and_results(
+                    instruction,
+                    block_id,
+                    Some(vec![reference_type]),
+                    CallStackId::root(),
+                )
+                .first();
+
+            let value = zeroed_value(dfg, func_id, block_id, element_type.as_ref());
+            let instruction = Instruction::Store { address: reference_id, value };
+            dfg.insert_instruction_and_results(instruction, block_id, None, CallStackId::root());
+
+            reference_id
         }
         Type::Function => dfg.import_function(func_id),
     }
@@ -599,6 +609,42 @@ mod test {
         ");
     }
 
+    #[test]
+    fn replaces_references_following_conditional_constraint_with_allocate_and_store_of_default() {
+        let src = r#"
+        acir(inline) predicate_pure fn main f0 {
+          b0(v0: u1):
+            v1 = make_array [] : [&mut u1; 0]
+            enable_side_effects v0
+            v2 = mod u32 1, u32 0
+            constrain u1 0 == v0, "Index out of bounds"
+            v3 = array_get v1, index v2 -> &mut u1
+            v4 = load v3 -> u1
+            enable_side_effects u1 1
+            return v4
+        }
+        "#;
+        let ssa = Ssa::from_str(src).unwrap();
+        let ssa = ssa.remove_unreachable_instructions();
+
+        // The `array_get` is replaced by `allocate+store`.
+        assert_ssa_snapshot!(ssa, @r#"
+        acir(inline) predicate_pure fn main f0 {
+          b0(v0: u1):
+            v1 = make_array [] : [&mut u1; 0]
+            enable_side_effects v0
+            v4 = mod u32 1, u32 0
+            constrain u1 0 == v0, "attempt to calculate the remainder with a divisor of zero"
+            constrain u1 0 == v0, "Index out of bounds"
+            v6 = allocate -> &mut u1
+            store u1 0 at v6
+            v7 = load v6 -> u1
+            enable_side_effects u1 1
+            return v7
+        }
+        "#);
+    }
+
     #[test]
     fn does_not_replace_instructions_following_sub_that_overflows_after_next_side_effects_condition()
      {
@@ -754,7 +800,7 @@ mod test {
     }
 
     #[test]
-    fn does_not_removes_instructions_from_non_dominated_block_1() {
+    fn does_not_remove_instructions_from_non_dominated_block_1() {
         // Here both b1 and b4 are successors of b3, but both are not dominated by it.
         let src = r#"
         acir(inline) predicate_pure fn main f0 {
@@ -796,7 +842,7 @@ mod test {
     }
 
     #[test]
-    fn does_not_removes_instructions_from_non_dominated_block_2() {
+    fn does_not_remove_instructions_from_non_dominated_block_2() {
         // Here b3 is a successor of b2 but is not dominated by it.
         let src = r#"
         acir(inline) predicate_pure fn main f0 {
@@ -832,7 +878,7 @@ mod test {
     }
 
     #[test]
-    fn does_not_removes_instructions_from_non_dominated_block_3() {
+    fn does_not_remove_instructions_from_non_dominated_block_3() {
         // Here b4 is a transitive successor of b2 but is not dominated by it.
         let src = r#"
         acir(inline) predicate_pure fn main f0 {
@@ -872,7 +918,7 @@ mod test {
     }
 
     #[test]
-    fn does_not_removes_instructions_from_non_dominated_block_4() {
+    fn does_not_remove_instructions_from_non_dominated_block_4() {
         // Here b5 is a transitive successor of b2, but is not dominated by it
         // (it's a transitive successor of b1)
         let src = r#"

test_programs/execution_success/regression_9594/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "regression_9594"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/execution_success/regression_9594/Prover.toml

@@ -0,0 +1,4 @@
+a = false
+b = true
+c = 1
+return = 2

test_programs/execution_success/regression_9594/src/main.nr

@@ -0,0 +1,9 @@
+fn main(a: bool, mut b: bool, c: i32) -> pub i32 {
+    if a {
+        let mut e: [&mut bool] = &[];
+        b = *e[(1132210792_u32 % e.len())];
+        c
+    } else {
+        2
+    }
+}