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

chore: avoid u128s in brillig memory #7363

Merged
merged 14 commits into from
Feb 13, 2025
Merged

Conversation

TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Feb 12, 2025

Description

Problem*

Resolves

Summary*

Currently we store all of our memory as u128s so we need to spend a bunch of time casting between this and smaller types. This PR makes our memory more flexible so that rather than just having a field and integer type (with an internal enum) we have a variant for each supported integer type.

This means that if we have a brillig program which acts exclusively on u8s, then we can maintain those types throughout execution rather than having to constantly cast up and down into u128.

Additional Context

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.

@TomAFrench TomAFrench marked this pull request as ready for review February 13, 2025 11:20
@TomAFrench TomAFrench requested a review from a team February 13, 2025 11:32
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e900992 Previous: 97afa52 Ratio
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 242 s 195 s 1.24

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench
Copy link
Member Author

⚠️ Performance Alert ⚠️

I think this is a false positive caused by AztecProtocol/aztec-packages#11897

Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@TomAFrench TomAFrench added this pull request to the merge queue Feb 13, 2025
Merged via the queue into master with commit c3deb6a Feb 13, 2025
103 checks passed
@TomAFrench TomAFrench deleted the tf/brillig-vm-experiments branch February 13, 2025 18:30
TomAFrench added a commit that referenced this pull request Feb 13, 2025
* master:
  chore: avoid u128s in brillig memory (#7363)
  chore: update docs about integer overflows (#7370)
  fix!: Only decrement the counter of an array if its address has not changed (#7297)
  fix: let LSP read `noirfmt.toml` for formatting files (#7355)
  chore: deprecate keccak256 (#7361)
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 13, 2025
chore: update docs about integer overflows (noir-lang/noir#7370)
fix!: Only decrement the counter of an array if its address has not changed (noir-lang/noir#7297)
fix: let LSP read `noirfmt.toml` for formatting files (noir-lang/noir#7355)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 13, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: avoid u128s in brillig memory
(noir-lang/noir#7363)
chore: update docs about integer overflows
(noir-lang/noir#7370)
fix!: Only decrement the counter of an array if its address has not
changed (noir-lang/noir#7297)
fix: let LSP read `noirfmt.toml` for formatting files
(noir-lang/noir#7355)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <[email protected]>
TomAFrench added a commit that referenced this pull request Feb 14, 2025
* master: (42 commits)
  fix: give "correct" error when trying to use AsTraitPath (#7360)
  chore: avoid u128s in brillig memory (#7363)
  chore: update docs about integer overflows (#7370)
  fix!: Only decrement the counter of an array if its address has not changed (#7297)
  fix: let LSP read `noirfmt.toml` for formatting files (#7355)
  chore: deprecate keccak256 (#7361)
  feat: `FunctionDefinition::as_typed_expr` (#7358)
  feat(performance): Check sub operations against induction variables (#7356)
  chore: avoid doing all brillig integer arithmetic on u128s (#7357)
  feat(cli): Add `--target-dir` option (#7350)
  fix(ssa): Make the lookback feature opt-in (#7190)
  feat(performance): Use unchecked ops based upon known induction variables (#7344)
  chore: mark sha256 as deprecated from the stdlib (#7351)
  fix: incorrect secondary file in LSP errors (#7347)
  chore: Basic test for MSM in Noir to catch performance improvements and regressions (#7341)
  fix(cli): Only lock the packages selected in the workspace (#7345)
  chore: remove some unused types and functions in the AST (#7339)
  chore: remove foreign calls array from Brillig VM constructor (#7337)
  chore(ci): Add Vecs and vecs to cspell (#7342)
  chore: redo typo PR by osrm (#7238)
  ...
TomAFrench added a commit that referenced this pull request Feb 14, 2025
…rom-brillig

* master:
  chore: allow opting in to displaying benchmark comments (#7399)
  chore: box `ExprValue` in `Value` enum (#7388)
  chore: pull out refactored methods from u128 branch (#7385)
  feat: require safety comments instead of safety doc comments (#7295)
  fix(ssa): Do not deduplicate division by a zero constant (#7393)
  chore: document traits required to be in scope (#7387)
  fix: field zero division in brillig (#7386)
  chore: box `ParserError`s in `InterpreterError` (#7373)
  chore: remove unnecessary dereferencing within brillig vm (#7375)
  fix: give "correct" error when trying to use AsTraitPath (#7360)
  chore: avoid u128s in brillig memory (#7363)
  chore: update docs about integer overflows (#7370)
  fix!: Only decrement the counter of an array if its address has not changed (#7297)
  fix: let LSP read `noirfmt.toml` for formatting files (#7355)
  chore: deprecate keccak256 (#7361)
  feat: `FunctionDefinition::as_typed_expr` (#7358)
  feat(performance): Check sub operations against induction variables (#7356)
  chore: avoid doing all brillig integer arithmetic on u128s (#7357)
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.

2 participants