From bcd897627c69b1ebcadc8b84abe2922ce3473c56 Mon Sep 17 00:00:00 2001
From: Maxim Vezenov <mvezenov@gmail.com>
Date: Fri, 25 Oct 2024 10:16:22 -0400
Subject: [PATCH] fix(ssa): Do not mark an array from a parameter mutable
 (#6355)

# Description

## Problem\*

Resolves #6349

## Summary\*

We check that we do not mark arrays mutable which comes from load
instructions as that array may be used by multiple values. This is
overridden by checking whether we are in the return block, in which case
we do mark the array as mutable. This however breaks when we no longer
inline everything. I switched the set of arrays from loads to a map of
arrays from loads to whether they come from a block parameter.

## Additional Context



## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
---
 .../noirc_evaluator/src/ssa/opt/array_set.rs  | 22 +++++++++++++------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
index 6ae13bc085a..7035345436e 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
@@ -53,7 +53,9 @@ struct Context<'f> {
     is_brillig_runtime: bool,
     array_to_last_use: HashMap<ValueId, InstructionId>,
     instructions_that_can_be_made_mutable: HashSet<InstructionId>,
-    arrays_from_load: HashSet<ValueId>,
+    // Mapping of an array that comes from a load and whether the address
+    // it was loaded from is a reference parameter.
+    arrays_from_load: HashMap<ValueId, bool>,
     inner_nested_arrays: HashMap<ValueId, InstructionId>,
 }
 
@@ -64,7 +66,7 @@ impl<'f> Context<'f> {
             is_brillig_runtime,
             array_to_last_use: HashMap::default(),
             instructions_that_can_be_made_mutable: HashSet::default(),
-            arrays_from_load: HashSet::default(),
+            arrays_from_load: HashMap::default(),
             inner_nested_arrays: HashMap::default(),
         }
     }
@@ -113,9 +115,13 @@ impl<'f> Context<'f> {
                             array_in_terminator = true;
                         }
                     });
-                    if (!self.arrays_from_load.contains(&array) || is_return_block)
-                        && !array_in_terminator
-                    {
+                    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.
+                        if !is_from_param && is_return_block {
+                            self.instructions_that_can_be_made_mutable.insert(*instruction_id);
+                        }
+                    } else if !array_in_terminator {
                         self.instructions_that_can_be_made_mutable.insert(*instruction_id);
                     }
                 }
@@ -133,10 +139,12 @@ impl<'f> Context<'f> {
                         }
                     }
                 }
-                Instruction::Load { .. } => {
+                Instruction::Load { address } => {
                     let result = self.dfg.instruction_results(*instruction_id)[0];
                     if matches!(self.dfg.type_of_value(result), Array { .. } | Slice { .. }) {
-                        self.arrays_from_load.insert(result);
+                        let is_reference_param =
+                            self.dfg.block_parameters(block_id).contains(address);
+                        self.arrays_from_load.insert(result, is_reference_param);
                     }
                 }
                 _ => (),