chore(ssa): Skip array_set pass for Brillig functions (#6513)

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

@@ -28,17 +28,21 @@ impl Ssa {
 
 impl Function {
     pub(crate) fn array_set_optimization(&mut self) {
+        if matches!(self.runtime(), RuntimeType::Brillig(_)) {
+            // Brillig is supposed to use refcounting to decide whether to mutate an array;
+            // array mutation was only meant for ACIR. We could use it with Brillig as well,
+            // but then some of the optimizations that we can do in ACIR around shared
+            // references have to be skipped, which makes it more cumbersome.
+            return;
+        }
+
         let reachable_blocks = self.reachable_blocks();
 
         if !self.runtime().is_entry_point() {
             assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
         }
 
-        let mut context = Context::new(
-            &self.dfg,
-            self.parameters(),
-            matches!(self.runtime(), RuntimeType::Brillig(_)),
-        );
+        let mut context = Context::new(&self.dfg);
 
         for block in reachable_blocks.iter() {
             context.analyze_last_uses(*block);
@@ -53,8 +57,6 @@ impl Function {
 
 struct Context<'f> {
     dfg: &'f DataFlowGraph,
-    function_parameters: &'f [ValueId],
-    is_brillig_runtime: bool,
     array_to_last_use: HashMap<ValueId, InstructionId>,
     instructions_that_can_be_made_mutable: HashSet<InstructionId>,
     // Mapping of an array that comes from a load and whether the address
@@ -64,15 +66,9 @@ struct Context<'f> {
 }
 
 impl<'f> Context<'f> {
-    fn new(
-        dfg: &'f DataFlowGraph,
-        function_parameters: &'f [ValueId],
-        is_brillig_runtime: bool,
-    ) -> Self {
+    fn new(dfg: &'f DataFlowGraph) -> Self {
         Context {
             dfg,
-            function_parameters,
-            is_brillig_runtime,
             array_to_last_use: HashMap::default(),
             instructions_that_can_be_made_mutable: HashSet::default(),
             arrays_from_load: HashMap::default(),
@@ -94,21 +90,12 @@ impl<'f> Context<'f> {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
                 }
-                Instruction::ArraySet { array, value, .. } => {
+                Instruction::ArraySet { array, .. } => {
                     let array = self.dfg.resolve(*array);
 
                     if let Some(existing) = self.array_to_last_use.insert(array, *instruction_id) {
                         self.instructions_that_can_be_made_mutable.remove(&existing);
                     }
-                    if self.is_brillig_runtime {
-                        let value = self.dfg.resolve(*value);
-
-                        if let Some(existing) = self.inner_nested_arrays.get(&value) {
-                            self.instructions_that_can_be_made_mutable.remove(existing);
-                        }
-                        let result = self.dfg.instruction_results(*instruction_id)[0];
-                        self.inner_nested_arrays.insert(result, *instruction_id);
-                    }
 
                     // If the array we are setting does not come from a load we can safely mark it mutable.
                     // If the array comes from a load we may potentially being mutating an array at a reference
@@ -128,17 +115,13 @@ impl<'f> Context<'f> {
                         }
                     });
 
-                    // We cannot safely mutate slices that are inputs to the function, as they might be shared with the caller.
-                    // NB checking the block parameters is not enough, as we might have jumped into a parameterless blocks inside the function.
-                    let is_function_param = self.function_parameters.contains(&array);
-
                     let can_mutate = if let Some(is_from_param) = self.arrays_from_load.get(&array)
                     {
                         // If the array was loaded from a reference parameter, we cannot
                         // safely mark that array mutable as it may be shared by another value.
                         !is_from_param && is_return_block
                     } else {
-                        !is_array_in_terminator && !is_function_param
+                        !is_array_in_terminator
                     };
 
                     if can_mutate {