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: replace bitwise ANDs used for truncation with Instruction::Truncate #4327

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

TomAFrench
Copy link
Member

Description

Problem*

Resolves

Summary*

It's common for us to have new Noir developers to perform bitwise operations in order to truncate a value before they cast to a smaller type, leading to programs such as the below:

fn main(num: u72) -> pub [u8; 8] {
    let mut out: [u8; 8] = [0; 8];
    for i in 0..8 {
        out[i] = (num >> (56 - (i * 8)) as u72 & 0xff) as u8;
    }

    out
}

This is expensive and unnecessary as we perform truncation on casts anyway. This PR then adds an optimisation which converts any bitwise AND of the form lhs & 0xff into an equivalent truncation instruction.

Before
acir fn main f0 {
  b0(v0: u72):
    v277 = div v0, u72 2⁵⁶
    v278 = and v277, u72 255
    v279 = truncate v278 to 8 bits, max_bit_size: 72
    v280 = cast v279 as u8
    v281 = div v0, u72 2⁴⁸
    v282 = and v281, u72 255
    v283 = truncate v282 to 8 bits, max_bit_size: 72
    v284 = cast v283 as u8
    v285 = div v0, u72 2⁴⁰
    v286 = and v285, u72 255
    v287 = truncate v286 to 8 bits, max_bit_size: 72
    v288 = cast v287 as u8
    v289 = div v0, u72 2³²
    v290 = and v289, u72 255
    v291 = truncate v290 to 8 bits, max_bit_size: 72
    v292 = cast v291 as u8
    v293 = div v0, u72 2²⁴
    v294 = and v293, u72 255
    v295 = truncate v294 to 8 bits, max_bit_size: 72
    v296 = cast v295 as u8
    v297 = div v0, u72 2¹⁶
    v298 = and v297, u72 255
    v299 = truncate v298 to 8 bits, max_bit_size: 72
    v300 = cast v299 as u8
    v301 = div v0, u72 2⁸
    v302 = and v301, u72 255
    v303 = truncate v302 to 8 bits, max_bit_size: 72
    v304 = cast v303 as u8
    v305 = and v0, u72 255
    v306 = truncate v305 to 8 bits, max_bit_size: 72
    v307 = cast v306 as u8
    return [v280, v284, v288, v292, v296, v300, v304, v307]
}

+--------------+----------------------+
| ACIR Opcodes | Backend Circuit Size |
+--------------+----------------------+
| 70           | 5617                 |
+--------------+----------------------+
After
acir fn main f0 {
  b0(v0: u72):
    v242 = div v0, u72 2⁵⁶
    v243 = truncate v242 to 8 bits, max_bit_size: 72
    v244 = cast v243 as u8
    v245 = div v0, u72 2⁴⁸
    v246 = truncate v245 to 8 bits, max_bit_size: 72
    v247 = cast v246 as u8
    v248 = div v0, u72 2⁴⁰
    v249 = truncate v248 to 8 bits, max_bit_size: 72
    v250 = cast v249 as u8
    v251 = div v0, u72 2³²
    v252 = truncate v251 to 8 bits, max_bit_size: 72
    v253 = cast v252 as u8
    v254 = div v0, u72 2²⁴
    v255 = truncate v254 to 8 bits, max_bit_size: 72
    v256 = cast v255 as u8
    v257 = div v0, u72 2¹⁶
    v258 = truncate v257 to 8 bits, max_bit_size: 72
    v259 = cast v258 as u8
    v260 = div v0, u72 2⁸
    v261 = truncate v260 to 8 bits, max_bit_size: 72
    v262 = cast v261 as u8
    v263 = truncate v0 to 8 bits, max_bit_size: 72
    v264 = cast v263 as u8
    return [v244, v247, v250, v253, v256, v259, v262, v264]
}

+--------------+----------------------+
| ACIR Opcodes | Backend Circuit Size |
+--------------+----------------------+
| 61           | 3801                 |
+--------------+----------------------+


