Skip to content

Commit

Permalink
fix: SSA typing for right shifts (#4302)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Partial work towards #4275

## Summary\*

Previously, SSA was issuing (lhs_type, Field) DIV for right shifts.
However this is wrong if we support shifts by the bit size (`u1 >> 1`)

This is interpreted in acir_gen as an euclidean division with bit_size =
lhs_type.bit_size(). However, u1 >> 1 worked because of a + 1 that is in
euclidean division
https://github.com/noir-lang/noir/blob/0e073037b00212fd17fc8ca9c531b567614eb4c5/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs#L703
(and it failed in unconstrained functions since they don't codegen that
extraneous + 1)

## 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.

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>
  • Loading branch information
sirasistant and TomAFrench authored Feb 8, 2024
1 parent 7420dbb commit 41ee1aa
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 1 deletion.
9 changes: 8 additions & 1 deletion compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,18 @@ impl FunctionBuilder {
rhs: ValueId,
bit_size: u32,
) -> ValueId {
let lhs_typ = self.type_of_value(lhs);
let base = self.field_constant(FieldElement::from(2_u128));
// we can safely cast to unsigned because overflow_checks prevent bit-shift with a negative value
let rhs_unsigned = self.insert_cast(rhs, Type::unsigned(bit_size));
let pow = self.pow(base, rhs_unsigned);
self.insert_binary(lhs, BinaryOp::Div, pow)
// We need at least one more bit for the case where rhs == bit_size
let div_type = Type::unsigned(bit_size + 1);
let casted_lhs = self.insert_cast(lhs, div_type.clone());
let casted_pow = self.insert_cast(pow, div_type);
let div_result = self.insert_binary(casted_lhs, BinaryOp::Div, casted_pow);
// We have to cast back to the original type
self.insert_cast(div_result, lhs_typ)
}

/// Computes lhs^rhs via square&multiply, using the bits decomposition of rhs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ fn main(x: u64, y: u64) {
assert(a << 7 == -128);
assert(a << -a == -2);

assert(x >> x == 0);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "brillig_bit_shifts_runtime"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 64
y = 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
unconstrained fn main(x: u64, y: u64) {
// runtime shifts on compile-time known values
assert(64 << y == 128);
assert(64 >> y == 32);
// runtime shifts on runtime values
assert(x << y == 128);
assert(x >> y == 32);

// Bit-shift with signed integers
let mut a :i8 = y as i8;
let mut b: i8 = x as i8;
assert(b << 1 == -128);
assert(b >> 2 == 16);
assert(b >> a == 32);
a = -a;
assert(a << 7 == -128);
assert(a << -a == -2);

assert(x >> x == 0);
}

0 comments on commit 41ee1aa

Please sign in to comment.