feat: added support for modulo ops on brillig (#1621)

* feat: added support for modulo ops

* chore: use ACVM 0.14.3

* revert change as its unrelated

* small comment refactor

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

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" }

crates/nargo_cli/tests/test_data_ssa_refactor/brillig_modulo/Nargo.toml

@@ -0,0 +1,5 @@
+[package]
+authors = [""]
+compiler_version = "0.1"
+
+[dependencies]

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
+}

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 {

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];