Skip to content

Commit

Permalink
chore: Avoid creating witness for simple multiplications (#5100)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Related to  #4629

## Summary\*
Do not create a witness when multiplying a witness (or an affine
transformation of a witness) with an expression of degree 1.


## Additional Context
This PR does NOT fix the issue #4629, but it should help reducing the
overall gates number.
The arithmetic expressions resulting from the multiplication should not
explode in width since I only consider multiplication with a witness (or
'affine witness').

## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** 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
guipublic authored May 24, 2024
1 parent 281ebf2 commit 9db206e
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
4 changes: 2 additions & 2 deletions acvm-repo/acvm/src/compiler/transformers/csat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,16 @@ impl CSatTransformer {
}
} else {
// No more usable elements left in the old opcode
opcode.linear_combinations = remaining_linear_terms;
break;
}
}
opcode.linear_combinations.extend(remaining_linear_terms);

// Constraint this intermediate_opcode to be equal to the temp variable by adding it into the IndexMap
// We need a unique name for our intermediate variable
// XXX: Another optimization, which could be applied in another algorithm
// If two opcodes have a large fan-in/out and they share a few common terms, then we should create intermediate variables for them
// Do some sort of subset matching algorithm for this on the terms of the polynomial

let inter_var = Self::get_or_create_intermediate_vars(
intermediate_variables,
intermediate_opcode,
Expand Down
41 changes: 36 additions & 5 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,13 +614,44 @@ impl AcirContext {
expr.push_multiplication_term(FieldElement::one(), lhs_witness, rhs_witness);
self.add_data(AcirVarData::Expr(expr))
}
(
AcirVarData::Expr(_) | AcirVarData::Witness(_),
AcirVarData::Expr(_) | AcirVarData::Witness(_),
) => {
(AcirVarData::Expr(expression), AcirVarData::Witness(witness))
| (AcirVarData::Witness(witness), AcirVarData::Expr(expression))
if expression.is_linear() =>
{
let mut expr = Expression::default();
for term in expression.linear_combinations.iter() {
expr.push_multiplication_term(term.0, term.1, witness);
}
expr.push_addition_term(expression.q_c, witness);
self.add_data(AcirVarData::Expr(expr))
}
(AcirVarData::Expr(lhs_expr), AcirVarData::Expr(rhs_expr)) => {
let degree_one = if lhs_expr.is_linear() && rhs_expr.is_degree_one_univariate() {
Some((lhs_expr, rhs_expr))
} else if rhs_expr.is_linear() && lhs_expr.is_degree_one_univariate() {
Some((rhs_expr, lhs_expr))
} else {
None
};
if let Some((lin, univariate)) = degree_one {
let mut expr = Expression::default();
let rhs_term = univariate.linear_combinations[0];
for term in lin.linear_combinations.iter() {
expr.push_multiplication_term(term.0 * rhs_term.0, term.1, rhs_term.1);
}
expr.push_addition_term(lin.q_c * rhs_term.0, rhs_term.1);
expr.sort();
expr = expr.add_mul(univariate.q_c, &lin);
self.add_data(AcirVarData::Expr(expr))
} else {
let lhs = self.get_or_create_witness_var(lhs)?;
let rhs = self.get_or_create_witness_var(rhs)?;
self.mul_var(lhs, rhs)?
}
}
_ => {
let lhs = self.get_or_create_witness_var(lhs)?;
let rhs = self.get_or_create_witness_var(rhs)?;

self.mul_var(lhs, rhs)?
}
};
Expand Down

0 comments on commit 9db206e

Please sign in to comment.