From 77154750c42b25d6265dc4fae6bda681334b28a8 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 22 Sep 2023 01:12:48 +0100 Subject: [PATCH] chore: remove redundant predicate from brillig quotients --- .../brillig/brillig_gen/brillig_directive.rs | 37 ++++++------------- .../ssa/acir_gen/acir_ir/generated_acir.rs | 6 +-- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index ee7bbeed46e..ea4ef0aa8cb 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -39,40 +39,37 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { } /// Generates brillig bytecode which computes `a / b` and returns the quotient and remainder. -/// It returns `(0,0)` if the predicate is null. -/// /// /// This is equivalent to the Noir (psuedo)code /// /// ```ignore -/// fn quotient(a: T, b: T, predicate: bool) -> (T,T) { -/// if predicate != 0 { -/// (a/b, a-a/b*b) -/// } else { -/// (0,0) -/// } +/// fn quotient(a: T, b: T) -> (T,T) { +/// (a/b, a-a/b*b) /// } /// ``` +/// +/// This bytecode explicitly does not handle the case where `b == 0`. This is because the [brillig opcode][brillig] +/// itself contains a predicate which can handle this case. If `b == 0` then this predicate should be set to zero, +/// which will result in the return values being set to (0, 0). +/// +/// [brillig]: acvm::acir::circuit::brillig::Brillig pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig { // `a` is (0) (i.e register index 0) // `b` is (1) - // `predicate` is (2) GeneratedBrillig { byte_code: vec![ - // If the predicate is zero, we jump to the exit segment - BrilligOpcode::JumpIfNot { condition: RegisterIndex::from(2), location: 6 }, - //q = a/b is set into register (3) + //q = a/b is set into register (2) BrilligOpcode::BinaryIntOp { op: BinaryIntOp::UnsignedDiv, lhs: RegisterIndex::from(0), rhs: RegisterIndex::from(1), - destination: RegisterIndex::from(3), + destination: RegisterIndex::from(2), bit_size, }, //(1)= q*b BrilligOpcode::BinaryIntOp { op: BinaryIntOp::Mul, - lhs: RegisterIndex::from(3), + lhs: RegisterIndex::from(2), rhs: RegisterIndex::from(1), destination: RegisterIndex::from(1), bit_size, @@ -88,17 +85,7 @@ pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig { //(0) = q BrilligOpcode::Mov { destination: RegisterIndex::from(0), - source: RegisterIndex::from(3), - }, - BrilligOpcode::Stop, - // Exit segment: we return 0,0 - BrilligOpcode::Const { - destination: RegisterIndex::from(0), - value: Value::from(0_usize), - }, - BrilligOpcode::Const { - destination: RegisterIndex::from(1), - value: Value::from(0_usize), + source: RegisterIndex::from(2), }, BrilligOpcode::Stop, ], diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 0adb14246b7..98180b51635 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -544,11 +544,7 @@ impl GeneratedAcir { let r_witness = self.next_witness_index(); let quotient_code = brillig_directive::directive_quotient(max_bit_size); - let inputs = vec![ - BrilligInputs::Single(lhs), - BrilligInputs::Single(rhs), - BrilligInputs::Single(predicate.clone()), - ]; + let inputs = vec![BrilligInputs::Single(lhs), BrilligInputs::Single(rhs)]; let outputs = vec![BrilligOutputs::Simple(q_witness), BrilligOutputs::Simple(r_witness)]; self.brillig(Some(predicate), quotient_code, inputs, outputs);