The ANDs then completely disappear as the truncation instructions they get converted into are merged into the existing truncations.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] 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

github-actions bot commented Feb 10, 2024

Changes to circuit sizes

Generated at commit: 40f8b6073fa578c0e0b499b2c16cc3afadff60cc, compared to commit: fc32703ca05e2da15c781d88c96e7e29ce80ad95

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression +8 ❌ +3.17% -890 ✅ -21.61%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
regression 260 (+8) +3.17% 3,228 (-890) -21.61%

@TomAFrench TomAFrench added this pull request to the merge queue Feb 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 12, 2024
@TomAFrench TomAFrench added this pull request to the merge queue Feb 12, 2024
Merged via the queue into master with commit eb67ff6 Feb 12, 2024
39 checks passed
@TomAFrench TomAFrench deleted the tf/optimize-out-useless-ands branch February 12, 2024 17:51
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
# Description

## Problem\*

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

## Summary\*

In the vein of #4327. We now replace modulo operations with a power of
two with the more specific `Instruction::Truncate`.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.
github-merge-queue bot pushed a commit that referenced this pull request Feb 12, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>0.24.0</summary>

## [0.24.0](v0.23.0...v0.24.0)
(2024-02-12)


### ⚠ BREAKING CHANGES

* rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
* init storage macro
(AztecProtocol/aztec-packages#4200)
* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
* Sync commits from `aztec-packages`
([#4144](#4144))

### Features

* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add brillig array index check
([#4127](#4127))
([c29f85f](c29f85f))
* Add definitions for From and Into traits to Noir prelude
([#4169](#4169))
([4421ce4](4421ce4))
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
([158c8ce](158c8ce))
* Add instrumentation for tracking variables in debugging
([#4122](#4122))
([c58d691](c58d691))
* Add option to print monomorphized program
([#4119](#4119))
([80f7e29](80f7e29))
* Add support for overriding expression width
([#4117](#4117))
([c8026d5](c8026d5))
* Add warnings for usage of restricted bit sizes
([#4234](#4234))
([0ffc38b](0ffc38b))
* Allow bitshifts to be represented in SSA for brillig
([#4301](#4301))
([d86ff1a](d86ff1a))
* Allow brillig to read arrays directly from memory
(AztecProtocol/aztec-packages#4460)
([158c8ce](158c8ce))
* Allow globals to refer to any expression
([#4293](#4293))
([479330e](479330e))
* Allow nested arrays and vectors in Brillig foreign calls
(AztecProtocol/aztec-packages#4478)
([158c8ce](158c8ce))
* Allow variables and stack trace inspection in the debugger
([#4184](#4184))
([bf263fc](bf263fc))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Deallocate stack items at the instruction level
([#4339](#4339))
([8f024a8](8f024a8))
* Disable constraint bubbling pass
([#4131](#4131))
([9ba2de6](9ba2de6))
* Disable unused variable checks on low-level and oracle functions
([#4179](#4179))
([8f70e57](8f70e57))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Improve Error Handling for Cargo in Bootstrap Script
([#4211](#4211))
([3a90849](3a90849))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* **lsp:** Goto type reference for Struct
([#4091](#4091))
([d56cac2](d56cac2))
* Move bounded_vec into the noir stdlib
([#4197](#4197))
([c50621f](c50621f))
* Multiply first to allow more ACIR gen optimizations
([#4201](#4201))
([882639d](882639d))
* Option expect method
([#4219](#4219))
([8e042f2](8e042f2))
* Perform constraints on uncasted values if they are the same type
([#4303](#4303))
([816fa85](816fa85))
* Remove predicate from `sort` intrinsic function
([#4228](#4228))
([d646243](d646243))
* Remove replacement of boolean range opcodes with `AssertZero` opcodes
([#4107](#4107))
([dac0e87](dac0e87))
* Replace bitwise ANDs used for truncation with `Instruction::Truncate`
([#4327](#4327))
([eb67ff6](eb67ff6))
* Replace modulo operations with truncations where possible
([#4329](#4329))
([70f2435](70f2435))
* Separate compilation and expression narrowing in `nargo` interface
([#4100](#4100))
([62a4e37](62a4e37))
* Simplify all unsigned constant NOT instructions
([#4230](#4230))
([fab4a6e](fab4a6e))
* Sync commits from `aztec-packages`
([#4144](#4144))
([0205d3b](0205d3b))
* Use constraint information to perform constant folding
([#4060](#4060))
([9a4bf16](9a4bf16))


### Bug Fixes

* Accurate tracking of slice capacities across blocks
([#4240](#4240))
([7420dbb](7420dbb))
* Allow function calls in global definitions
([#4320](#4320))
([0dc205c](0dc205c))
* Allow performing bitwise NOT on unsigned integers
([#4229](#4229))
([b3ddf10](b3ddf10))
* Apply generic arguments from trait constraints before instantiating
identifiers ([#4121](#4121))
([eb6fc0f](eb6fc0f))
* Apply range constraints to return values from unconstrained functions
([#4217](#4217))
([3af2a89](3af2a89))
* Apply trait constraints from method calls
([#4152](#4152))
([68c5486](68c5486))
* Better errors for missing `fn` keyword
([#4154](#4154))
([057c208](057c208))
* Check for tests in all packages before failing due to an unsatisfied
test filter ([#4114](#4114))
([1107373](1107373))
* Clean error when attemping to return a slice from Brillig to ACIR
([#4280](#4280))
([bcad4ec](bcad4ec))
* Correct result when assigning shared arrays in unconstrained code
([#4210](#4210))
([bdd8a96](bdd8a96))
* **docs:** Codegen docs before cutting a new version
([#4183](#4183))
([2914310](2914310))
* Ensure that destination register is allocated when moving between
registers in brillig gen
([#4316](#4316))
([ca0a56e](ca0a56e))
* Ensure that unconstrained entrypoint functions don't generate
constraints ([#4292](#4292))
([fae4ead](fae4ead))
* From field with constant values
([#4226](#4226))
([593916b](593916b))
* **lsp:** Crash when file not in workspace
([#4146](#4146))
([cf7130f](cf7130f))
* **lsp:** Replace panics with errors
([#4209](#4209))
([26e9618](26e9618))
* Maintain correct type when simplifying `x ^ x`
([#4082](#4082))
([9d83c2b](9d83c2b))
* Message formatting for assert statement
([#4323](#4323))
([3972ead](3972ead))
* Prevent debugger crashing on circuits with no opcodes
([#4283](#4283))
([2e32845](2e32845))
* Prevent declarations of blackbox functions outside of the stdlib
([#4177](#4177))
([9fb6b09](9fb6b09))
* Remove panic from `init_log_level` in `acvm_js`
([#4195](#4195))
([2e26530](2e26530))
* Respect order in bubble up for redundant asserts
([#4109](#4109))
([189aa48](189aa48))
* Revert "correct result when assigning shared arrays" and added
regression test ([#4333](#4333))
([05e78b3](05e78b3))
* Save the data bus to the current function before generating others
([#4047](#4047))
([0a5bd4f](0a5bd4f))
* Simplify constant assert messages into `ConstrainError::Static`
([#4287](#4287))
([fd15052](fd15052))
* Ssa typing for array & slice indexes
([#4278](#4278))
([4074bab](4074bab))
* Ssa typing for assign_lvalue_index
([#4289](#4289))
([37f149c](37f149c))
* SSA typing for right shifts
([#4302](#4302))
([41ee1aa](41ee1aa))
* Ssa typing of make_offset
([#4277](#4277))
([e4378ee](e4378ee))
* Track graphs of item dependencies to find dependency cycles
([#4266](#4266))
([61eabf1](61eabf1))
* Type check ACIR mutable reference passed to brillig
([#4281](#4281))
([7e139de](7e139de))
* Update array method type signatures in the docs
([#4178](#4178))
([7c0a955](7c0a955))
* Zero out input to `to_radix` calls if inactive
([#4116](#4116))
([3f5bad3](3f5bad3))


### Miscellaneous Chores

* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
([158c8ce](158c8ce))
* Rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
([158c8ce](158c8ce))
</details>

<details><summary>0.40.0</summary>

## [0.40.0](v0.39.0...v0.40.0)
(2024-02-12)


### ⚠ BREAKING CHANGES

* rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
* init storage macro
(AztecProtocol/aztec-packages#4200)
* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
* Sync commits from `aztec-packages`
([#4144](#4144))
* Breaking changes from aztec-packages
([#3955](#3955))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
* Remove partial backend feature
([#3805](#3805))

### Features

* Add bit size to const opcode
(AztecProtocol/aztec-packages#4385)
([158c8ce](158c8ce))
* Add expression width into acir
(AztecProtocol/aztec-packages#4014)
([158c8ce](158c8ce))
* Add instrumentation for tracking variables in debugging
([#4122](#4122))
([c58d691](c58d691))
* Add support for overriding expression width
([#4117](#4117))
([c8026d5](c8026d5))
* Allow brillig to read arrays directly from memory
(AztecProtocol/aztec-packages#4460)
([158c8ce](158c8ce))
* Allow nested arrays and vectors in Brillig foreign calls
(AztecProtocol/aztec-packages#4478)
([158c8ce](158c8ce))
* Allow variables and stack trace inspection in the debugger
([#4184](#4184))
([bf263fc](bf263fc))
* **avm:** Back in avm context with macro - refactor context
(AztecProtocol/aztec-packages#4438)
([158c8ce](158c8ce))
* **aztec-nr:** Initial work for aztec public vm macro
(AztecProtocol/aztec-packages#4400)
([158c8ce](158c8ce))
* Aztec-packages
([#3754](#3754))
([c043265](c043265))
* Breaking changes from aztec-packages
([#3955](#3955))
([5be049e](5be049e))
* Evaluation of dynamic assert messages
([#4101](#4101))
([c284e01](c284e01))
* Init storage macro
(AztecProtocol/aztec-packages#4200)
([158c8ce](158c8ce))
* Remove range constraints from witnesses which are constrained to be
constants ([#3928](#3928))
([afe9c7a](afe9c7a))
* Remove replacement of boolean range opcodes with `AssertZero` opcodes
([#4107](#4107))
([dac0e87](dac0e87))
* Speed up transformation of debug messages
([#3815](#3815))
([2a8af1e](2a8af1e))
* Sync `aztec-packages`
([#4011](#4011))
([fee2452](fee2452))
* Sync commits from `aztec-packages`
([#4068](#4068))
([7a8f3a3](7a8f3a3))
* Sync commits from `aztec-packages`
([#4144](#4144))
([0205d3b](0205d3b))


### Bug Fixes

* Deserialize odd length hex literals
([#3747](#3747))
([4000fb2](4000fb2))
* Remove panic from `init_log_level` in `acvm_js`
([#4195](#4195))
([2e26530](2e26530))
* Return error rather instead of panicking on invalid circuit
([#3976](#3976))
([67201bf](67201bf))


### Miscellaneous Chores

* **acir:** Move `is_recursive` flag to be part of the circuit
definition (AztecProtocol/aztec-packages#4221)
([158c8ce](158c8ce))
* Remove partial backend feature
([#3805](#3805))
([0383100](0383100))
* Remove unused methods on ACIR opcodes
([#3841](#3841))
([9e5d0e8](9e5d0e8))
* Rename Arithmetic opcode to AssertZero
([#3840](#3840))
([836f171](836f171))
* Rename bigint_neg into bigint_sub
(AztecProtocol/aztec-packages#4420)
([158c8ce](158c8ce))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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