From 466793a5320a8b42dbe365225d7e123781d26cdc Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 8 Feb 2024 11:50:18 +0000 Subject: [PATCH 1/3] fix: SSA type for right shifts --- compiler/noirc_evaluator/src/ssa/function_builder/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 989249d04ef..4386d7234c2 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -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_rhs = self.insert_cast(pow, div_type); + let div_result = self.insert_binary(casted_lhs, BinaryOp::Div, casted_rhs); + // 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 From b4fe4813fa358402fbd7a502a227adee270646ba Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 8 Feb 2024 12:29:31 +0000 Subject: [PATCH 2/3] style: rename --- compiler/noirc_evaluator/src/ssa/function_builder/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 4386d7234c2..f3f96cad637 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -344,8 +344,8 @@ impl FunctionBuilder { // 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_rhs = self.insert_cast(pow, div_type); - let div_result = self.insert_binary(casted_lhs, BinaryOp::Div, casted_rhs); + 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) } From 6a6217831bdb3de10f2c37f20823ba44344ebf30 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 8 Feb 2024 13:50:24 +0000 Subject: [PATCH 3/3] chore: add regression test for rightshifting a value by its bitsize --- .../bit_shifts_runtime/src/main.nr | 1 + .../brillig_bit_shifts_runtime/Nargo.toml | 6 ++++++ .../brillig_bit_shifts_runtime/Prover.toml | 2 ++ .../brillig_bit_shifts_runtime/src/main.nr | 20 +++++++++++++++++++ 4 files changed, 29 insertions(+) create mode 100644 test_programs/execution_success/brillig_bit_shifts_runtime/Nargo.toml create mode 100644 test_programs/execution_success/brillig_bit_shifts_runtime/Prover.toml create mode 100644 test_programs/execution_success/brillig_bit_shifts_runtime/src/main.nr diff --git a/test_programs/execution_success/bit_shifts_runtime/src/main.nr b/test_programs/execution_success/bit_shifts_runtime/src/main.nr index 33d68765598..28b3ef656c1 100644 --- a/test_programs/execution_success/bit_shifts_runtime/src/main.nr +++ b/test_programs/execution_success/bit_shifts_runtime/src/main.nr @@ -16,4 +16,5 @@ fn main(x: u64, y: u64) { assert(a << 7 == -128); assert(a << -a == -2); + assert(x >> x == 0); } diff --git a/test_programs/execution_success/brillig_bit_shifts_runtime/Nargo.toml b/test_programs/execution_success/brillig_bit_shifts_runtime/Nargo.toml new file mode 100644 index 00000000000..ed8200d8a95 --- /dev/null +++ b/test_programs/execution_success/brillig_bit_shifts_runtime/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "brillig_bit_shifts_runtime" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/brillig_bit_shifts_runtime/Prover.toml b/test_programs/execution_success/brillig_bit_shifts_runtime/Prover.toml new file mode 100644 index 00000000000..98d8630792e --- /dev/null +++ b/test_programs/execution_success/brillig_bit_shifts_runtime/Prover.toml @@ -0,0 +1,2 @@ +x = 64 +y = 1 \ No newline at end of file diff --git a/test_programs/execution_success/brillig_bit_shifts_runtime/src/main.nr b/test_programs/execution_success/brillig_bit_shifts_runtime/src/main.nr new file mode 100644 index 00000000000..f22166b5993 --- /dev/null +++ b/test_programs/execution_success/brillig_bit_shifts_runtime/src/main.nr @@ -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); +}