Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added support for modulo ops on brillig #1621

Merged
merged 5 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
Empty file.
Original file line number Diff line number Diff line change
@@ -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
}
38 changes: 19 additions & 19 deletions crates/noirc_evaluator/src/brillig/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
68 changes: 66 additions & 2 deletions crates/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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];
Expand Down