fix(acir-gen): Use current side effect predicate in memory ops for slices

compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs

@@ -185,7 +185,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
                     // We generate witnesses corresponding to the array values
                     let index_var = self.add_constant(i);
 
-                    let value_read_var = self.read_from_memory(block_id, &index_var)?;
+                    let value_read_var = self.read_from_memory(block_id, &index_var, None)?;
                     let value_read = AcirValue::Var(value_read_var, AcirType::field());
 
                     self.brillig_array_input(var_expressions, value_read)?;

compiler/noirc_evaluator/src/acir/acir_context/mod.rs

@@ -1453,7 +1453,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
                     let index_var = self.add_constant(i);
 
                     Ok::<(AcirVar, AcirType), InternalError>((
-                        self.read_from_memory(block_id, &index_var)?,
+                        self.read_from_memory(block_id, &index_var, None)?,
                         value_types[i].into(),
                     ))
                 })
@@ -1485,12 +1485,26 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         id
     }
 
+    /// Convert an optional predicate to an expression, adding it to the witnesses if necessary.
+    ///
+    /// Returns `None` if the predicate is a constant 1.
+    fn predicate_to_expression(
+        &mut self,
+        predicate: Option<AcirVar>,
+    ) -> Result<Option<Expression<F>>, InternalError> {
+        predicate
+            .filter(|v| !self.is_constant_one(v))
+            .map(|v| self.get_or_create_witness_var(v).and_then(|v| self.var_to_expression(v)))
+            .transpose()
+    }
+
     /// Returns a Variable that is constrained to be the result of reading
     /// from the memory `block_id` at the given `index`.
     pub(crate) fn read_from_memory(
         &mut self,
         block_id: BlockId,
         index: &AcirVar,
+        predicate: Option<AcirVar>,
     ) -> Result<AcirVar, InternalError> {
         // Fetch the witness corresponding to the index
         let index_var = self.get_or_create_witness_var(*index)?;
@@ -1500,9 +1514,12 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         let value_read_var = self.add_variable();
         let value_read_witness = self.var_to_witness(value_read_var)?;
 
+        // Optionally disable the operation with a predicate.
+        let predicate = self.predicate_to_expression(predicate)?;
+
         // Add the memory read operation to the list of opcodes
         let op = MemOp::read_at_mem_index(index_witness.into(), value_read_witness);
-        self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op, predicate: None });
+        self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op, predicate });
 
         Ok(value_read_var)
     }
@@ -1513,6 +1530,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         block_id: BlockId,
         index: &AcirVar,
         value: &AcirVar,
+        predicate: Option<AcirVar>,
     ) -> Result<(), InternalError> {
         // Fetch the witness corresponding to the index
         let index_var = self.get_or_create_witness_var(*index)?;
@@ -1522,9 +1540,12 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         let value_write_var = self.get_or_create_witness_var(*value)?;
         let value_write_witness = self.var_to_witness(value_write_var)?;
 
+        // Optionally disable the operation with a predicate.
+        let predicate = self.predicate_to_expression(predicate)?;
+
         // Add the memory write operation to the list of opcodes
         let op = MemOp::write_to_mem_index(index_witness.into(), value_write_witness.into());
-        self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op, predicate: None });
+        self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op, predicate });
 
         Ok(())
     }
@@ -1591,7 +1612,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
                 let dynamic_array_values = try_vecmap(0..len, |i| {
                     let index_var = self.add_constant(i);
 
-                    let read = self.read_from_memory(block_id, &index_var)?;
+                    let read = self.read_from_memory(block_id, &index_var, None)?;
                     Ok::<AcirValue, InternalError>(AcirValue::Var(read, AcirType::field()))
                 })?;
                 for value in dynamic_array_values {

compiler/noirc_evaluator/src/acir/arrays.rs

@@ -300,7 +300,7 @@ impl Context<'_> {
                 let mut dummy_predicate_index = predicate_index;
                 // We must setup the dummy value to match the type of the value we wish to store
                 let dummy =
-                    self.array_get_value(&store_type, block_id, &mut dummy_predicate_index)?;
+                    self.array_get_value(&store_type, block_id, &mut dummy_predicate_index, false)?;
 
                 Some(self.convert_array_set_store_value(&store_value, &dummy)?)
             }
