From 4c504d46fdcf1b95949993fffb0c88b827adb1cf Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 19 Mar 2024 09:37:15 +0000 Subject: [PATCH] fix: Signed integer comparisons in brillig --- .../src/brillig/brillig_gen/brillig_block.rs | 39 ++++++++++++++++++- .../brillig_signed_cmp/Nargo.toml | 6 +++ .../brillig_signed_cmp/Prover.toml | 1 + .../brillig_signed_cmp/src/main.nr | 8 ++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test_programs/execution_success/brillig_signed_cmp/Nargo.toml create mode 100644 test_programs/execution_success/brillig_signed_cmp/Prover.toml create mode 100644 test_programs/execution_success/brillig_signed_cmp/src/main.nr diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index f808bfac43b..a9eea74ddd9 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -1306,7 +1306,14 @@ impl<'block> BrilligBlock<'block> { BinaryOp::Sub => BrilligBinaryOp::Sub, BinaryOp::Mul => BrilligBinaryOp::Mul, BinaryOp::Eq => BrilligBinaryOp::Equals, - BinaryOp::Lt => BrilligBinaryOp::LessThan, + BinaryOp::Lt => { + if is_signed { + self.convert_signed_less_than(left, right, result_variable); + return; + } else { + BrilligBinaryOp::LessThan + } + } BinaryOp::And => BrilligBinaryOp::And, BinaryOp::Or => BrilligBinaryOp::Or, BinaryOp::Xor => BrilligBinaryOp::Xor, @@ -1434,6 +1441,36 @@ impl<'block> BrilligBlock<'block> { self.brillig_context.deallocate_single_addr(scratch_var_j); } + fn convert_signed_less_than( + &mut self, + left: SingleAddrVariable, + right: SingleAddrVariable, + result: SingleAddrVariable, + ) { + let biased_left = + SingleAddrVariable::new(self.brillig_context.allocate_register(), left.bit_size); + let biased_right = + SingleAddrVariable::new(self.brillig_context.allocate_register(), right.bit_size); + + let bias = self + .brillig_context + .make_constant_instruction((1_u128 << (left.bit_size - 1)).into(), left.bit_size); + + self.brillig_context.binary_instruction(left, bias, biased_left, BrilligBinaryOp::Add); + self.brillig_context.binary_instruction(right, bias, biased_right, BrilligBinaryOp::Add); + + self.brillig_context.binary_instruction( + biased_left, + biased_right, + result, + BrilligBinaryOp::LessThan, + ); + + self.brillig_context.deallocate_single_addr(biased_left); + self.brillig_context.deallocate_single_addr(biased_right); + self.brillig_context.deallocate_single_addr(bias); + } + fn add_overflow_check( &mut self, binary_operation: BrilligBinaryOp, diff --git a/test_programs/execution_success/brillig_signed_cmp/Nargo.toml b/test_programs/execution_success/brillig_signed_cmp/Nargo.toml new file mode 100644 index 00000000000..3f485df4a82 --- /dev/null +++ b/test_programs/execution_success/brillig_signed_cmp/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "brillig_signed_cmp" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/brillig_signed_cmp/Prover.toml b/test_programs/execution_success/brillig_signed_cmp/Prover.toml new file mode 100644 index 00000000000..4b719f83c16 --- /dev/null +++ b/test_programs/execution_success/brillig_signed_cmp/Prover.toml @@ -0,0 +1 @@ +minus_one = 255 diff --git a/test_programs/execution_success/brillig_signed_cmp/src/main.nr b/test_programs/execution_success/brillig_signed_cmp/src/main.nr new file mode 100644 index 00000000000..3e3ea0f4b0f --- /dev/null +++ b/test_programs/execution_success/brillig_signed_cmp/src/main.nr @@ -0,0 +1,8 @@ +unconstrained fn main(minus_one: i8) { + assert(minus_one < 0); + assert(0 < minus_one as u8); + assert(0 > minus_one); + let most_negative_number = minus_one * 127 - 1; + assert(most_negative_number < 0); + assert(127 > most_negative_number); +}