Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIE is removing necessary inc_rc instructions #6583

Open
jfecher opened this issue Nov 21, 2024 · 0 comments · May be fixed by #6585
Open

DIE is removing necessary inc_rc instructions #6583

jfecher opened this issue Nov 21, 2024 · 0 comments · May be fixed by #6585
Labels
bug Something isn't working

Comments

@jfecher
Copy link
Contributor

jfecher commented Nov 21, 2024

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.

1 participant