From 9db206e98555ccd8089256144ef39ed43bd981b6 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 24 May 2024 18:03:43 +0200 Subject: [PATCH] chore: Avoid creating witness for simple multiplications (#5100) # 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. --- .../acvm/src/compiler/transformers/csat.rs | 4 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 41 ++++++++++++++++--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/transformers/csat.rs b/acvm-repo/acvm/src/compiler/transformers/csat.rs index 9e2e3091c74..12a37e3e37a 100644 --- a/acvm-repo/acvm/src/compiler/transformers/csat.rs +++ b/acvm-repo/acvm/src/compiler/transformers/csat.rs @@ -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, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 90cdbb650c2..b6e88a8d4b0 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -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)? } };