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

feat(ci): Brillig opcode report workflow #5745

Closed
wants to merge 7 commits into from

Conversation

vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Aug 16, 2024

Description

Problem*

Related to #5101. Just that this is not a benchmark for execution speed and instead byte code size. This will be useful as we look to make more optimizations in the SSA to reduce the byte code size of unconstrained Noir code.

Summary*

Similar to the "Report gates diff" we now have a "Report Brillig opcodes diff" action. This will compile all of the tests under execution_success with --force-brillig. The report produced by gates_report_brillig.sh will be sent to a noir-gates-diff action that has been updated to handled the "unconstrained_functions" field of an InfoReport produced by nargo info.

Additional Context

The relevant noir-gates-diff updates can be found here: noir-lang/noir-gates-diff#5.

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.

Copy link
Contributor

Changes to Brillig bytecode sizes

Generated at commit: ea41f09df6d0cf993438873bdf2116a59407cd88

There are no changes in circuit sizes

github-merge-queue bot pushed a commit that referenced this pull request Aug 19, 2024
…s-diff report (#5747)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Successful example report:
<img width="964" alt="Screenshot 2024-08-19 at 9 31 35 AM"
src="https://github.com/user-attachments/assets/ccbddccd-a38a-4853-bcc2-1ac6bcb1fe36">

I originally just had this PR as a draft to test and close, but we can
just keep the PR now to add a size regression test for issue #4535:
```
struct EnumEmulation {
    a: Option<Field>,
    b: Option<Field>,
    c: Option<Field>,
}

unconstrained fn main() -> pub Field {
    let mut emulated_enum = EnumEmulation { a: Option::some(1), b: Option::none(), c: Option::none() };

    assert_eq(emulated_enum.a.unwrap(), 1);

    emulated_enum.a = Option::some(2);
    emulated_enum.a.unwrap()
}
```

This PR also provides a quicker way of updating the noir-gates-diff
commit as the original PR (#5745)
will first search for a report on master where a Brillig report does not
exist. On this branch we have a reference report on
`mv/brillig-opcode-report`. I think we could merge
`mv/brillig-opcode-report` into master and then any more commit hash
updates for the `noir-gates-diff` repo can be made on this PR.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <[email protected]>
@vezenovm
Copy link
Contributor Author

Closing as #5747 is merged

@vezenovm vezenovm closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant