Skip to content

feat: add native u128 type#7301

Merged
TomAFrench merged 48 commits intomasterfrom
tf/u128-hacky
Feb 20, 2025
Merged

feat: add native u128 type#7301
TomAFrench merged 48 commits intomasterfrom
tf/u128-hacky

Conversation

@TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

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.

@kashbrti
Copy link
Contributor

kashbrti commented Feb 12, 2025

The following programs all fail with failed assertions for the addition and subtraction and overflows for mul and div,

#[test]
fn test_u128_sub() {
    let a: u128 = 671967750576550571863734675757137222;
    let b = 671967750576550571863734675757137221; 
    let c = a -1;  
    assert(c == b);
}


#[test]
fn test_u128_add() {
    let a: u128 = 671967750576550571863734675757137222;
    let b: u128 = 671967750576550571863734675757137221; 
    let c: u128 = b+1;  
    assert(c == a);
}

#[test]
fn test_u128_mul() {
    let a: u128 = 671967750576550571863734675757137222;
    let b: u128 = 335983875288275285931867337878568611; 
    let c: u128 = b* 2;  
    assert(c == a);
}

#[test]
fn test_u128_div() {
    let a: u128 = 671967750576550571863734675757137222;
    let b: u128 = 335983875288275285931867337878568611; 
    let c = a / 2;  
    assert(c == b);
}

I'm running the code after running noirup --pr 7301

TomAFrench and others added 4 commits February 13, 2025 18:40
* 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 and others added 2 commits February 14, 2025 20:16
* 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)
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: 8159a92 Previous: 093a8ec Ratio
AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 66 s 54 s 1.22

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

CC: @TomAFrench

@TomAFrench
Copy link
Member Author

TomAFrench commented Feb 17, 2025

@asterite Can you take over this PR? I'm not sure that I can devote enough time to it atm.