@@ -368,7 +368,7 @@ impl Context<'_> {
                 let values = try_vecmap(0..*len, |i| {
                     let index_var = self.acir_context.add_constant(i);
 
-                    let read = self.acir_context.read_from_memory(*block_id, &index_var)?;
+                    let read = self.acir_context.read_from_memory(*block_id, &index_var, None)?;
                     Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
                 })?;
 
@@ -396,7 +396,7 @@ impl Context<'_> {
         typ: &Type,
     ) -> Result<AcirValue, RuntimeError> {
         match typ {
-            Type::Numeric(_) => self.array_get_value(typ, call_data_block, offset),
+            Type::Numeric(_) => self.array_get_value(typ, call_data_block, offset, false),
             Type::Array(arc, len) => {
                 let mut result = im::Vector::new();
                 for _i in 0..*len {
@@ -440,7 +440,7 @@ impl Context<'_> {
                 !res_typ.contains_slice_element(),
                 "ICE: Nested slice result found during ACIR generation"
             );
-            self.array_get_value(&res_typ, block_id, &mut var_index)?
+            self.array_get_value(&res_typ, block_id, &mut var_index, false)?
         };
 
         if let AcirValue::Var(value_var, typ) = &value {
@@ -474,12 +474,17 @@ impl Context<'_> {
         ssa_type: &Type,
         block_id: BlockId,
         var_index: &mut AcirVar,
+        use_predicate: bool,
     ) -> Result<AcirValue, RuntimeError> {
         let one = self.acir_context.add_constant(FieldElement::one());
         match ssa_type.clone() {
             Type::Numeric(numeric_type) => {
                 // Read the value from the array at the specified index
-                let read = self.acir_context.read_from_memory(block_id, var_index)?;
+                let read = self.acir_context.read_from_memory(
+                    block_id,
+                    var_index,
+                    use_predicate.then_some(self.current_side_effects_enabled_var),
+                )?;
 
                 // Increment the var_index in case of a nested array
                 *var_index = self.acir_context.add_var(*var_index, one)?;
@@ -491,13 +496,18 @@ impl Context<'_> {
                 let mut values = im::Vector::new();
                 for _ in 0..len {
                     for typ in element_types.as_ref() {
-                        values.push_back(self.array_get_value(typ, block_id, var_index)?);
+                        values.push_back(self.array_get_value(
+                            typ,
+                            block_id,
+                            var_index,
+                            use_predicate,
+                        )?);
                     }
                 }
                 Ok(AcirValue::Array(values))
             }
             Type::Reference(reference_type) => {
-                self.array_get_value(reference_type.as_ref(), block_id, var_index)
+                self.array_get_value(reference_type.as_ref(), block_id, var_index, use_predicate)
             }
             _ => unreachable!("ICE: Expected an array or numeric but got {ssa_type:?}"),
         }
@@ -620,7 +630,7 @@ impl Context<'_> {
         match value {
             AcirValue::Var(store_var, _) => {
                 // Write the new value into the new array at the specified index
-                self.acir_context.write_to_memory(block_id, var_index, store_var)?;
+                self.acir_context.write_to_memory(block_id, var_index, store_var, None)?;
                 // Increment the var_index in case of a nested array
                 *var_index = self.acir_context.add_var(*var_index, one)?;
             }
@@ -633,7 +643,8 @@ impl Context<'_> {
                 let values = try_vecmap(0..*len, |i| {
                     let index_var = self.acir_context.add_constant(i);
 
-                    let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?;
+                    let read =
+                        self.acir_context.read_from_memory(*inner_block_id, &index_var, None)?;
                     Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
                 })?;
                 self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?;
@@ -779,8 +790,7 @@ impl Context<'_> {
     ) -> Result<im::Vector<AcirValue>, RuntimeError> {
         let init_values = try_vecmap(0..array_len, |i| {
             let index_var = self.acir_context.add_constant(i);
-
-            let read = self.acir_context.read_from_memory(source, &index_var)?;
+            let read = self.acir_context.read_from_memory(source, &index_var, None)?;
             Ok::<AcirValue, RuntimeError>(AcirValue::Var(read, AcirType::field()))
         })?;
         Ok(init_values.into())
@@ -815,7 +825,7 @@ impl Context<'_> {
                 self.acir_context.mul_var(var_index, self.current_side_effects_enabled_var)?;
 
             self.acir_context
-                .read_from_memory(element_type_sizes, &predicate_index)
+                .read_from_memory(element_type_sizes, &predicate_index, None)
                 .map_err(RuntimeError::from)
         }
     }

compiler/noirc_evaluator/src/acir/mod.rs

@@ -1401,8 +1401,12 @@ impl<'a> Context<'a> {
 
                 let mut popped_elements = Vec::new();
                 for res in &result_ids[2..] {
-                    let elem =
-                        self.array_get_value(&dfg.type_of_value(*res), block_id, &mut var_index)?;
+                    let elem = self.array_get_value(
+                        &dfg.type_of_value(*res),
+                        block_id,
+                        &mut var_index,
+                        true,
+                    )?;
                     popped_elements.push(elem);
                 }
 
@@ -1443,8 +1447,12 @@ impl<'a> Context<'a> {
                 // In the case of non-nested slice the logic is simple as we do not
                 // need to account for the internal slice sizes or flattening the index.
                 for res in &result_ids[..element_size] {
-                    let element =
-                        self.array_get_value(&dfg.type_of_value(*res), block_id, &mut var_index)?;
+                    let element = self.array_get_value(
+                        &dfg.type_of_value(*res),
+                        block_id,
+                        &mut var_index,
+                        true,
+                    )?;
                     let elem_size = arrays::flattened_value_size(&element);
                     popped_elements_size += elem_size;
                     popped_elements.push(element);
@@ -1543,8 +1551,11 @@ impl<'a> Context<'a> {
                         self.acir_context.add_var(use_shifted_index_pred, use_current_index_pred)?
                     };
 
-                    let value_shifted_index =
-                        self.acir_context.read_from_memory(block_id, &shifted_index)?;
+                    let value_shifted_index = self.acir_context.read_from_memory(
+                        block_id,
+                        &shifted_index,
+                        Some(self.current_side_effects_enabled_var),
+                    )?;
 
                     // Final predicate to determine whether we are within the insertion bounds
                     let should_insert_value_pred =
@@ -1565,6 +1576,7 @@ impl<'a> Context<'a> {
                         result_block_id,
                         &current_index,
                         &new_value,
+                        Some(self.current_side_effects_enabled_var),
                     )?;
 
                     current_insert_index += 1;
@@ -1645,8 +1657,12 @@ impl<'a> Context<'a> {
                 // the index internally.
                 let mut temp_index = flat_user_index;
                 for res in &result_ids[2..(2 + element_size)] {
-                    let element =
-                        self.array_get_value(&dfg.type_of_value(*res), block_id, &mut temp_index)?;
+                    let element = self.array_get_value(
+                        &dfg.type_of_value(*res),
+                        block_id,
+                        &mut temp_index,
+                        true,
+                    )?;
                     let elem_size = arrays::flattened_value_size(&element);
                     popped_elements_size += elem_size;
                     popped_elements.push(element);
@@ -1673,8 +1689,11 @@ impl<'a> Context<'a> {
                         let shifted_index =
                             self.acir_context.add_constant(i + popped_elements_size);
 
-                        let value_shifted_index =
-                            self.acir_context.read_from_memory(block_id, &shifted_index)?;
+                        let value_shifted_index = self.acir_context.read_from_memory(
+                            block_id,
+                            &shifted_index,
+                            Some(self.current_side_effects_enabled_var),
+                        )?;
 
                         let use_shifted_value = self.acir_context.more_than_eq_var(
                             current_index,
@@ -1695,6 +1714,7 @@ impl<'a> Context<'a> {
                             result_block_id,
                             &current_index,
                             &new_value,
+                            Some(self.current_side_effects_enabled_var),
                         )?;
                     };
                 }

compiler/noirc_evaluator/src/acir/tests/intrinsics.rs

@@ -43,7 +43,7 @@ fn slice_push_front_not_affected_by_predicate() {
 }
 
 #[test]
-fn slice_pop_back_not_affected_by_predicate() {
+fn slice_pop_back_affected_by_predicate() {
     let func_with_pred = &get_slice_intrinsic_acir(
         "v9, v10, v11, v12",
         &Intrinsic::SlicePopBack.to_string(),
@@ -57,11 +57,11 @@ fn slice_pop_back_not_affected_by_predicate() {
         false,
     )[0];
     assert_eq!(func_with_pred.current_witness_index(), func_no_pred.current_witness_index());
-    assert_eq!(func_with_pred.opcodes(), func_no_pred.opcodes());
+    assert_ne!(func_with_pred.opcodes(), func_no_pred.opcodes());
 }
 
 #[test]
-fn slice_pop_front_not_affected_by_predicate() {
+fn slice_pop_front_affected_by_predicate() {
     let func_with_pred = &get_slice_intrinsic_acir(
         "v9, v10, v11, v12",
         &Intrinsic::SlicePopFront.to_string(),
@@ -75,7 +75,7 @@ fn slice_pop_front_not_affected_by_predicate() {
         false,
     )[0];
     assert_eq!(func_with_pred.current_witness_index(), func_no_pred.current_witness_index());
-    assert_eq!(func_with_pred.opcodes(), func_no_pred.opcodes());
+    assert_ne!(func_with_pred.opcodes(), func_no_pred.opcodes());
 }
 
 #[test]

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

@@ -393,7 +393,13 @@ impl Instruction {
                 Value::Intrinsic(intrinsic) => {
                     // These utilize `noirc_evaluator::acir::Context::get_flattened_index` internally
                     // which uses the side effects predicate.
-                    matches!(intrinsic, Intrinsic::SliceInsert | Intrinsic::SliceRemove)
+                    matches!(
+                        intrinsic,
+                        Intrinsic::SliceInsert
+                            | Intrinsic::SliceRemove
+                            | Intrinsic::SlicePopBack
+                            | Intrinsic::SlicePopFront
+                    )
                 }
                 _ => false,
             },

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

@@ -275,23 +275,23 @@ mod test {
     }
 
     #[test]
-    fn remove_enable_side_effects_for_slice_pop_back() {
+    fn keep_enable_side_effects_for_slice_pop_back() {
         let src = get_slice_intrinsic_src(
             "v13, v14, v15",
             &Intrinsic::SlicePopBack.to_string(),
             ") -> (u32, [Field], Field)",
         );
-        verify_all_enable_side_effects_removed(&src);
+        verify_ssa_unchanged(&src);
     }
 
     #[test]
-    fn remove_enable_side_effects_for_slice_pop_front() {
+    fn keep_enable_side_effects_for_slice_pop_front() {
         let src = get_slice_intrinsic_src(
             "v13, v14, v15",
             &Intrinsic::SlicePopFront.to_string(),
             ") -> (Field, u32, [Field])",
         );
-        verify_all_enable_side_effects_removed(&src);
+        verify_ssa_unchanged(&src);
     }
 
     #[test]

test_programs/execution_success/regression_9467/Nargo.toml

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

test_programs/execution_success/regression_9467/Prover.toml

@@ -0,0 +1,2 @@
+return = [1, 1]
+b = false

test_programs/execution_success/regression_9467/src/main.nr

@@ -0,0 +1,8 @@
+global G_A: [u32] = &[];
+fn main(b: bool) -> pub (u32, u32) {
+    if b {
+        (G_A.pop_front().0, G_A.remove(0).1)
+    } else {
+        (1, 1)
+    }
+}