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

chore(ssa refactor): Add code to handle left and right shifts #1471

Merged
merged 9 commits into from
Jun 1, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[package]
authors = [""]
compiler_version = "0.1"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "3"
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Tests a very simple program.
//
// The features being tested are left and right shifts.
fn main(x : u32) {
let z = x >> 4;
let t = x << 4;
assert(z == t >> 8);
}
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,54 @@ impl AcirContext {
}
}
}
/// Returns an `AcirVar` that is constrained to be lhs << rhs.
///
/// We convert left shifts to multiplications, so this is equivalent to
/// lhs * 2^rhs.
///
/// TODO: Currently, we maintain that the rhs needs to be a constant so that we can
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
/// TODO: compute 2^rhs in the compiler. However, we can extend it to witnesses
/// TODO: by first bit decomposing rhs and then using square-and-multiply to
/// TODO: compute 2^{rhs}. This will require if statements so it would be good
/// TODO: to have this implementation in Noir and the << operator uses that.
pub(crate) fn shift_left_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> AcirVar {
let rhs_data = &self.data[&rhs];

// Compute 2^{rhs}
let two_pow_rhs = match rhs_data.as_constant() {
Some(exponent) => FieldElement::from(2_i128).pow(&exponent),
None => unimplemented!("rhs must be a constant when doing a right shift"),
};
let two_pow_rhs_var = self.add_constant(two_pow_rhs);

self.mul_var(lhs, two_pow_rhs_var)
}

/// Returns an `AcirVar` that is constrained to be lhs >> rhs.
///
/// We convert right shifts to divisions, so this is equivalent to
/// lhs / 2^rhs.
///
/// TODO: the previous code seems to have only done a right shift, if the
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
/// TODO rhs was a constant. Moreover, it did an unsigned division.
///
/// TODO: We should rationalize whether we want shift left and shift right to work
/// TODO only for unsigned integers.
/// TODO:
/// TODO: This would mean that both implementation would be doing integer mul and
/// TODO integer division. The previous code was doing Mul and integer division.
pub(crate) fn shift_right_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> AcirVar {
let rhs_data = &self.data[&rhs];

// Compute 2^{rhs}
let two_pow_rhs = match rhs_data.as_constant() {
Some(exponent) => FieldElement::from(2_i128).pow(&exponent),
None => unimplemented!("rhs must be a constant when doing a right shift"),
};
let two_pow_rhs_var = self.add_constant(two_pow_rhs);

self.div_var(lhs, two_pow_rhs_var)
}

/// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the
/// `GeneratedAcir`'s return witnesses.
Expand Down
2 changes: 2 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ impl Context {
.acir_context
.less_than_var(lhs, rhs)
.expect("add Result types to all methods so errors bubble up"),
BinaryOp::Shl => self.acir_context.shift_left_var(lhs, rhs),
BinaryOp::Shr => self.acir_context.shift_right_var(lhs, rhs),
_ => todo!(),
}
}
Expand Down