diff --git a/Cargo.lock b/Cargo.lock index 90260cc3009..86d0a9cf8fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "acir" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cafbc2adec7783509c6e06831ef57e672fc89c963c855a5fe0449c1ebb2822bc" +checksum = "5f2d2b80c4e6e0c6f2b5d45693925f6eac1e6fcf9a908aad70158e58b9ed18b0" dependencies = [ "acir_field", "brillig_vm", @@ -18,9 +18,9 @@ dependencies = [ [[package]] name = "acir_field" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a33855c911b77f0eddd9e0a6b9410238a5ba80a4956dcf335dd65dd4d2303d6" +checksum = "c984be877eae94755269391ebde859d65416c7aa7955badc3338464dbbdd1f38" dependencies = [ "ark-bn254", "ark-ff", @@ -32,9 +32,9 @@ dependencies = [ [[package]] name = "acvm" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ad7d5020f3cee249e162504deeeb1a83fa2a727e4c6a5277136bdecbe82cf2b" +checksum = "f31caccd81f7ea1ce5a9417ff283da0f4932ee28d63f6554049de03fe3d69d77" dependencies = [ "acir", "acvm_stdlib", @@ -71,9 +71,9 @@ dependencies = [ [[package]] name = "acvm_stdlib" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c9bf39b1e3b486f090ca1c0ef1e7d47fabfdb522af58117d1d86de048ac0571" +checksum = "b42f48d5c9ecfb1662ec7cbcf337fb4bb9323daf6e0168574431a9ec29684beb" dependencies = [ "acir", ] @@ -514,9 +514,9 @@ dependencies = [ [[package]] name = "brillig_vm" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a577e930e991623dd1ec5f9540b20eb601d06ae9d7f66181b81ff699441fc159" +checksum = "331e59215d1d45d136bcba2f1bbf18cf6c93477375afb851c4afaf8d7aa21b27" dependencies = [ "acir_field", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 38231e4bb24..72469ca2e20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ edition = "2021" rust-version = "1.66" [workspace.dependencies] -acvm = "=0.14.2" +acvm = "=0.14.3" arena = { path = "crates/arena" } fm = { path = "crates/fm" } iter-extended = { path = "crates/iter-extended" } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr new file mode 100644 index 00000000000..1cab78ecb95 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/src/main.nr @@ -0,0 +1,28 @@ +// Tests a very simple program. +// +// The features being tested is modulo operations on brillig +fn main() { + assert(modulo(47, 3) == 2); + assert(modulo(2, 3) == 2); + assert(signed_modulo(5, 3) == 2); + assert(signed_modulo(2, 3) == 2); + + let minus_two: i4 = 14; + let minus_three: i4 = 13; + let minus_five: i4 = 11; + + // (5 / -3) * -3 + 2 = -1 * -3 + 2 = 3 + 2 = 5 + assert(signed_modulo(5, minus_three) == 2); + // (-5 / 3) * 3 - 2 = -1 * 3 - 2 = -3 - 2 = -5 + assert(signed_modulo(minus_five, 3) == minus_two); + // (-5 / -3) * -3 - 2 = 1 * -3 - 2 = -3 - 2 = -5 + assert(signed_modulo(minus_five, minus_three) == minus_two); +} + +unconstrained fn modulo(x: u32, y: u32) -> u32 { + x % y +} + +unconstrained fn signed_modulo(x: i4, y: i4) -> i4 { + x % y +} diff --git a/crates/noirc_evaluator/src/brillig/binary.rs b/crates/noirc_evaluator/src/brillig/binary.rs index 93b307e1b70..56f75b70f11 100644 --- a/crates/noirc_evaluator/src/brillig/binary.rs +++ b/crates/noirc_evaluator/src/brillig/binary.rs @@ -44,25 +44,25 @@ impl BrilligBinaryOp { } fn binary_op_to_int_op(op: BinaryOp, is_signed: bool) -> BinaryIntOp { match op { - BinaryOp::Add => BinaryIntOp::Add, - BinaryOp::Sub => BinaryIntOp::Sub, - BinaryOp::Mul => BinaryIntOp::Mul, - BinaryOp::Div => { - if is_signed { - BinaryIntOp::SignedDiv - } else { - BinaryIntOp::UnsignedDiv - } - }, - BinaryOp::Mod => todo!("This is not supported by Brillig. It should either be added into Brillig or legalized by the SSA IR"), - BinaryOp::Eq => BinaryIntOp::Equals, - BinaryOp::Lt => BinaryIntOp::LessThan, - BinaryOp::And => BinaryIntOp::And, - BinaryOp::Or => BinaryIntOp::Or, - BinaryOp::Xor => BinaryIntOp::Xor, - BinaryOp::Shl => BinaryIntOp::Shl, - BinaryOp::Shr => BinaryIntOp::Shr, - } + BinaryOp::Add => BinaryIntOp::Add, + BinaryOp::Sub => BinaryIntOp::Sub, + BinaryOp::Mul => BinaryIntOp::Mul, + BinaryOp::Div => { + if is_signed { + BinaryIntOp::SignedDiv + } else { + BinaryIntOp::UnsignedDiv + } + } + BinaryOp::Mod => unreachable!("Modulo operations are handled separately"), + BinaryOp::Eq => BinaryIntOp::Equals, + BinaryOp::Lt => BinaryIntOp::LessThan, + BinaryOp::And => BinaryIntOp::And, + BinaryOp::Or => BinaryIntOp::Or, + BinaryOp::Xor => BinaryIntOp::Xor, + BinaryOp::Shl => BinaryIntOp::Shl, + BinaryOp::Shr => BinaryIntOp::Shr, + } } // If bit size is available then it is a binary integer operation match bit_size_signedness { diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen.rs b/crates/noirc_evaluator/src/brillig/brillig_gen.rs index 519e341b2be..07e9adb00ac 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen.rs @@ -7,9 +7,9 @@ use crate::ssa_refactor::ir::{ basic_block::{BasicBlock, BasicBlockId}, dfg::DataFlowGraph, function::Function, - instruction::{Binary, Instruction, InstructionId, TerminatorInstruction}, + instruction::{Binary, BinaryOp, Instruction, InstructionId, TerminatorInstruction}, post_order::PostOrder, - types::Type, + types::{NumericType, Type}, value::{Value, ValueId}, }; use acvm::{ @@ -241,6 +241,23 @@ impl BrilligGen { let left = self.convert_ssa_value(binary.lhs, dfg); let right = self.convert_ssa_value(binary.rhs, dfg); + // Process modulo operator separately as there is no + // Brillig modulo operator and the result is multiple + // brillig opcodes. + if let BinaryOp::Mod = binary.operator { + match binary_type { + Type::Numeric(NumericType::Unsigned { bit_size }) => { + self.convert_integer_mod(result_register, left, right, bit_size, false); + return; + } + Type::Numeric(NumericType::Signed { bit_size }) => { + self.convert_integer_mod(result_register, left, right, bit_size, true); + return; + } + _ => unimplemented!("ICE: Modulo operation not supported for type {binary_type:?}"), + } + } + let brillig_binary_op = BrilligBinaryOp::convert_ssa_binary_op_to_brillig_binary_op( binary.operator, binary_type, @@ -268,6 +285,53 @@ impl BrilligGen { } } + /// Computes left % right by emitting the necessary Brillig opcodes. + /// + /// This is done by using the following formula: + /// + /// a % b = a - (b * (a / b)) + fn convert_integer_mod( + &mut self, + result_register: RegisterIndex, + left: RegisterIndex, + right: RegisterIndex, + bit_size: u32, + signed: bool, + ) { + let scratch_register_i = self.create_register(); + let scratch_register_j = self.create_register(); + + // i = left / right + self.push_code(BrilligOpcode::BinaryIntOp { + op: match signed { + true => BinaryIntOp::SignedDiv, + false => BinaryIntOp::UnsignedDiv, + }, + destination: scratch_register_i, + bit_size, + lhs: left, + rhs: right, + }); + + // j = i * right + self.push_code(BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Mul, + destination: scratch_register_j, + bit_size, + lhs: scratch_register_i, + rhs: right, + }); + + // result_register = left - j + self.push_code(BrilligOpcode::BinaryIntOp { + op: BinaryIntOp::Sub, + destination: result_register, + bit_size, + lhs: left, + rhs: scratch_register_j, + }); + } + /// Converts an SSA `ValueId` into a `RegisterIndex`. fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> RegisterIndex { let value = &dfg[value_id];