Skip to content

fix: correct bitsize in signed division#8733

Merged
TomAFrench merged 5 commits intomasterfrom
gd/issue_8726
Jun 3, 2025
Merged

fix: correct bitsize in signed division#8733
TomAFrench merged 5 commits intomasterfrom
gd/issue_8726

Conversation

@guipublic
Copy link
Contributor

Description

Problem*

Resolves #8726

Summary*

The absolute value of a n-bits signed integer has for its bit size n-1, except for -2^{n-1}.
So when the signed division is converted to an unsigned division, the bit size must be n.

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.

@github-actions
Copy link
Contributor

github-actions bot commented May 30, 2025

Changes to circuit sizes

Generated at commit: 8e6413d9cc2ff8f0af5915a78c08adef07a93392, compared to commit: 2e69fe88094e96cf62a28067240b775a4064cdc4

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_1144_1169_2399_6609 0 ➖ 0.00% +1,371 ❌ +35.76%
bit_shifts_runtime 0 ➖ 0.00% +1,375 ❌ +33.09%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression_1144_1169_2399_6609 292 (0) 0.00% 5,205 (+1,371) +35.76%
bit_shifts_runtime 436 (0) 0.00% 5,530 (+1,375) +33.09%
signed_div 495 (0) 0.00% 539 (+1) +0.19%
signed_inactive_division_by_zero 39 (0) 0.00% 2,804 (-1) -0.04%

@guipublic guipublic requested a review from a team June 2, 2025 10:14
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: 52010a4 Previous: cb4951d Ratio
test_report_zkpassport_noir_rsa_ 3 s 2 s 1.50

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

CC: @TomAFrench

@TomAFrench TomAFrench added this pull request to the merge queue Jun 3, 2025
Merged via the queue into master with commit 0b94ffc Jun 3, 2025
118 checks passed
@TomAFrench TomAFrench deleted the gd/issue_8726 branch June 3, 2025 10:10
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Jun 6, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for #8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
danielntmd pushed a commit to danielntmd/aztec-packages that referenced this pull request Jul 16, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: type unification tests, and try moving constants to the other side
(noir-lang/noir#8807)
fix!: disallow casting numeric to bool
(noir-lang/noir#8703)
fix: delay associated constants resolution
(noir-lang/noir#8744)
fix: right shift overflow to 0
(noir-lang/noir#8772)
fix: use value predicated by range checks
(noir-lang/noir#8778)
fix: unify infix expressions by isolating unbound type variables
(noir-lang/noir#8796)
feat: use `asm` feature flag in arkworks
(noir-lang/noir#8792)
chore: add a failing test for AztecProtocol#8780
(noir-lang/noir#8785)
chore: Adjust the frequency of 'for' statements in ACIR fuzz generation
(noir-lang/noir#8788)
fix: Merge `replacement_type` and `is_function_type` in
`defunctionalization` (noir-lang/noir#8784)
chore(fuzz): Remove unreachable functions in the AST fuzzer
(noir-lang/noir#8782)
chore(docs): Copy attribute docs into versioned docs
(noir-lang/noir#8777)
fix(licm): Preserve semantic ordering of side-effectual instructions
when hoisting (noir-lang/noir#8724)
fix: Create SSA interpreter arguments from scratch for each invocation
(noir-lang/noir#8762)
chore: mark sha512 as non-critical
(noir-lang/noir#8776)
fix!: disallow specifying associated items via generics
(noir-lang/noir#8756)
fix: stop inserting instructions after break and continue
(noir-lang/noir#8712)
fix: Fix comptime casts of negative integer to field
(noir-lang/noir#8696)
chore(SSA): restrict `shr` and `shl` right-hand side to u8
(noir-lang/noir#8753)
chore: bump some JS packages
(noir-lang/noir#8771)
chore: document `allow(dead_code)` and reorganize attributes
(noir-lang/noir#8766)
fix: Add missing cases for finding function values in
`find_functions_as_values` (noir-lang/noir#8738)
fix: correct bitsize in signed division
(noir-lang/noir#8733)
chore: remove noir-lang/noir_rsa from external libraries
(noir-lang/noir#8752)
chore: bump external pinned commits
(noir-lang/noir#8747)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
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.

ACIR vs Brillig: Signed div

3 participants