From 199d129bd7ae02c4336399455ecda69905dfa1bd Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 22 Jan 2024 14:09:36 +0000 Subject: [PATCH] chore: replace explicit subtractions with nots (#4097) # Description ## Problem\* Resolves ## Summary\* Small PR to replace usage of `(1-x)` instead of `not(x)` for boolean `x`. This improves readability of the codegen imo. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_evaluator/src/ssa/function_builder/mod.rs | 8 +++++--- compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs | 12 +++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 852848afb81..44be423be10 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -367,10 +367,12 @@ impl FunctionBuilder { let r_squared = self.insert_binary(r, BinaryOp::Mul, r); let a = self.insert_binary(r_squared, BinaryOp::Mul, lhs); let idx = self.field_constant(FieldElement::from((bit_size - i) as i128)); - let b = self.insert_array_get(rhs_bits, idx, Type::field()); + let b = self.insert_array_get(rhs_bits, idx, Type::bool()); + let not_b = self.insert_not(b); + let b = self.insert_cast(b, Type::field()); + let not_b = self.insert_cast(not_b, Type::field()); let r1 = self.insert_binary(a, BinaryOp::Mul, b); - let c = self.insert_binary(one, BinaryOp::Sub, b); - let r2 = self.insert_binary(c, BinaryOp::Mul, r_squared); + let r2 = self.insert_binary(r_squared, BinaryOp::Mul, not_b); r = self.insert_binary(r1, BinaryOp::Add, r2); } r diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index f1a2154d3a8..0e155776545 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -270,11 +270,12 @@ impl<'a> FunctionContext<'a> { /// helper function which add instructions to the block computing the absolute value of the /// given signed integer input. When the input is negative, we return its two complement, and itself when it is positive. fn absolute_value_helper(&mut self, input: ValueId, sign: ValueId, bit_size: u32) -> ValueId { + assert_eq!(self.builder.type_of_value(sign), Type::bool()); + // We compute the absolute value of lhs - let one = self.builder.numeric_constant(FieldElement::one(), Type::bool()); let bit_width = self.builder.numeric_constant(FieldElement::from(2_i128.pow(bit_size)), Type::field()); - let sign_not = self.builder.insert_binary(one, BinaryOp::Sub, sign); + let sign_not = self.builder.insert_not(sign); // We use unsafe casts here, this is fine as we're casting to a `field` type. let as_field = self.builder.insert_cast(input, Type::field()); @@ -472,7 +473,6 @@ impl<'a> FunctionContext<'a> { location: Location, ) { let is_sub = operator == BinaryOpKind::Subtract; - let one = self.builder.numeric_constant(FieldElement::one(), Type::bool()); let half_width = self.builder.numeric_constant( FieldElement::from(2_i128.pow(bit_size - 1)), Type::unsigned(bit_size), @@ -484,7 +484,7 @@ impl<'a> FunctionContext<'a> { let mut rhs_sign = self.builder.insert_binary(rhs_as_unsigned, BinaryOp::Lt, half_width); let message = if is_sub { // lhs - rhs = lhs + (-rhs) - rhs_sign = self.builder.insert_binary(one, BinaryOp::Sub, rhs_sign); + rhs_sign = self.builder.insert_not(rhs_sign); "attempt to subtract with overflow".to_string() } else { "attempt to add with overflow".to_string() @@ -518,13 +518,15 @@ impl<'a> FunctionContext<'a> { let product = self.builder.insert_cast(product_field, Type::unsigned(bit_size)); // Then we check the signed product fits in a signed integer of bit_size-bits - let not_same = self.builder.insert_binary(one, BinaryOp::Sub, same_sign); + let not_same = self.builder.insert_not(same_sign); let not_same_sign_field = self.insert_safe_cast(not_same, Type::unsigned(bit_size), location); let positive_maximum_with_offset = self.builder.insert_binary(half_width, BinaryOp::Add, not_same_sign_field); let product_overflow_check = self.builder.insert_binary(product, BinaryOp::Lt, positive_maximum_with_offset); + + let one = self.builder.numeric_constant(FieldElement::one(), Type::bool()); self.builder.set_location(location).insert_constrain( product_overflow_check, one,