Remaining things to do include:

  • Solve the overflow issue
  • Block creation of the i128 type (we can potentially add this in future but I don't think we should add this in this PR)
  • Add an SSA pass which lowers multiplication and division of u128s into being implemented in smaller types for which we can perform these operations safely
    • We have implementations for the U128 struct here which can be used as inspiration:
      fn mul(self: Self, b: U128) -> U128 {
      assert(self.hi * b.hi == 0, "attempt to multiply with overflow");
      let low = self.lo * b.lo;
      let lo = low as u64 as Field;
      let carry = (low - lo) / pow64;
      let high = if crate::field::modulus_num_bits() as u32 > 196 {
      (self.lo + self.hi) * (b.lo + b.hi) - low + carry
      } else {
      self.lo * b.hi + self.hi * b.lo + carry
      };
      let hi = high as u64 as Field;
      assert(hi == high, "attempt to multiply with overflow");
      U128 { lo, hi }
      }
    • addition (and potentially subtraction) should be safe to leave as is.
    • This pass should only affect ACIR functions as brillig has full support for u128 types.
  • Add more selective version of this check: https://github.com/noir-lang/noir/pull/7301/files#diff-7c64342380b96ff80bc78d976a826185fbf2f08c0a10d77318dde72f697cb3b7 which only blocks unsafe operations.
  • A bunch of tests

@asterite
Copy link
Collaborator

Add an SSA pass which lowers multiplication and division of u128s into being implemented in smaller types for which we can perform these operations safely

Is there an operation that currently fails for this? I'm mainly trying to understand how it fails in order to understand how to implement this.

Add more selective version of this check

Why is that check needed? I know it says "for the truncation technique" but where is that, and why every math operation needs this check?

@TomAFrench
Copy link
Member Author

Is there an operation that currently fails for this? I'm mainly trying to understand how it fails in order to understand how to implement this.

The issue is that the bn254 field can only represent numbers up to ~2^253 safely as otherwise we overflow the field modulus. If we then multiply 2 128 bit numbers together it's possible that we'll wrap around the field modulus and get a "small" result, this will then pass the overflow check as it fits within 128 bits. This isn't an issue for the other types as they're small enough that we'll never overflow the field.

We then want to make sure that there's no operations which we do in ACIR gen which can overflow the field modulus. These should then be lowered to be implemented in terms of the smaller types to avoid the potential for an uncaught overflow.

@TomAFrench
Copy link
Member Author

LGTM

@TomAFrench TomAFrench added this pull request to the merge queue Feb 20, 2025
Merged via the queue into master with commit 8783e48 Feb 20, 2025
102 checks passed
@TomAFrench TomAFrench deleted the tf/u128-hacky branch February 20, 2025 10:21
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 20, 2025
chore: remove `disable_macros` compile option (noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467)
feat(experimental): show macro errors where they happen (noir-lang/noir#7333)
feat: optimize FieldElement::num_bits (noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation (noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456)
feat: add native `u128` type (noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 20, 2025
chore: remove `disable_macros` compile option (noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467)
feat(experimental): show macro errors where they happen (noir-lang/noir#7333)
feat: optimize FieldElement::num_bits (noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation (noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456)
feat: add native `u128` type (noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)
TomAFrench added a commit that referenced this pull request Feb 20, 2025
* master: (89 commits)
  chore: bump external pinned commits (#7472)
  chore: remove `disable_macros` compile option (#7468)
  chore(ci): add workflow to automate bumping aztec-packages commit (#7465)
  chore: Release Noir(1.0.0-beta.3) (#7346)
  chore(ci): Missing dash in profiler command argument (#7467)
  feat(experimental): show macro errors where they happen (#7333)
  feat: optimize FieldElement::num_bits (#7147)
  chore(profiler): Docs on profiler command and more complete error reporting (#7436)
  feat(ci): Release noir-inspector in binaries (#7464)
  chore(docs): Noir Profiler external documentation (#7457)
  feat(ci): Publish binaries for noir-profiler (#7443)
  chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (#7458)
  fix: prevent incorrect ACIRgen caused by noop truncations (#7456)
  feat: add native `u128` type (#7301)
  chore: standardize that doc comments on top of statements and expression are allowed but warn (#7450)
  fix: don't let nargo fmt produce multiple trailing newlines (#7444)
  feat(cli): add noir-execute binary (#7384)
  chore!: make `ResolverError::OracleMarkedAsConstrained` into a full error (#7426)
  chore: simplify reports (#7421)
  fix: do not discard negative sign from field literals in comptime interpreter (#7439)
  ...
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 20, 2025
fix: don't panic when shifting too much (noir-lang/noir#7429)
chore: bump external pinned commits (noir-lang/noir#7472)
chore: remove `disable_macros` compile option (noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467)
feat(experimental): show macro errors where they happen (noir-lang/noir#7333)
feat: optimize FieldElement::num_bits (noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation (noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456)
feat: add native `u128` type (noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 21, 2025
fix: don't panic when shifting too much (noir-lang/noir#7429)
chore: bump external pinned commits (noir-lang/noir#7472)
chore: remove `disable_macros` compile option (noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467)
feat(experimental): show macro errors where they happen (noir-lang/noir#7333)
feat: optimize FieldElement::num_bits (noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation (noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456)
feat: add native `u128` type (noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 21, 2025
fix: don't panic when shifting too much (noir-lang/noir#7429)
chore: bump external pinned commits (noir-lang/noir#7472)
chore: remove `disable_macros` compile option (noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467)
feat(experimental): show macro errors where they happen (noir-lang/noir#7333)
feat: optimize FieldElement::num_bits (noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation (noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456)
feat: add native `u128` type (noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)
TomAFrench added a commit to AztecProtocol/aztec-packages that referenced this pull request Feb 21, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: Sync from aztec-packages
(noir-lang/noir#7474)
fix: don't panic when shifting too much
(noir-lang/noir#7429)
chore: bump external pinned commits
(noir-lang/noir#7472)
chore: remove `disable_macros` compile option
(noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit
(noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3)
(noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument
(noir-lang/noir#7467)
feat(experimental): show macro errors where they happen
(noir-lang/noir#7333)
feat: optimize FieldElement::num_bits
(noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error
reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries
(noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation
(noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler
(noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs
(noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations
(noir-lang/noir#7456)
feat: add native `u128` type
(noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression
are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines
(noir-lang/noir#7444)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
AztecBot added a commit to AztecProtocol/aztec-nr that referenced this pull request Feb 22, 2025
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: Sync from aztec-packages
(noir-lang/noir#7474)
fix: don't panic when shifting too much
(noir-lang/noir#7429)
chore: bump external pinned commits
(noir-lang/noir#7472)
chore: remove `disable_macros` compile option
(noir-lang/noir#7468)
chore(ci): add workflow to automate bumping aztec-packages commit
(noir-lang/noir#7465)
chore: Release Noir(1.0.0-beta.3)
(noir-lang/noir#7346)
chore(ci): Missing dash in profiler command argument
(noir-lang/noir#7467)
feat(experimental): show macro errors where they happen
(noir-lang/noir#7333)
feat: optimize FieldElement::num_bits
(noir-lang/noir#7147)
chore(profiler): Docs on profiler command and more complete error
reporting (noir-lang/noir#7436)
feat(ci): Release noir-inspector in binaries
(noir-lang/noir#7464)
chore(docs): Noir Profiler external documentation
(noir-lang/noir#7457)
feat(ci): Publish binaries for noir-profiler
(noir-lang/noir#7443)
chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs
(noir-lang/noir#7458)
fix: prevent incorrect ACIRgen caused by noop truncations
(noir-lang/noir#7456)
feat: add native `u128` type
(noir-lang/noir#7301)
chore: standardize that doc comments on top of statements and expression
are allowed but warn (noir-lang/noir#7450)
fix: don't let nargo fmt produce multiple trailing newlines
(noir-lang/noir#7444)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <tom@tomfren.ch>
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.

3 participants