Skip to content

Commit

Permalink
chore: replace explicit subtractions with nots (#4097)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## 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.
  • Loading branch information
TomAFrench authored Jan 22, 2024
1 parent 455399f commit 199d129
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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),
Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 199d129

Please sign in to comment.