fix(ssa): Change array_set to not mutate slices coming from function inputs

[aakoshh]

Description

Problem*

Resolves #6444

Summary*

#6355 changed the logic in the array_set SSA pass so that arrays which were loaded from references that are inputs to the block are not set with mutation, as the reference might be shared with something outside the block.

In this instance what we had was a slice passed to a function like v1: [Field], as opposed to the above which passed a reference v1: &mut [Field; 3], but the effect was similar in that the code ended up mutating the slice backing v1, over which a refcount was even increased, instead of making a copy.

The PR changes the decision so that if we are setting an item on an array that is part of the function inputs (not the block inputs), then it will not use mutation.

Additional Context

See #6444 (comment) for the SSA samples.
I have tested this with #6429 but opened the PR to master.

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: 95a505a13e6c9d0d99628fc1aeb62b4fa6ee857f, compared to commit: b74b4eaf7acf6becd0f98ab25d0d5ccde3affebe

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
array_dynamic_main_output	+24 ❌	+36.36%
regression_mem_op_predicate	+24 ❌	+28.57%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
array_dynamic_main_output	90 (+24)	+36.36%
regression_mem_op_predicate	108 (+24)	+28.57%
array_if_cond_simple	131 (+24)	+22.43%
nested_dyn_array_regression_5782	169 (+24)	+16.55%
array_dynamic	331 (+3)	+0.91%
array_dynamic_nested_blackbox_input	1,010 (+3)	+0.30%

[github-actions]

Changes to circuit sizes

Generated at commit: 95a505a13e6c9d0d99628fc1aeb62b4fa6ee857f, compared to commit: b74b4eaf7acf6becd0f98ab25d0d5ccde3affebe

🧾 Summary (10% most significant diffs)

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
array_dynamic_main_output	+11 ❌	+33.33%	+42 ❌	+23.46%
array_dynamic_nested_blackbox_input	+28 ❌	+11.34%	+92 ❌	+1.26%

---

Full diff report 👇

Program	ACIR opcodes (+/-)	%	Circuit size (+/-)	%
array_dynamic_main_output	44 (+11)	+33.33%	221 (+42)	+23.46%
array_dynamic_nested_blackbox_input	275 (+28)	+11.34%	7,407 (+92)	+1.26%
array_dynamic	108 (+6)	+5.88%	3,731 (+17)	+0.46%
nested_dyn_array_regression_5782	35 (+3)	+9.38%	2,863 (+4)	+0.14%
nested_array_dynamic	3,195 (+29)	+0.92%	12,941 (+11)	+0.09%

[jfecher]

This only affects brillig code, right? If so, we should probably ignore the mutability on array sets completely there - that was only ever intended for Acir. Brillig should use the ref counts on each array to determine mutability.

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: 95a505a13e6c9d0d99628fc1aeb62b4fa6ee857f, compared to commit: b74b4eaf7acf6becd0f98ab25d0d5ccde3affebe

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
array_if_cond_simple	+234 ❌	+76.72%
array_dynamic_main_output	+94 ❌	+46.31%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
array_if_cond_simple	539 (+234)	+76.72%
array_dynamic_main_output	297 (+94)	+46.31%
nested_dyn_array_regression_5782	164 (+38)	+30.16%
array_dynamic_nested_blackbox_input	4,595 (+87)	+1.93%
array_dynamic	508 (+9)	+1.80%

[aakoshh]

This only affects brillig code, right?

Only brillig tests failed with this.

It is also my understanding that ACIR doesn't care about this mutability, and that it's a Brillig-only feature. In that light I don't understand why the ACIR code sizes increased in the report🤔

No I'm mixing it up with something. Ah yes, I read this on the Instructions::IncrementRc but remembered it in reverse:

// An instruction to increment the reference count of a value.
///
/// This currently only has an effect in Brillig code where array sharing and copy on write is
/// implemented via reference counting. In ACIR code this is done with im::Vector and these
/// IncrementRc instructions are ignored.

[aakoshh]

So you're saying that if Brillig ignored the mutable field on Instruction::ArraySet then it wouldn't make a mistake, but ACIR, which ignores the refcounts, should still be given the correct mutable field, right? In this instance, for example, adding mut is not correct, is it? If that's true, why doesn't the ACIR version of the test fail?

[jfecher]

So you're saying that if Brillig ignored the mutable field on Instruction::ArraySet then it wouldn't make a mistake, but ACIR, which ignores the refcounts, should still be given the correct mutable field, right? In this instance, for example, adding mut is not correct, is it? If that's true, why doesn't the ACIR version of the test fail?

The original intention was for Acir to only use the mutable field (and for that pass to only be for acir code), and for brillig to only use ref counts for mutation.
I'll look into the test more in-depth now to see why the acir version doesn't fail but generally speaking all acir code is inlined into one function and mutable references can't be passed between acir & brillig boundaries, so acir generally doesn't need to worry about function parameters. All loads and stores are also expected to be removed by the end of SSA in acir code.

[TomAFrench]

This PR was discussed in scrum and it was agreed to move forwards with this and then follow up with a removal of the concept of array mutability in brillig functions in a followup PR.

[jfecher]

@TomAFrench Making a note for later that this should be reverted then once the next PR is made. Since it prevents us from mutating arrays passed by parameters to main in acir and is extra logic that is unneeded once brillig is updated to not check the mutable flag