fix(ssa): Keep reference count increments for array set values

[vezenovm]

Description

Problem*

Resolves #9155

Summary*

Take the SSA posted from the issue:

Details

brillig(inline) predicate_pure fn func_1 f1 {
  b0(v0: [[Field; 1]; 1]):
    v2 = allocate -> &mut [[Field; 1]; 1]
    store v0 at v2
    v4 = make_array [Field 2] : [Field; 1]
    jmp b1(u32 0)
  b1(v1: u32):
    v7 = lt v1, u32 2
    jmpif v7 then: b2, else: b3
  b2():
    v11 = allocate -> &mut Field
    store Field 0 at v11
    jmp b4()
  b3():
    v8 = load v2 -> [[Field; 1]; 1]
    v9 = array_get v8, index u32 0 -> [Field; 1]
    inc_rc v9
    v10 = array_get v9, index u32 0 -> Field
    return v10
  b4():
    v13 = load v11 -> Field
    v15 = eq v13, Field 1
    jmpif v15 then: b5, else: b6
  b5():
    inc_rc v4                                                               // <- this inc_rc gets removed
    v22 = load v2 -> [[Field; 1]; 1]
    v23 = array_set v22, index u32 0, value v4
    store v23 at v2
    v25 = unchecked_add v1, u32 1
    jmp b1(v25)
  b6():
    v16 = load v11 -> Field
    v17 = add v16, Field 1
    store v17 at v11
    v18 = load v2 -> [[Field; 1]; 1]
    v19 = array_get v18, index u32 0 -> [Field; 1]
    v20 = array_set v19, index u32 0, value Field 1
    v21 = array_set v18, index u32 0, value v20
    store v21 at v2
    jmp b4()
}

When inc_rc v4 gets removed, we risk it later being mutated in v20 = array_set v19, index u32 0, value Field 1. Thus, when we later go to do v23 = array_set v22, index u32 0, value v4 once more, we will be writing [1] rather than [2].

The inc_rc in b5 is inserted by LICM (correctly). And if we hoist [2] manually and check the monomorphized program we can see that we are expected to clone [2]:

unconstrained fn func_1$f1(mut a$l0: [[Field; 1]; 1]) -> Field {
    let two$l1 = [2];
    {
        let mut i$l2 = 0;
        loop {
            if (i$l2 == 2) {
                break
            } else {
                i$l2 = (i$l2 + 1);
                let mut j$l3 = 0;
                loop {
                    if (j$l3 == 1) {
                        break
                    } else {
                        j$l3 = (j$l3 + 1);
                        a$l0[0][0] = 1
                    }
                };
                a$l0[0] = two$l1.clone()
            }
        }
    };;
    a$l0[0].clone()[0]
}

Additional Context

Thanks to @asterite for debugging this issue.

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: 51b4a8a8fcc0b84b28b9c655b52f75553862f7d6, compared to commit: 4f733adf44501d52eea35d03c9761b27e295af0f

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
fold_complex_outputs_inliner_max	+54 ❌	+14.96%
nested_array_in_slice_inliner_max	+90 ❌	+10.23%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_complex_outputs_inliner_max	415 (+54)	+14.96%
nested_array_in_slice_inliner_max	970 (+90)	+10.23%
fold_complex_outputs_inliner_zero	471 (+36)	+8.28%
fold_complex_outputs_inliner_min	542 (+36)	+7.11%
nested_array_dynamic_inliner_max	1,768 (+44)	+2.55%

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: 51b4a8a8fcc0b84b28b9c655b52f75553862f7d6, compared to commit: 4f733adf44501d52eea35d03c9761b27e295af0f

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
fold_complex_outputs_inliner_max	+54 ❌	+11.95%
nested_array_in_slice_inliner_max	+90 ❌	+8.40%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_complex_outputs_inliner_max	506 (+54)	+11.95%
nested_array_in_slice_inliner_max	1,162 (+90)	+8.40%
fold_complex_outputs_inliner_zero	681 (+36)	+5.58%
fold_complex_outputs_inliner_min	754 (+36)	+5.01%
nested_array_dynamic_inliner_max	2,693 (+44)	+1.66%

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 03a0b9a	Previous: 4f733ad	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr	4 s	3 s	1.33
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib	2 s	1 s	2
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types	2 s	1 s	2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[asterite]

LGTM

[jfecher]

@guipublic this seems related to #9341, can you confirm whether we still need that PR with this change?

Edit: Wrong PR, this one is not the one for lvalues