feat: Apply optimizations to unconstrained code

[jfecher]

Description

Problem*

Resolves #2066
Resolves #2336 - See note

Potentially addresses #2382 and #2355.

Summary*

#2336 was caused by us not removing old copies of functions if we ever failed to inline a function. While looking into it, performing a full reachability analysis to find which functions are still reachable was more difficult than just reworking inlining to count each unconstrained function as an entry point function along with main, and inline into all entry points. Once this was done I could move the generation of brillig to the end of ssa optimizations as it was no longer incompatible with inlining.

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

[vezenovm]

Can we remove the code introduced here: #2190

[jfecher]

Can we remove the code introduced here: #2190

I was thinking this could be a later PR, but sure

[TomAFrench]

Can you use the rebuild.sh script to regenerate the commited ACIR once this is ready? You'll need to have nargo installed from this branch in order to do this (run noirup -p ./ from the repository root)

[jfecher]

In a cruel twist of fate, this PR is also blocked on the mem2reg pass needing updates (and potentially more issues).

brillig_references for example contains a function

unconstrained fn add1(x: &mut Field) {
    *x += 1;
}

Which breaks mem2reg since the function is not inlined and it erroneously simplifies instructions after this function call with the old value of x. I was under the impression we didn't want to allow mutable references to be passed to Brillig functions? If we do then mem2reg needs to be updated not only to handle aliases but to handle function calls as well.

[jfecher]

brillig_to_le_bytes also panics with Message: index out of bounds: the len is 31 but the index is 31. I'm not sure why for this one - the final generated SSA looks noticeably more optimized (loops are unrolled) but otherwise equivalent to the original (which also accesses index 31):

After Dead Instruction Elimination:
brillig fn main f0 {
  b0(v0: Field):
    v45 = call to_le_radix(v0, u32 2⁸, u32 31)
    v47 = array_get v45, index Field 0
    v49 = array_get v45, index Field 1
    v51 = array_get v45, index Field 2
    v55 = array_get v45, index Field 31
    return [v47, v49, v51, v55]
}

[vezenovm]

brillig_to_le_bytes also panics with Message: index out of bounds: the len is 31 but the index is 31. I'm not sure why for this one - the final generated SSA looks noticeably more optimized (loops are unrolled) but otherwise equivalent to the original (which also accesses index 31):

I think this may be related to an error in the codegen for Brillig radix_instruction. I had to do this extra command for merging of slices (https://github.com/noir-lang/noir/pull/2347/files#diff-77a15e4d5423fb81c916909fd0c5854c33725d6ede78c5694916d8f53f6e739eR862) in order to get the accurate slice len for brillig_to_le_bytes. It looks like I also need to update the target_vector.size as otherwise memory can be overwritten.

[jfecher]

@vezenovm do you mind breaking that change into a separate PR?

[vezenovm]

do you mind breaking that change into a separate PR?

Will do

[ludamad]

I was under the impression we didn't want to allow mutable references to be passed to Brillig functions?

It's probably not the most sensible to have a mutable reference and change a bunch of constrained data models in Brillig, but within Brillig I would imagine you would want to (especially if Brillig is the execution model for externally constrained e.g. Aztec public fn's)

[jfecher]

Was using this PR to experiment with some mem2reg changes on top of the current PR, hence the additional changes here. Optimizing brillig_references necessitated some additional changes. I'll open another PR for those changes when ready, although this PR is failing without them.

[jfecher]

This PR is ready for review now

[vezenovm]

Some non-blocking nits, LGTM

[vezenovm]

Just need the ACIR artifacts to be reconciled and I can approve

[jfecher]

@vezenovm should be good now