noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
905 stars 206 forks source link

DIE is removing necessary `inc_rc` instructions #6583

Open jfecher opened 4 days ago

jfecher commented 4 days ago

Aim

Writing the function:

unconstrained fn borrow_mut(array: &mut [Field; 3]) {
    assert_refcount(*array, 2); // normal function call
    array[0] = 5;
    println(array[0]);
}

(with --inliner-aggressiveness -1000)

Expected Behavior

SSA before DIE is as expected:

brillig(inline) fn borrow_mut f2 {
  b0(v0: &mut [Field; 3]):
    v1 = load v0 -> [Field; 3]
    inc_rc v1                                    // increase rc of array/v0
    v2 = load v0 -> [Field; 3]
    call f5(v2, u32 2)
    v5 = load v0 -> [Field; 3]
    inc_rc v5                                    // extra rc (removed in https://github.com/noir-lang/noir/pull/6568)
    v6 = load v0 -> [Field; 3]
    v9 = array_set v6, index u32 0, value Field 5
    store v9 at v0
    call f6(Field 5)
    dec_rc v9
    return
}

Bug

Both v1 and v5 are otherwise unused in the above SSA so the program is incorrectly reduced down to:

brillig(inline) fn borrow_mut f2 {
  b0(v0: &mut [Field; 3]):
    v1 = load v0 -> [Field; 3]
    call f5(v1, u32 2)
    v4 = load v0 -> [Field; 3]
    v7 = array_set v4, index u32 0, value Field 5
    store v7 at v0
    call f6(Field 5)
    dec_rc v7
    return
}

Where all the inc_rcs are removed and the dec_rc even incorrectly remains

To Reproduce

1. 2. 3. 4.

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

None

Blocker Context

No response

Nargo Version

No response

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response