From 379bb9168b3a98eda83c878a0e1d69d67923318f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 20 Jul 2023 18:17:09 +0000 Subject: [PATCH 1/3] feat: avoid unnecessary witness assignments in euclidian division --- .../acir_gen/acir_ir/generated_acir.rs | 21 ++++--------------- .../src/ssa_refactor/acir_gen/acir_ir/sort.rs | 2 +- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 5b05b1a0c1d..36ecd243f73 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -424,26 +424,13 @@ impl GeneratedAcir { self.bound_constraint_with_offset(&r_witness.into(), rhs, predicate, max_bit_size)?; // a * predicate == (b * q + r) * predicate - // => predicate * ( a - b * q - r) == 0 + // => predicate * (a - b * q - r) == 0 // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let rhs_reduced: Expression = self.create_witness_for_expression(rhs).into(); - let mut rhs_constraint = (&rhs_reduced * &Expression::from(q_witness)) - .expect("rhs_reduced is expected to be a degree-1 witness"); - rhs_constraint = &rhs_constraint + r_witness; - - // Reduce the rhs_constraint to a witness - let rhs_constrain_reduced: Expression = - self.create_witness_for_expression(&rhs_constraint).into(); - // Reduce the lhs_constraint to a witness - let lhs_reduced: Expression = self.create_witness_for_expression(lhs).into(); - - let div_euclidean = &(&lhs_reduced * predicate).expect( - "lhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", - ) - &(&rhs_constrain_reduced * predicate).expect( - "rhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", - ); + let rhs_constraint = &self.mul_with_witness(&rhs, &q_witness.into()) + r_witness; + let div_euclidean = &self.mul_with_witness(lhs, predicate) + - &self.mul_with_witness(&rhs_constraint, predicate); self.push_opcode(AcirOpcode::Arithmetic(div_euclidean)); diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs index d645f683666..517608a2827 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/sort.rs @@ -75,7 +75,7 @@ impl GeneratedAcir { /// Returns an expression which represents a*b /// If one has multiplicative term and the other is of degree one or more, /// the function creates intermediate variables accordindly - fn mul_with_witness(&mut self, a: &Expression, b: &Expression) -> Expression { + pub(super) fn mul_with_witness(&mut self, a: &Expression, b: &Expression) -> Expression { let a_arith; let a_arith = if !a.mul_terms.is_empty() && !b.is_const() { let a_witness = self.get_or_create_witness(a); From 427ce683a168d5ba510d8e123d607183db94a7b2 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 20 Jul 2023 18:27:03 +0000 Subject: [PATCH 2/3] feat: remove unnecessary witness creation in bound check --- .../src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 36ecd243f73..f3d3226edad 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -492,7 +492,7 @@ impl GeneratedAcir { assert!(bits + bit_size < FieldElement::max_num_bits()); //we need to ensure lhs_offset + r does not overflow let mut aor = lhs_offset; aor.q_c += FieldElement::from(r); - let witness = self.create_witness_for_expression(&aor); + let witness = self.get_or_create_witness(&aor); // lhs_offset<=rhs_offset <=> lhs_offset + r < rhs_offset + r = 2^bit_size <=> witness < 2^bit_size self.range_constraint(witness, bit_size)?; return Ok(()); From ac1de8650fea307d167554c02e47ef6751930ee9 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 20 Jul 2023 19:17:22 +0000 Subject: [PATCH 3/3] chore: clippy --- .../src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index f3d3226edad..af01f39593d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -428,7 +428,7 @@ impl GeneratedAcir { // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let rhs_constraint = &self.mul_with_witness(&rhs, &q_witness.into()) + r_witness; + let rhs_constraint = &self.mul_with_witness(rhs, &q_witness.into()) + r_witness; let div_euclidean = &self.mul_with_witness(lhs, predicate) - &self.mul_with_witness(&rhs_constraint, predicate);