Skip to content

Commit

Permalink
feat: multiply first to allow more ACIR gen optimizations (#4201)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

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

## Summary\*

I noticed that we were getting suboptimal acir generation when using
`.any()` on `BoundedVec` where the `exceededLen` check would get more
expensive the longer the `BoundedVec` was.

The issue turned out to be that because we we would sum before
multiplying when performing the ORs on the length check, we weren't
taking advantage of the fact that multiplication can result in one of
its inputs being replaced with a single witness if the result would have
been degree 3 or more.

This meant that rather than having a simple `a + b - 2ab` for each OR
with `a` and `b` being witnesses , `a` would consist of a quadratic and
2 linear terms. We'd then continue to pick up extra terms for each
element in the `BoundedVec`.

The simple fix is then whenever we are performing operations on two
arguments where one of those operations is a multiplication (or anything
which can result in a simplification) we should do that first in order
to apply those simplifications to the other operations.

## 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.
  • Loading branch information
TomAFrench authored Jan 29, 2024
1 parent c50621f commit 882639d
Showing 1 changed file with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ impl AcirContext {
// Operands are booleans.
//
// a ^ b == a + b - 2*a*b
let sum = self.add_var(lhs, rhs)?;
let prod = self.mul_var(lhs, rhs)?;
let sum = self.add_var(lhs, rhs)?;
self.add_mul_var(sum, -FieldElement::from(2_i128), prod)
} else {
let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)];
Expand Down Expand Up @@ -457,8 +457,8 @@ impl AcirContext {
if bit_size == 1 {
// Operands are booleans
// a + b - ab
let sum = self.add_var(lhs, rhs)?;
let mul = self.mul_var(lhs, rhs)?;
let sum = self.add_var(lhs, rhs)?;
self.sub_var(sum, mul)
} else {
// Implement OR in terms of AND
Expand Down

0 comments on commit 882639d

Please sign in to comment.