From 000799244b6df18f6d3a1335d2e054c983afa0d1 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Sun, 5 Jan 2025 20:55:09 +0000 Subject: [PATCH 1/5] chore: also print test output to stdout in CI (#6930) --- .github/workflows/test-js-packages.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index 247cc87c39..5222b667c2 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -597,7 +597,7 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} run: | - out=$(nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl + nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true From f4745d4578a4aec526b3440707e5fdcb0452811f Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 11:24:57 +0000 Subject: [PATCH 2/5] feat(test): Enable the test fuzzer for Wasm (#6835) --- Cargo.lock | 34 +++------- Cargo.toml | 8 ++- tooling/fuzzer/src/dictionary/mod.rs | 3 + tooling/fuzzer/src/lib.rs | 18 ++--- tooling/fuzzer/src/strategies/int.rs | 12 ++-- tooling/fuzzer/src/strategies/mod.rs | 30 ++++---- tooling/fuzzer/src/strategies/uint.rs | 28 +++++--- tooling/nargo/Cargo.toml | 6 +- tooling/nargo/src/ops/test.rs | 98 ++++++++++++--------------- 9 files changed, 113 insertions(+), 124 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb6d4327fa..6235fbf4f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -654,18 +654,18 @@ dependencies = [ [[package]] name = "bit-set" -version = "0.5.3" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0700ddab506f33b20a03b13996eccd309a48e5ff77d0d95926aa0210fb4e95f1" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" dependencies = [ "bit-vec", ] [[package]] name = "bit-vec" -version = "0.6.3" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" [[package]] name = "bitflags" @@ -2775,12 +2775,6 @@ version = "0.2.162" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398" -[[package]] -name = "libm" -version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8355be11b20d696c8f18f6cc018c4e372165b1fa8126cef092399c9951984ffa" - [[package]] name = "libredox" version = "0.0.2" @@ -3247,7 +3241,7 @@ dependencies = [ "num-bigint", "num-traits", "proptest", - "proptest-derive 0.4.0", + "proptest-derive", "serde", "serde_json", "strum", @@ -3374,7 +3368,7 @@ dependencies = [ "num-traits", "petgraph", "proptest", - "proptest-derive 0.5.0", + "proptest-derive", "rangemap", "rustc-hash 1.1.0", "serde", @@ -3486,7 +3480,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", - "libm", ] [[package]] @@ -3868,9 +3861,9 @@ dependencies = [ [[package]] name = "proptest" -version = "1.5.0" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4c2511913b88df1637da85cc8d96ec8e43a3f8bb8ccb71ee1ac240d6f3df58d" +checksum = "14cae93065090804185d3b75f0bf93b8eeda30c7a9b4a33d3bdb3988d6229e50" dependencies = [ "bit-set", "bit-vec", @@ -3886,17 +3879,6 @@ dependencies = [ "unarray", ] -[[package]] -name = "proptest-derive" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cf16337405ca084e9c78985114633b6827711d22b9e6ef6c6c0d665eb3f0b6e" -dependencies = [ - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "proptest-derive" version = "0.5.0" diff --git a/Cargo.toml b/Cargo.toml index 73937667c1..8d48a84645 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -152,8 +152,12 @@ jsonrpsee = { version = "0.24.7", features = ["client-core"] } flate2 = "1.0.24" color-eyre = "0.6.2" rand = "0.8.5" -proptest = "1.2.0" -proptest-derive = "0.4.0" +# The `fork` and `timeout` feature doesn't compile with Wasm (wait-timeout doesn't find the `imp` module). +proptest = { version = "1.6.0", default-features = false, features = [ + "std", + "bit-set", +] } +proptest-derive = "0.5.0" rayon = "1.8.0" sha2 = { version = "0.10.6", features = ["compress"] } sha3 = "0.10.6" diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs index 172edfa54c..7fe1c4cd60 100644 --- a/tooling/fuzzer/src/dictionary/mod.rs +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -33,9 +33,11 @@ pub(super) fn build_dictionary_from_program(program: &Program) constants } +/// Collect `Field` values used in the opcodes of an ACIR circuit. fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet { let mut constants: HashSet = HashSet::new(); + /// Pull out all the fields from an expression. fn insert_expr(dictionary: &mut HashSet, expr: &Expression) { let quad_coefficients = expr.mul_terms.iter().map(|(k, _, _)| *k); let linear_coefficients = expr.linear_combinations.iter().map(|(k, _)| *k); @@ -104,6 +106,7 @@ fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet< constants } +/// Collect `Field` values used in the opcodes of a Brillig function. fn build_dictionary_from_unconstrained_function( function: &BrilligBytecode, ) -> HashSet { diff --git a/tooling/fuzzer/src/lib.rs b/tooling/fuzzer/src/lib.rs index 28a43279c9..324be323fc 100644 --- a/tooling/fuzzer/src/lib.rs +++ b/tooling/fuzzer/src/lib.rs @@ -22,7 +22,7 @@ use types::{CaseOutcome, CounterExampleOutcome, FuzzOutcome, FuzzTestResult}; use noirc_artifacts::program::ProgramArtifact; -/// An executor for Noir programs which which provides fuzzing support using [`proptest`]. +/// An executor for Noir programs which provides fuzzing support using [`proptest`]. /// /// After instantiation, calling `fuzz` will proceed to hammer the program with /// inputs, until it finds a counterexample. The provided [`TestRunner`] contains all the @@ -38,14 +38,14 @@ pub struct FuzzedExecutor { runner: TestRunner, } -impl< - E: Fn( - &Program, - WitnessMap, - ) -> Result, String>, - > FuzzedExecutor +impl FuzzedExecutor +where + E: Fn( + &Program, + WitnessMap, + ) -> Result, String>, { - /// Instantiates a fuzzed executor given a testrunner + /// Instantiates a fuzzed executor given a [TestRunner]. pub fn new(program: ProgramArtifact, executor: E, runner: TestRunner) -> Self { Self { program, executor, runner } } @@ -53,7 +53,7 @@ impl< /// Fuzzes the provided program. pub fn fuzz(&self) -> FuzzTestResult { let dictionary = build_dictionary_from_program(&self.program.bytecode); - let strategy = strategies::arb_input_map(&self.program.abi, dictionary); + let strategy = strategies::arb_input_map(&self.program.abi, &dictionary); let run_result: Result<(), TestError> = self.runner.clone().run(&strategy, |input_map| { diff --git a/tooling/fuzzer/src/strategies/int.rs b/tooling/fuzzer/src/strategies/int.rs index d11cafcfae..22dded7c7b 100644 --- a/tooling/fuzzer/src/strategies/int.rs +++ b/tooling/fuzzer/src/strategies/int.rs @@ -4,6 +4,8 @@ use proptest::{ }; use rand::Rng; +type BinarySearch = proptest::num::i128::BinarySearch; + /// Strategy for signed ints (up to i128). /// The strategy combines 2 different strategies, each assigned a specific weight: /// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` @@ -27,6 +29,7 @@ impl IntStrategy { Self { bits, edge_weight: 10usize, random_weight: 50usize } } + /// Generate random values near MIN or the MAX value. fn generate_edge_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); @@ -40,16 +43,16 @@ impl IntStrategy { 3 => self.type_max() - offset, _ => unreachable!(), }; - Ok(proptest::num::i128::BinarySearch::new(start)) + Ok(BinarySearch::new(start)) } fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); - let start: i128 = rng.gen_range(self.type_min()..=self.type_max()); - Ok(proptest::num::i128::BinarySearch::new(start)) + Ok(BinarySearch::new(start)) } + /// Maximum allowed positive number. fn type_max(&self) -> i128 { if self.bits < 128 { (1i128 << (self.bits - 1)) - 1 @@ -58,6 +61,7 @@ impl IntStrategy { } } + /// Minimum allowed negative number. fn type_min(&self) -> i128 { if self.bits < 128 { -(1i128 << (self.bits - 1)) @@ -68,7 +72,7 @@ impl IntStrategy { } impl Strategy for IntStrategy { - type Tree = proptest::num::i128::BinarySearch; + type Tree = BinarySearch; type Value = i128; fn new_tree(&self, runner: &mut TestRunner) -> NewTree { diff --git a/tooling/fuzzer/src/strategies/mod.rs b/tooling/fuzzer/src/strategies/mod.rs index 46187a28d5..99c7ca56f2 100644 --- a/tooling/fuzzer/src/strategies/mod.rs +++ b/tooling/fuzzer/src/strategies/mod.rs @@ -11,22 +11,28 @@ use uint::UintStrategy; mod int; mod uint; +/// Create a strategy for generating random values for an [AbiType]. +/// +/// Uses the `dictionary` for unsigned integer types. pub(super) fn arb_value_from_abi_type( abi_type: &AbiType, - dictionary: HashSet, + dictionary: &HashSet, ) -> SBoxedStrategy { match abi_type { AbiType::Field => vec(any::(), 32) .prop_map(|bytes| InputValue::Field(FieldElement::from_be_bytes_reduce(&bytes))) .sboxed(), AbiType::Integer { width, sign } if sign == &Sign::Unsigned => { - UintStrategy::new(*width as usize, dictionary) + // We've restricted the type system to only allow u64s as the maximum integer type. + let width = (*width).min(64); + UintStrategy::new(width as usize, dictionary) .prop_map(|uint| InputValue::Field(uint.into())) .sboxed() } AbiType::Integer { width, .. } => { - let shift = 2i128.pow(*width); - IntStrategy::new(*width as usize) + let width = (*width).min(64); + let shift = 2i128.pow(width); + IntStrategy::new(width as usize) .prop_map(move |mut int| { if int < 0 { int += shift @@ -38,7 +44,6 @@ pub(super) fn arb_value_from_abi_type( AbiType::Boolean => { any::().prop_map(|val| InputValue::Field(FieldElement::from(val))).sboxed() } - AbiType::String { length } => { // Strings only allow ASCII characters as each character must be able to be represented by a single byte. let string_regex = format!("[[:ascii:]]{{{length}}}"); @@ -53,12 +58,11 @@ pub(super) fn arb_value_from_abi_type( elements.prop_map(InputValue::Vec).sboxed() } - AbiType::Struct { fields, .. } => { let fields: Vec> = fields .iter() .map(|(name, typ)| { - (Just(name.clone()), arb_value_from_abi_type(typ, dictionary.clone())).sboxed() + (Just(name.clone()), arb_value_from_abi_type(typ, dictionary)).sboxed() }) .collect(); @@ -69,25 +73,25 @@ pub(super) fn arb_value_from_abi_type( }) .sboxed() } - AbiType::Tuple { fields } => { let fields: Vec<_> = - fields.iter().map(|typ| arb_value_from_abi_type(typ, dictionary.clone())).collect(); + fields.iter().map(|typ| arb_value_from_abi_type(typ, dictionary)).collect(); fields.prop_map(InputValue::Vec).sboxed() } } } +/// Given the [Abi] description of a [ProgramArtifact], generate random [InputValue]s for each circuit parameter. +/// +/// Use the `dictionary` to draw values from for numeric types. pub(super) fn arb_input_map( abi: &Abi, - dictionary: HashSet, + dictionary: &HashSet, ) -> BoxedStrategy { let values: Vec<_> = abi .parameters .iter() - .map(|param| { - (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ, dictionary.clone())) - }) + .map(|param| (Just(param.name.clone()), arb_value_from_abi_type(¶m.typ, dictionary))) .collect(); values diff --git a/tooling/fuzzer/src/strategies/uint.rs b/tooling/fuzzer/src/strategies/uint.rs index 94610dbc82..402e655975 100644 --- a/tooling/fuzzer/src/strategies/uint.rs +++ b/tooling/fuzzer/src/strategies/uint.rs @@ -7,6 +7,8 @@ use proptest::{ }; use rand::Rng; +type BinarySearch = proptest::num::u128::BinarySearch; + /// Value tree for unsigned ints (up to u128). /// The strategy combines 2 different strategies, each assigned a specific weight: /// 1. Generate purely random value in a range. This will first choose bit size uniformly (up `bits` @@ -14,7 +16,7 @@ use rand::Rng; /// 2. Generate a random value around the edges (+/- 3 around 0 and max possible value) #[derive(Debug)] pub struct UintStrategy { - /// Bit size of uint (e.g. 128) + /// Bit size of uint (e.g. 64) bits: usize, /// A set of fixtures to be generated fixtures: Vec, @@ -31,25 +33,29 @@ impl UintStrategy { /// # Arguments /// * `bits` - Size of uint in bits /// * `fixtures` - Set of `FieldElements` representing values which the fuzzer weight towards testing. - pub fn new(bits: usize, fixtures: HashSet) -> Self { + pub fn new(bits: usize, fixtures: &HashSet) -> Self { Self { bits, - fixtures: fixtures.into_iter().collect(), + // We can only consider the fixtures which fit into the bit width. + fixtures: fixtures.iter().filter(|f| f.num_bits() <= bits as u32).copied().collect(), edge_weight: 10usize, fixtures_weight: 40usize, random_weight: 50usize, } } + /// Generate random numbers starting from near 0 or the maximum of the range. fn generate_edge_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); // Choose if we want values around 0 or max let is_min = rng.gen_bool(0.5); let offset = rng.gen_range(0..4); let start = if is_min { offset } else { self.type_max().saturating_sub(offset) }; - Ok(proptest::num::u128::BinarySearch::new(start)) + Ok(BinarySearch::new(start)) } + /// Pick a random `FieldElement` from the `fixtures` as a starting point for + /// generating random numbers. fn generate_fixtures_tree(&self, runner: &mut TestRunner) -> NewTree { // generate random cases if there's no fixtures if self.fixtures.is_empty() { @@ -58,21 +64,19 @@ impl UintStrategy { // Generate value tree from fixture. let fixture = &self.fixtures[runner.rng().gen_range(0..self.fixtures.len())]; - if fixture.num_bits() <= self.bits as u32 { - return Ok(proptest::num::u128::BinarySearch::new(fixture.to_u128())); - } - // If fixture is not a valid type, generate random value. - self.generate_random_tree(runner) + Ok(BinarySearch::new(fixture.to_u128())) } + /// Generate random values between 0 and the MAX with the given bit width. fn generate_random_tree(&self, runner: &mut TestRunner) -> NewTree { let rng = runner.rng(); let start = rng.gen_range(0..=self.type_max()); - Ok(proptest::num::u128::BinarySearch::new(start)) + Ok(BinarySearch::new(start)) } + /// Maximum integer that fits in the given bit width. fn type_max(&self) -> u128 { if self.bits < 128 { (1 << self.bits) - 1 @@ -83,8 +87,10 @@ impl UintStrategy { } impl Strategy for UintStrategy { - type Tree = proptest::num::u128::BinarySearch; + type Tree = BinarySearch; type Value = u128; + + /// Pick randomly from the 3 available strategies for generating unsigned integers. fn new_tree(&self, runner: &mut TestRunner) -> NewTree { let total_weight = self.random_weight + self.fixtures_weight + self.edge_weight; let bias = runner.rng().gen_range(0..total_weight); diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 9fb46b78bc..4587638d69 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -28,15 +28,13 @@ tracing.workspace = true serde.workspace = true serde_json.workspace = true walkdir = "2.5.0" +noir_fuzzer = { workspace = true } +proptest = { workspace = true } # Some dependencies are optional so we can compile to Wasm. tokio = { workspace = true, optional = true } rand = { workspace = true, optional = true } -[target.'cfg(not(target_arch = "wasm32"))'.dependencies] -noir_fuzzer = { workspace = true } -proptest = { workspace = true } - [dev-dependencies] jsonrpsee = { workspace = true, features = ["server"] } diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index bfd5cd3713..a2f94cd61e 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -96,70 +96,58 @@ where status } } else { - #[cfg(target_arch = "wasm32")] - { - // We currently don't support fuzz testing on wasm32 as the u128 strategies do not exist on this platform. - TestStatus::Fail { - message: "Fuzz tests are not supported on wasm32".to_string(), - error_diagnostic: None, - } - } + use acvm::acir::circuit::Program; + use noir_fuzzer::FuzzedExecutor; + use proptest::test_runner::Config; + use proptest::test_runner::TestRunner; - #[cfg(not(target_arch = "wasm32"))] - { - use acvm::acir::circuit::Program; - use noir_fuzzer::FuzzedExecutor; - use proptest::test_runner::Config; - use proptest::test_runner::TestRunner; + let runner = + TestRunner::new(Config { failure_persistence: None, ..Config::default() }); - let runner = - TestRunner::new(Config { failure_persistence: None, ..Config::default() }); + let abi = compiled_program.abi.clone(); + let debug = compiled_program.debug.clone(); - let abi = compiled_program.abi.clone(); - let debug = compiled_program.debug.clone(); + let executor = |program: &Program, + initial_witness: WitnessMap| + -> Result, String> { + // Use a base layer that doesn't handle anything, which we handle in the `execute` below. + let inner_executor = + build_foreign_call_executor(PrintOutput::None, layers::Unhandled); - let executor = - |program: &Program, - initial_witness: WitnessMap| - -> Result, String> { - // Use a base layer that doesn't handle anything, which we handle in the `execute` below. - let inner_executor = - build_foreign_call_executor(PrintOutput::None, layers::Unhandled); - let mut foreign_call_executor = - TestForeignCallExecutor::new(inner_executor); + let mut foreign_call_executor = TestForeignCallExecutor::new(inner_executor); - let circuit_execution = execute_program( - program, - initial_witness, - blackbox_solver, - &mut foreign_call_executor, - ); + let circuit_execution = execute_program( + program, + initial_witness, + blackbox_solver, + &mut foreign_call_executor, + ); - let status = test_status_program_compile_pass( - test_function, - &abi, - &debug, - &circuit_execution, - ); + // Check if a failure was actually expected. + let status = test_status_program_compile_pass( + test_function, + &abi, + &debug, + &circuit_execution, + ); - if let TestStatus::Fail { message, error_diagnostic: _ } = status { - Err(message) - } else { - // The fuzzer doesn't care about the actual result. - Ok(WitnessStack::default()) - } - }; + if let TestStatus::Fail { message, error_diagnostic: _ } = status { + Err(message) + } else { + // The fuzzer doesn't care about the actual result. + Ok(WitnessStack::default()) + } + }; - let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); - let result = fuzzer.fuzz(); - if result.success { - TestStatus::Pass - } else { - TestStatus::Fail { - message: result.reason.unwrap_or_default(), - error_diagnostic: None, - } + let result = fuzzer.fuzz(); + if result.success { + TestStatus::Pass + } else { + TestStatus::Fail { + message: result.reason.unwrap_or_default(), + error_diagnostic: None, } } } From fd29c2f835fd7ce4bbf3face58ba31e783d4a0e3 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:59:07 +0000 Subject: [PATCH 3/5] chore: bump `noir-gates-diff` (#6949) --- .github/workflows/reports.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reports.yml b/.github/workflows/reports.yml index 9ab66c4c16..6c93156edd 100644 --- a/.github/workflows/reports.yml +++ b/.github/workflows/reports.yml @@ -76,7 +76,7 @@ jobs: - name: Compare gates reports id: gates_diff - uses: noir-lang/noir-gates-diff@60c57a6e3b67bf1553a5774ded82920150f2210e + uses: noir-lang/noir-gates-diff@7e4ddaa91c69380f15ccba514eac17bc7432a8cc with: report: gates_report.json summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) @@ -121,7 +121,7 @@ jobs: - name: Compare Brillig bytecode size reports id: brillig_bytecode_diff - uses: noir-lang/noir-gates-diff@60c57a6e3b67bf1553a5774ded82920150f2210e + uses: noir-lang/noir-gates-diff@7e4ddaa91c69380f15ccba514eac17bc7432a8cc with: report: gates_report_brillig.json header: | From bbdf1a2935f3c23b0ecf5e359626975471a31701 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:57:35 +0000 Subject: [PATCH 4/5] chore: add reproduction case for bignum test failure (#6464) --- .../regression_bignum/Nargo.toml | 7 +++ .../regression_bignum/src/main.nr | 51 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 test_programs/execution_success/regression_bignum/Nargo.toml create mode 100644 test_programs/execution_success/regression_bignum/src/main.nr diff --git a/test_programs/execution_success/regression_bignum/Nargo.toml b/test_programs/execution_success/regression_bignum/Nargo.toml new file mode 100644 index 0000000000..5cadd364e4 --- /dev/null +++ b/test_programs/execution_success/regression_bignum/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_bignum" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_bignum/src/main.nr b/test_programs/execution_success/regression_bignum/src/main.nr new file mode 100644 index 0000000000..013ab6b202 --- /dev/null +++ b/test_programs/execution_success/regression_bignum/src/main.nr @@ -0,0 +1,51 @@ +fn main() { + let numerator = + [790096867046896348, 1063071665130103641, 602707730209562162, 996751591622961462, 28650, 0]; + unsafe { __udiv_mod(numerator) }; + + let denominator = [12, 0, 0, 0, 0, 0]; + let result = unsafe { __validate_gt_remainder(denominator) }; + assert(result[4] == 0); + assert(result[5] == 0); +} + +unconstrained fn __udiv_mod(remainder_u60: [u64; 6]) { + let bit_difference = get_msb(remainder_u60); + let accumulator_u60: [u64; 6] = shl(bit_difference); +} + +unconstrained fn __validate_gt_remainder(a_u60: [u64; 6]) -> [u64; 6] { + let mut addend_u60: [u64; 6] = [0; 6]; + let mut result_u60: [u64; 6] = [0; 6]; + + for i in 0..6 { + result_u60[i] = a_u60[i] + addend_u60[i]; + } + + result_u60 +} + +unconstrained fn get_msb(val: [u64; 6]) -> u32 { + let mut count = 0; + for i in 0..6 { + let v = val[(6 - 1) - i]; + if (v > 0) { + count = 60 * ((6 - 1) - i); + break; + } + } + count +} + +unconstrained fn shl(shift: u32) -> [u64; 6] { + let num_shifted_limbs = shift / 60; + let limb_shift = (shift % 60) as u8; + + let mut result = [0; 6]; + result[num_shifted_limbs] = (1 << limb_shift); + + for i in 1..(6 - num_shifted_limbs) { + result[i + num_shifted_limbs] = 0; + } + result +} From da94c2b45fd43cbd6a5f2ba15b665dcb3a5e33a5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 11:51:55 -0300 Subject: [PATCH 5/5] feat: don't simplify SSA instructions when creating them from a string (#6948) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../src/ssa/function_builder/mod.rs | 26 ++++++++++++++----- .../src/ssa/ir/instruction/call.rs | 2 +- .../src/ssa/ir/instruction/call/blackbox.rs | 6 ++--- .../src/ssa/opt/constant_folding.rs | 2 +- .../src/ssa/opt/flatten_cfg.rs | 1 + .../src/ssa/opt/normalize_value_ids.rs | 13 +++++----- .../src/ssa/parser/into_ssa.rs | 11 ++++---- .../noirc_evaluator/src/ssa/parser/mod.rs | 18 +++++++++---- .../noirc_evaluator/src/ssa/parser/tests.rs | 12 +++++++++ 9 files changed, 64 insertions(+), 27 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 28cf48bb17..2be8e571cd 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -40,6 +40,10 @@ pub(crate) struct FunctionBuilder { finished_functions: Vec, call_stack: CallStackId, error_types: BTreeMap, + + /// Whether instructions are simplified as soon as they are inserted into this builder. + /// This is true by default unless changed to false after constructing a builder. + pub(crate) simplify: bool, } impl FunctionBuilder { @@ -56,6 +60,7 @@ impl FunctionBuilder { finished_functions: Vec::new(), call_stack: CallStackId::root(), error_types: BTreeMap::default(), + simplify: true, } } @@ -172,12 +177,21 @@ impl FunctionBuilder { ctrl_typevars: Option>, ) -> InsertInstructionResult { let block = self.current_block(); - self.current_function.dfg.insert_instruction_and_results( - instruction, - block, - ctrl_typevars, - self.call_stack, - ) + if self.simplify { + self.current_function.dfg.insert_instruction_and_results( + instruction, + block, + ctrl_typevars, + self.call_stack, + ) + } else { + self.current_function.dfg.insert_instruction_and_results_without_simplification( + instruction, + block, + ctrl_typevars, + self.call_stack, + ) + } } /// Switch to inserting instructions in the given block. diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index b6b1ac3f06..b0e5506e8d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -730,7 +730,7 @@ mod tests { return v2 } "#; - let ssa = Ssa::from_str(src).unwrap(); + let ssa = Ssa::from_str_simplifying(src).unwrap(); let expected = r#" brillig(inline) fn main f0 { diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index ffacf6fe8b..fac2e8b4d5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -323,7 +323,7 @@ mod test { v2 = call multi_scalar_mul (v1, v0) -> [Field; 3] return v2 }"#; - let ssa = Ssa::from_str(src).unwrap(); + let ssa = Ssa::from_str_simplifying(src).unwrap(); let expected_src = r#" acir(inline) fn main f0 { @@ -351,7 +351,7 @@ mod test { return v4 }"#; - let ssa = Ssa::from_str(src).unwrap(); + let ssa = Ssa::from_str_simplifying(src).unwrap(); //First point is zero, second scalar is zero, so we should be left with the scalar mul of the last point. let expected_src = r#" acir(inline) fn main f0 { @@ -379,7 +379,7 @@ mod test { v4 = call multi_scalar_mul (v3, v2) -> [Field; 3] return v4 }"#; - let ssa = Ssa::from_str(src).unwrap(); + let ssa = Ssa::from_str_simplifying(src).unwrap(); //First and last scalar/point are constant, so we should be left with the msm of the middle point and the folded constant point let expected_src = r#" acir(inline) fn main f0 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index c81a557178..e3b8c6e99a 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1045,7 +1045,7 @@ mod test { return } "; - let ssa = Ssa::from_str(src).unwrap(); + let ssa = Ssa::from_str_simplifying(src).unwrap(); let expected = " acir(inline) fn main f0 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 16abf03eec..748867c740 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1081,6 +1081,7 @@ mod test { v23 = mul v20, Field 6 v24 = mul v21, v16 v25 = add v23, v24 + enable_side_effects v0 enable_side_effects v3 v26 = cast v3 as Field v27 = cast v0 as Field diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index 63ca523bd5..7f5352155d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -96,12 +96,13 @@ impl Context { .requires_ctrl_typevars() .then(|| vecmap(old_results, |result| old_function.dfg.type_of_value(*result))); - let new_results = new_function.dfg.insert_instruction_and_results( - instruction, - new_block_id, - ctrl_typevars, - new_call_stack, - ); + let new_results = + new_function.dfg.insert_instruction_and_results_without_simplification( + instruction, + new_block_id, + ctrl_typevars, + new_call_stack, + ); assert_eq!(old_results.len(), new_results.len()); for (old_result, new_result) in old_results.iter().zip(new_results.results().iter()) diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 7c7e977c6c..afada8c0e2 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -16,8 +16,8 @@ use super::{ }; impl ParsedSsa { - pub(crate) fn into_ssa(self) -> Result { - Translator::translate(self) + pub(crate) fn into_ssa(self, simplify: bool) -> Result { + Translator::translate(self, simplify) } } @@ -41,8 +41,8 @@ struct Translator { } impl Translator { - fn translate(mut parsed_ssa: ParsedSsa) -> Result { - let mut translator = Self::new(&mut parsed_ssa)?; + fn translate(mut parsed_ssa: ParsedSsa, simplify: bool) -> Result { + let mut translator = Self::new(&mut parsed_ssa, simplify)?; // Note that the `new` call above removed the main function, // so all we are left with are non-main functions. @@ -53,12 +53,13 @@ impl Translator { Ok(translator.finish()) } - fn new(parsed_ssa: &mut ParsedSsa) -> Result { + fn new(parsed_ssa: &mut ParsedSsa, simplify: bool) -> Result { // A FunctionBuilder must be created with a main Function, so here wer remove it // from the parsed SSA to avoid adding it twice later on. let main_function = parsed_ssa.functions.remove(0); let main_id = FunctionId::test_new(0); let mut builder = FunctionBuilder::new(main_function.external_name.clone(), main_id); + builder.simplify = simplify; builder.set_runtime(main_function.runtime_type); // Map function names to their IDs so calls can be resolved diff --git a/compiler/noirc_evaluator/src/ssa/parser/mod.rs b/compiler/noirc_evaluator/src/ssa/parser/mod.rs index b400c28875..96aef1f3c3 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/mod.rs @@ -32,16 +32,24 @@ mod token; impl Ssa { /// Creates an Ssa object from the given string. - /// - /// Note that the resulting Ssa might not be exactly the same as the given string. - /// This is because, internally, the Ssa is built using a `FunctionBuilder`, so - /// some instructions might be simplified while they are inserted. pub(crate) fn from_str(src: &str) -> Result { + Self::from_str_impl(src, false) + } + + /// Creates an Ssa object from the given string but trying to simplify + /// each parsed instruction as it's inserted into the final SSA. + pub(crate) fn from_str_simplifying(src: &str) -> Result { + Self::from_str_impl(src, true) + } + + fn from_str_impl(src: &str, simplify: bool) -> Result { let mut parser = Parser::new(src).map_err(|err| SsaErrorWithSource::parse_error(err, src))?; let parsed_ssa = parser.parse_ssa().map_err(|err| SsaErrorWithSource::parse_error(err, src))?; - parsed_ssa.into_ssa().map_err(|error| SsaErrorWithSource { src: src.to_string(), error }) + parsed_ssa + .into_ssa(simplify) + .map_err(|error| SsaErrorWithSource { src: src.to_string(), error }) } } diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index dab96dfa04..899a5a571e 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -502,3 +502,15 @@ fn test_function_type() { "; assert_ssa_roundtrip(src); } + +#[test] +fn test_does_not_simplify() { + let src = " + acir(inline) fn main f0 { + b0(): + v2 = add Field 1, Field 2 + return v2 + } + "; + assert_ssa_roundtrip(src); +}