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

chore: Avoid creating witness for simple multiplications #5100

Merged
merged 2 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -87,7 +87,7 @@
// This optimization will search for combinations of terms which can be represented in a single assert-zero opcode
// Case 1 : qM * wL * wR + qL * wL + qR * wR + qO * wO + qC
// This polynomial does not require any further optimizations, it can be safely represented in one opcode
// ie a polynomial with 1 mul(bi-variate) term and 3 (univariate) terms where 2 of those terms match the bivariate term

Check warning on line 90 in acvm-repo/acvm/src/compiler/transformers/csat.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bivariate)
// wL and wR, we can represent it in one opcode
// GENERALIZED for WIDTH: instead of the number 3, we use `WIDTH`
//
Expand Down Expand Up @@ -210,16 +210,16 @@
}
} 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
Loading