diff --git a/Cargo.lock b/Cargo.lock index 960ad27dbcb..379d6298846 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "acir" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a8e08ddd57e123075a643f9515f712c34ebcfd6333628a4e8972af41f21a694" +checksum = "269617cfc7050d47915308c2ae20c33642cbfbc5c25097ba10a522539556065b" dependencies = [ "acir_field", "bincode", @@ -18,9 +18,9 @@ dependencies = [ [[package]] name = "acir_field" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "729849a0f1ca22f267c83b4d04043f6fa44f159a6606dc58e586c131c67ab774" +checksum = "d0ace9d882b85479aa678c7272aa717acdcabe1f4d0cb6236cd17dcda8c4bb32" dependencies = [ "ark-bn254", "ark-ff", @@ -32,9 +32,9 @@ dependencies = [ [[package]] name = "acvm" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f5f1c272779208fe823fe2a327d5e85efb6b8861e858507df4d532ec372514ee" +checksum = "f5eeabcd9c38133eb860c1ad4c0ced8d0032f9f22b6de0699bddacb69bd772ec" dependencies = [ "acir", "acvm_blackbox_solver", @@ -70,9 +70,9 @@ dependencies = [ [[package]] name = "acvm_blackbox_solver" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ee8080267f367360522f24a5cd0de2bf7b9d31795266471c69e352083ceda39" +checksum = "53e3636cfc0b6a6e6e02a5b813b12941ef0979ec532290ee83587f42d7610dae" dependencies = [ "acir", "blake2", @@ -85,9 +85,9 @@ dependencies = [ [[package]] name = "acvm_stdlib" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e4b52ef8c927058b451576fa7ab7e173c2b302ee81a0337d2420d3ca01c71d5e" +checksum = "87fc3882d621cb13af2793e35fff3aa8ba34959c486808e665212d5423835386" dependencies = [ "acir", ] @@ -533,9 +533,9 @@ dependencies = [ [[package]] name = "brillig" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30647da3d1347c3f3a3c0e955f75b1fd3a91c123ee87d6b55d94c042d68e2ae6" +checksum = "23339a18ea207882da08076a70564b5a44bad80ce2ebb3a46e89172979ae09fb" dependencies = [ "acir_field", "serde", @@ -543,9 +543,9 @@ dependencies = [ [[package]] name = "brillig_vm" -version = "0.19.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0dc5c4ce5b6553b4d48f9895ca6b336a3672f1dc802792a37f13dcb8902d47b5" +checksum = "024f52eb86d8f320cbbf8c2f231acdd84fe2171d3d1926c4bc95d64e933aa23e" dependencies = [ "acir", "acvm_blackbox_solver", diff --git a/Cargo.toml b/Cargo.toml index 42028afd18f..2dde6ff67c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ edition = "2021" rust-version = "1.66" [workspace.dependencies] -acvm = "0.19.0" +acvm = "0.19.1" arena = { path = "crates/arena" } fm = { path = "crates/fm" } iter-extended = { path = "crates/iter-extended" } @@ -40,7 +40,7 @@ noir_wasm = { path = "crates/wasm" } cfg-if = "1.0.0" clap = { version = "4.1.4", features = ["derive"] } -codespan = {version = "0.11.1", features = ["serialization"]} +codespan = { version = "0.11.1", features = ["serialization"] } codespan-lsp = "0.11.1" codespan-reporting = "0.11.1" chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" } diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml new file mode 100644 index 00000000000..baad8be126a --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/Prover.toml @@ -0,0 +1,38 @@ +c=[2, 4, 3, 0, ] +a=0 +x = [104, 101, 108, 108, 111] + +result = [ + 0x2c, + 0xf2, + 0x4d, + 0xba, + 0x5f, + 0xb0, + 0xa3, + 0x0e, + 0x26, + 0xe8, + 0x3b, + 0x2a, + 0xc5, + 0xb9, + 0xe2, + 0x9e, + 0x1b, + 0x16, + 0x1e, + 0x5c, + 0x1f, + 0xa7, + 0x42, + 0x5e, + 0x73, + 0x04, + 0x33, + 0x62, + 0x93, + 0x8b, + 0x98, + 0x24, +] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr new file mode 100644 index 00000000000..48ac639ecf0 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/9_conditional/src/main.nr @@ -0,0 +1,277 @@ +use dep::std; + +fn sort(mut a: [u32; 4]) -> [u32; 4] { + for i in 1..4 { + for j in 0..i { + if a[i] < a[j] { + let c = a[j]; + a[j] = a[i]; + a[i] = c; + } + } + } + a +} + +fn call_intrinsic(x: [u8; 5], result: [u8; 32]) { + let mut digest = std::hash::sha256(x); + digest[0] = 5 as u8; + digest = std::hash::sha256(x); + assert(digest == result); +} + +fn must_be_zero(x: u8) { + assert(x == 0); +} + +fn test3 (x: u8) { + if x == 0 { + must_be_zero(x); + } +} + +fn test4() -> [u32; 4] { + let b: [u32; 4] = [1,2,3,4]; + b +} + +fn main(a: u32, mut c: [u32; 4], x: [u8; 5], result: pub [u8; 32]){ + // Regression test for issue #547 + // Warning: it must be kept at the start of main + let arr: [u8; 2] = [1, 2]; + if arr[0] != arr[1] { + for i in 0..1 { + assert(i != 2); + } + } + + //Issue reported in #421 + if a == c[0] { + assert(c[0] == 0); + } else { + if a == c[1] { + assert(c[1] == 0); + } else { + if a == c[2] { + assert(c[2] == 0); + } + } + } + + //Regression for to_le_bits() constant evaluation + // binary array representation of u8 1 + let as_bits_hardcode_1 = [1, 0]; + let mut c1 = 0; + for i in 0..2 { + let mut as_bits = (arr[i] as Field).to_le_bits(2); + c1 = c1 + as_bits[0] as Field; + + if i == 0 { + assert(arr[i] == 1);// 1 + for k in 0..2 { + assert(as_bits_hardcode_1[k] == as_bits[k]); + } + } + if i == 1 { + assert(arr[i] == 2);//2 + for k in 0..2 { + assert(as_bits_hardcode_1[k] != as_bits[k]); + } + } + } + assert(c1==1); + + //Regression for Issue #579 + let result1_true = test(true); + assert(result1_true.array_param[0] == 1); + let result1_false = test(false); + assert(result1_false.array_param[0] == 0); + + //Test case for short-circuit + let mut data = [0 as u32; 32]; + let mut ba = a; + for i in 0..32 { + let i_u32 = i as u32; + if i_u32 == a { + for j in 0..4 { + data[i + j] = c[4 - 1 - j]; + for k in 0..4 { + ba = ba +data[k]; + } + if ba == 4864 { + c[3]=ba; + } + } + } + } + assert(data[31] == 0); + assert(ba != 13); + //regression for short-circuit2 + if 35 == a { + assert(false); + } + bar(a as Field); + + if a == 3 { + c = test4(); + } + assert(c[1] != 2); + call_intrinsic(x, result); + + //Test case for conditional with arrays from function parameters + let b = sort([1,2,3,4]); + assert(b[0] == 1); + + if a == 0 { + must_be_zero(0); + c[0] = 3; + } else { + must_be_zero(1); + c[0] = 1; + c[1] = c[2] / a + 11 % a; + let f1 = a as Field; + assert(10/f1 != 0); + } + assert(c[0] == 3); + + let mut y = 0; + if a == 0 { + let digest = std::hash::sha256(x); + y = digest[0]; + } else { + y = 5; + } + assert(y == result[0]); + c = sort(c); + assert(c[0]==0); + + //test 1 + let mut x: u32 = 0; + if a == 0 { + c[0] = 12; + if a != 0 { + x = 6; + } else { + x = 2; + assert(x == 2); + } + } else { + x = 5; + assert(x == 5); + } + if c[0] == 0 { + x = 3; + } + assert(x == 2); + + //test2: loops! + x = 0; + x = a - a; + for i in 0..4 { + if c[i] == 0 { + x = i as u32 +2; + } + } + assert(x == 0); + + test3(1); + + if a == 0 { + c = test4(); + } else { + assert(c[1] != 2); + } + if false { + c[1] = 5; + } + assert(c[1] == 2); + + test5(4); + + // Regression for issue #661: + let mut c_661 :[u32;1]=[0]; + if a > 5 { + c_661 = issue_661_foo(issue_661_bar(c), a); + } else { + c_661 = issue_661_foo(issue_661_bar(c), x); + } + assert(c_661[0] < 20000); + + // Test case for function synchronisation + let mut c_sync = 0; + if a == 42 { + c_sync = foo2(); + } else { + c_sync = foo2() + foo2(); + } + assert(c_sync == 6); + + // Regression for predicate simplification + safe_inverse(0); +} + +fn test5(a : u32) { + if a > 1 { + let q = a / 2; + assert(q == 2); + } +} + + + +fn foo() { + let mut x = 1; + x /= 0; +} + +fn bar(x:Field) { + if x == 15 { + foo(); + } +} + + +struct MyStruct579 { + array_param: [u32; 2] +} + +impl MyStruct579 { + fn new(array_param: [u32; 2]) -> MyStruct579 { + MyStruct579 { + array_param: array_param + } + } +} + +fn test(flag: bool) -> MyStruct579 { + let mut my_struct = MyStruct579::new([0; 2]); + + if flag == true { + my_struct= MyStruct579::new([1; 2]); + } + my_struct +} + +fn issue_661_foo(array: [u32;4], b:u32) ->[u32;1] { + [array[0]+b] +} + +fn issue_661_bar(a : [u32;4]) ->[u32;4] { + let mut b:[u32;4] = [0;4]; + b[0]=a[0]+1; + b +} + +fn foo2() -> Field { + 3 +} + +fn safe_inverse(n: Field) -> Field +{ + if n == 0 { + 0 + } + else { + 1 / n + } +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs index a1fb66ab749..61c3386ccbd 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/acir_variable.rs @@ -156,7 +156,11 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the inverse of `var`. - pub(crate) fn inv_var(&mut self, var: AcirVar) -> Result { + pub(crate) fn inv_var( + &mut self, + var: AcirVar, + predicate: AcirVar, + ) -> Result { let var_data = &self.vars[&var]; if let AcirVarData::Const(constant) = var_data { // Note that this will return a 0 if the inverse is not available @@ -169,7 +173,7 @@ impl AcirContext { let field_type = AcirType::NumericType(NumericType::NativeField); let results = self.brillig( - None, + Some(predicate), inverse_code, vec![AcirValue::Var(var, field_type.clone())], vec![field_type], @@ -177,15 +181,28 @@ impl AcirContext { let inverted_var = Self::expect_one_var(results); let should_be_one = self.mul_var(inverted_var, var)?; - self.assert_eq_one(should_be_one)?; + self.maybe_eq_predicate(should_be_one, predicate)?; Ok(inverted_var) } - /// Constrains the lhs to be equal to the constant value `1` + // Constrains `var` to be equal to the constant value `1` pub(crate) fn assert_eq_one(&mut self, var: AcirVar) -> Result<(), AcirGenError> { - let one_var = self.add_constant(FieldElement::one()); - self.assert_eq_var(var, one_var) + let one = self.add_constant(FieldElement::one()); + self.assert_eq_var(var, one) + } + + // Constrains `var` to be equal to predicate if the predicate is true + // or to be equal to 0 if the predicate is false. + // + // Since we multiply `var` by the predicate, this is a no-op if the predicate is false + pub(crate) fn maybe_eq_predicate( + &mut self, + var: AcirVar, + predicate: AcirVar, + ) -> Result<(), AcirGenError> { + let pred_mul_var = self.mul_var(var, predicate)?; + self.assert_eq_var(pred_mul_var, predicate) } // Returns the variable from the results, assuming it is the only result @@ -296,6 +313,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, + predicate: AcirVar, ) -> Result { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, @@ -305,12 +323,12 @@ impl AcirContext { }; match numeric_type { NumericType::NativeField => { - let inv_rhs = self.inv_var(rhs)?; + let inv_rhs = self.inv_var(rhs, predicate)?; self.mul_var(lhs, inv_rhs) } NumericType::Unsigned { bit_size } => { let (quotient_var, _remainder_var) = - self.euclidean_division_var(lhs, rhs, bit_size)?; + self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(quotient_var) } NumericType::Signed { bit_size } => { @@ -430,17 +448,18 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, + predicate: AcirVar, ) -> Result<(AcirVar, AcirVar), AcirGenError> { - let predicate = Expression::one(); - let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; + let predicate_data = &self.vars[&predicate]; let lhs_expr = lhs_data.to_expression(); let rhs_expr = rhs_data.to_expression(); + let predicate_expr = predicate_data.to_expression(); let (quotient, remainder) = - self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, bit_size, &predicate)?; + self.acir_ir.euclidean_division(&lhs_expr, &rhs_expr, bit_size, &predicate_expr)?; let quotient_var = self.add_data(AcirVarData::Witness(quotient)); let remainder_var = self.add_data(AcirVarData::Witness(remainder)); @@ -485,8 +504,9 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, + predicate: AcirVar, ) -> Result { - let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size)?; + let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder) } @@ -505,6 +525,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, + predicate: AcirVar, ) -> Result { let rhs_data = &self.vars[&rhs]; @@ -515,7 +536,7 @@ impl AcirContext { }; let two_pow_rhs_var = self.add_constant(two_pow_rhs); - self.div_var(lhs, two_pow_rhs_var, typ) + self.div_var(lhs, two_pow_rhs_var, typ, predicate) } /// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the @@ -575,7 +596,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - predicate: Option, + predicate: AcirVar, ) -> Result { let lhs_data = &self.vars[&lhs]; let rhs_data = &self.vars[&rhs]; @@ -586,10 +607,9 @@ impl AcirContext { // TODO: check what happens when we do (a as u8) >= (b as u32) // TODO: The frontend should shout in this case - let predicate = predicate.map(|acir_var| { - let predicate_data = &self.vars[&acir_var]; - predicate_data.to_expression().into_owned() - }); + let predicate_data = &self.vars[&predicate]; + let predicate = predicate_data.to_expression().into_owned(); + let is_greater_than_eq = self.acir_ir.more_than_eq_comparison(&lhs_expr, &rhs_expr, bit_size, predicate)?; @@ -607,7 +627,7 @@ impl AcirContext { ) -> Result { // Flip the result of calling more than equal method to // compute less than. - let comparison = self.more_than_eq_var(lhs, rhs, bit_size, Some(predicate))?; + let comparison = self.more_than_eq_var(lhs, rhs, bit_size, predicate)?; let one = self.add_constant(FieldElement::one()); self.sub_var(one, comparison) // comparison_negated @@ -858,6 +878,7 @@ impl AcirContext { &mut self, inputs: Vec, bit_size: u32, + predicate: AcirVar, ) -> Result, AcirGenError> { let len = inputs.len(); // Convert the inputs into expressions @@ -875,7 +896,7 @@ impl AcirContext { // Enforce the outputs to be sorted for i in 0..(outputs_var.len() - 1) { - self.less_than_constrain(outputs_var[i], outputs_var[i + 1], bit_size, None)?; + self.less_than_constrain(outputs_var[i], outputs_var[i + 1], bit_size, predicate)?; } Ok(outputs_var) @@ -887,10 +908,10 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - predicate: Option, + predicate: AcirVar, ) -> Result<(), AcirGenError> { let lhs_less_than_rhs = self.more_than_eq_var(rhs, lhs, bit_size, predicate)?; - self.assert_eq_one(lhs_less_than_rhs) + self.maybe_eq_predicate(lhs_less_than_rhs, predicate) } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs index 16e07c29730..5b05b1a0c1d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs @@ -428,11 +428,22 @@ impl GeneratedAcir { // When the predicate is 0, the equation always passes. // When the predicate is 1, the euclidean division needs to be // true. - let mut rhs_constraint = (rhs * &Expression::from(q_witness)).unwrap(); + let rhs_reduced: Expression = self.create_witness_for_expression(rhs).into(); + let mut rhs_constraint = (&rhs_reduced * &Expression::from(q_witness)) + .expect("rhs_reduced is expected to be a degree-1 witness"); rhs_constraint = &rhs_constraint + r_witness; - rhs_constraint = (&rhs_constraint * predicate).unwrap(); - let lhs_constraint = (lhs * predicate).unwrap(); - let div_euclidean = &lhs_constraint - &rhs_constraint; + + // Reduce the rhs_constraint to a witness + let rhs_constrain_reduced: Expression = + self.create_witness_for_expression(&rhs_constraint).into(); + // Reduce the lhs_constraint to a witness + let lhs_reduced: Expression = self.create_witness_for_expression(lhs).into(); + + let div_euclidean = &(&lhs_reduced * predicate).expect( + "lhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", + ) - &(&rhs_constrain_reduced * predicate).expect( + "rhs_reduced should be a degree-1 witness and predicate should be a degree-1 witness", + ); self.push_opcode(AcirOpcode::Arithmetic(div_euclidean)); @@ -711,7 +722,7 @@ impl GeneratedAcir { a: &Expression, b: &Expression, max_bits: u32, - predicate: Option, + predicate: Expression, ) -> Result { // Ensure that 2^{max_bits + 1} is less than the field size // @@ -737,7 +748,7 @@ impl GeneratedAcir { let (q_witness, r_witness) = self.quotient_directive( comparison_evaluation.clone(), two_max_bits.into(), - predicate, + Some(predicate), q_max_bits, r_max_bits, )?; diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 72194828f5c..0e665466d42 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -548,7 +548,12 @@ impl Context { BinaryOp::Add => self.acir_context.add_var(lhs, rhs), BinaryOp::Sub => self.acir_context.sub_var(lhs, rhs), BinaryOp::Mul => self.acir_context.mul_var(lhs, rhs), - BinaryOp::Div => self.acir_context.div_var(lhs, rhs, binary_type), + BinaryOp::Div => self.acir_context.div_var( + lhs, + rhs, + binary_type, + self.current_side_effects_enabled_var, + ), // Note: that this produces unnecessary constraints when // this Eq instruction is being used for a constrain statement BinaryOp::Eq => self.acir_context.eq_var(lhs, rhs), @@ -559,11 +564,21 @@ impl Context { self.current_side_effects_enabled_var, ), BinaryOp::Shl => self.acir_context.shift_left_var(lhs, rhs, binary_type), - BinaryOp::Shr => self.acir_context.shift_right_var(lhs, rhs, binary_type), + BinaryOp::Shr => self.acir_context.shift_right_var( + lhs, + rhs, + binary_type, + self.current_side_effects_enabled_var, + ), BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, binary_type), BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type), BinaryOp::Or => self.acir_context.or_var(lhs, rhs, binary_type), - BinaryOp::Mod => self.acir_context.modulo_var(lhs, rhs, bit_count), + BinaryOp::Mod => self.acir_context.modulo_var( + lhs, + rhs, + bit_count, + self.current_side_effects_enabled_var, + ), } } @@ -739,8 +754,10 @@ impl Context { } } // Generate the sorted output variables - let out_vars = - self.acir_context.sort(input_vars, bit_size).expect("Could not sort"); + let out_vars = self + .acir_context + .sort(input_vars, bit_size, self.current_side_effects_enabled_var) + .expect("Could not sort"); Ok(Self::convert_vars_to_values(out_vars, dfg, result_ids)) }