Changes to circuit sizes

Generated at commit: e2ec5c9fdf97dd3af932886bc90796a2ac0442e0, compared to commit: 046fec77631958f75356b2d2445dd269509cb954

🧾 Summary (10% most significant diffs)

We are often going to eliminate all usage of certain brillig functions at this step but they end up being retained (and show up in show-ssa) currently. Can you add some tracking to drop these unused functions after this pass?

Sure!

Though... I just checked with this program:

fn main(x: Field) -> pub Field {
    let (a, _) = unsafe { identity(5) };
    assert_eq(a, 5);
    x + a
}

unconstrained fn identity(x: Field) -> (Field, Field) {
    let y = x;
    let z = y - 1;
    let w = z + 1;
    identity2(w)
}

unconstrained fn identity2(x: Field) -> (Field, Field) {
    (x, x)
}

and --show-ssa shows me this:

After Array Set Optimizations:
acir(inline) fn main f0 {
  b0(v0: Field):
    v2 = add v0, Field 5
    return v2
}

It seems after the inline pass the unused brillig functions are already being removed. Or... do you have some code where this doesn't happen?

I get the below SSA

After Inlining Const Brillig Calls:
acir(inline) fn main f0 {
  b0(v0: Field):
    v3 = add v0, Field 5
    return v3
}
brillig(inline) fn identity f1 {
  b0(v0: Field):
    v2 = sub v0, Field 1
    v3 = add v2, Field 1
    return v3, v3
}

We do remove this after the final inlining step but there's 9 passes in between these two steps so we get a good amount of unnecessary output in the SSA display

Oooh... you mean removing them in this same pass, right? Sure, I'll do it :-)

Yep, that's it

One extra thing to note is that this branch panics on the testcase added in #6464 (as opposed to just compiling to incorrect code).

Obviously there's a different issue in the compiler affecting that code already but fyi.

Ah, it's also with loop unrolling. I had to change something in this PR to workaround an issue... I guess I'll have to learn how loop unrolling works, and why it doesn't work in these brillig functions turned into ACIR.

I managed to reduce the code that triggers the panic in bignum_regression:

fn main() {
    unsafe { foo() };
}

unconstrained fn foo() {
    for i in 0..1 {
        if i == 0 {
            break;
        }
    }
}

It seems it's failing because of that break. It's likely that the optimization was made without break into account because loop unrolling only happens in ACIR code, and ACIR code doesn't allow breaks.

I can try to fix loop unrolling to account for this case, though it will likely take me a lot of time. Alternatively, we can avoid trying to inline these functions if we know they have break (for now).

Which one should I do?

For now I pushed a commit that makes it not panic.

I wonder if loop unrolling should be able to work in two modes, ACIR or "ACIR from Brillig". That way we can still panic and use expect for the normal case, and just ignore those errors for the "ACIR from Brillig" case. What do you think?

I can try to fix loop unrolling to account for this case, though it will likely take me a lot of time. Alternatively, we can avoid trying to inline these functions if we know they have break (for now).

Which one should I do?

For now let's just skip inlining that function and we can make improvements later.

Sounds good, done!

(just asked a review to signal that this is ready for review)

I marked this PR as draft because I want to try the other approach first (trying to invoke brillig functions, then inlining the results into the SSA and optimizing again)