From 8e976ea2104153b428d9df6e06ab53051e2832a7 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Thu, 3 Aug 2023 10:12:07 +0200 Subject: [PATCH] chore: replace usage of `Directive::Quotient` with brillig opcode (#1766) * Use brillig instead of inverse directive * *wip* * Remove usage of quotient directive in favor of brillig * chore: improve import * chore: remove unnecessary brillig quotient * chore: remove unnecessary comment change * chore: comment change * chore: push clone further up call stack * chore: correct comment on `brillig_quotient` * chore: improve docs for `directive_quotient` * chore: ignore pseudocode entirely --------- Co-authored-by: TomAFrench --- .../brillig/brillig_gen/brillig_directive.rs | 60 +++++++++++++++- .../ssa/acir_gen/acir_ir/generated_acir.rs | 68 +++++++++---------- 2 files changed, 93 insertions(+), 35 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index 219a954a595..93e760f9737 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -1,4 +1,6 @@ -use acvm::acir::brillig::{BinaryFieldOp, Opcode as BrilligOpcode, RegisterIndex, Value}; +use acvm::acir::brillig::{ + BinaryFieldOp, BinaryIntOp, Opcode as BrilligOpcode, RegisterIndex, Value, +}; /// Generates brillig bytecode which computes the inverse of its input if not null, and zero else. pub(crate) fn directive_invert() -> Vec { @@ -29,3 +31,59 @@ pub(crate) fn directive_invert() -> Vec { BrilligOpcode::Stop, ] } + +/// 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) +/// } +/// } +/// ``` +pub(crate) fn directive_quotient(bit_size: u32) -> Vec { + // `a` is (0) (i.e register index 0) + // `b` is (1) + // `predicate` is (2) + 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) + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::UnsignedDiv, + lhs: RegisterIndex::from(0), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(3), + bit_size, + }, + //(1)= q*b + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Mul, + lhs: RegisterIndex::from(3), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(1), + bit_size, + }, + //(1) = a-q*b + BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Sub, + lhs: RegisterIndex::from(0), + rhs: RegisterIndex::from(1), + destination: RegisterIndex::from(1), + bit_size, + }, + //(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) }, + BrilligOpcode::Stop, + ] +} diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 738387fbaab..b425eab42d3 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -11,7 +11,7 @@ use acvm::acir::{ brillig::Opcode as BrilligOpcode, circuit::{ brillig::{Brillig as AcvmBrillig, BrilligInputs, BrilligOutputs}, - directives::{LogInfo, QuotientDirective}, + directives::LogInfo, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, }, native_types::Witness, @@ -432,13 +432,13 @@ impl GeneratedAcir { } } - let (q_witness, r_witness) = self.quotient_directive( - lhs.clone(), - rhs.clone(), - Some(predicate.clone()), - max_q_bits, - max_rhs_bits, - )?; + let (q_witness, r_witness) = + self.brillig_quotient(lhs.clone(), rhs.clone(), predicate.clone(), max_bit_size + 1); + + // Apply range constraints to injected witness values. + // Constrains `q` to be 0 <= q < 2^{q_max_bits}, etc. + self.range_constraint(q_witness, max_q_bits)?; + self.range_constraint(r_witness, max_rhs_bits)?; // Constrain r < rhs self.bound_constraint_with_offset(&r_witness.into(), rhs, predicate, max_rhs_bits)?; @@ -457,6 +457,32 @@ impl GeneratedAcir { Ok((q_witness, r_witness)) } + /// Adds a brillig opcode which injects witnesses with values `q = a / b` and `r = a % b`. + /// + /// Suitable range constraints for `q` and `r` must be applied externally. + pub(crate) fn brillig_quotient( + &mut self, + lhs: Expression, + rhs: Expression, + predicate: Expression, + max_bit_size: u32, + ) -> (Witness, Witness) { + // Create the witness for the result + let q_witness = self.next_witness_index(); + 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 outputs = vec![BrilligOutputs::Simple(q_witness), BrilligOutputs::Simple(r_witness)]; + self.brillig(Some(predicate), quotient_code, inputs, outputs); + + (q_witness, r_witness) + } + /// Generate constraints that are satisfied iff /// lhs < rhs , when offset is 1, or /// lhs <= rhs, when offset is 0 @@ -692,32 +718,6 @@ impl GeneratedAcir { Ok(()) } - /// Adds a directive which injects witnesses with values `q = a / b` and `r = a % b`. - /// - /// Suitable range constraints are also applied to `q` and `r`. - pub(crate) fn quotient_directive( - &mut self, - a: Expression, - b: Expression, - predicate: Option, - q_max_bits: u32, - r_max_bits: u32, - ) -> Result<(Witness, Witness), RuntimeError> { - let q_witness = self.next_witness_index(); - let r_witness = self.next_witness_index(); - - let directive = - Directive::Quotient(QuotientDirective { a, b, q: q_witness, r: r_witness, predicate }); - self.push_opcode(AcirOpcode::Directive(directive)); - - // Apply range constraints to injected witness values. - // Constrains `q` to be 0 <= q < 2^{q_max_bits}, etc. - self.range_constraint(q_witness, q_max_bits)?; - self.range_constraint(r_witness, r_max_bits)?; - - Ok((q_witness, r_witness)) - } - /// Returns a `Witness` that is constrained to be: /// - `1` if lhs >= rhs /// - `0` otherwise