From 88c2f3d72ef7b1b4df14d6a4dd5707b60cd7466c Mon Sep 17 00:00:00 2001 From: AztecBot Date: Sun, 24 Nov 2024 08:03:03 +0000 Subject: [PATCH 1/2] [1 changes] feat: Add `array_refcount` and `slice_refcount` builtins for debugging (https://github.com/noir-lang/noir/pull/6584) chore!: Require types of globals to be specified (https://github.com/noir-lang/noir/pull/6592) fix: don't report visibility errors when elaborating comptime value (https://github.com/noir-lang/noir/pull/6498) fix: preserve newlines between comments when formatting statements (https://github.com/noir-lang/noir/pull/6601) fix: parse a bit more SSA stuff (https://github.com/noir-lang/noir/pull/6599) chore!: remove eddsa from stdlib (https://github.com/noir-lang/noir/pull/6591) chore: Typo in oracles how to (https://github.com/noir-lang/noir/pull/6598) feat(ssa): Loop invariant code motion (https://github.com/noir-lang/noir/pull/6563) fix: remove `compiler_version` from new `Nargo.toml` (https://github.com/noir-lang/noir/pull/6590) feat: Avoid incrementing reference counts in some cases (https://github.com/noir-lang/noir/pull/6568) chore: fix typo in test name (https://github.com/noir-lang/noir/pull/6589) fix: consider prereleases to be compatible with pre-1.0.0 releases (https://github.com/noir-lang/noir/pull/6580) feat: try to inline brillig calls with all constant arguments (https://github.com/noir-lang/noir/pull/6548) fix: correct type when simplifying `derive_pedersen_generators` (https://github.com/noir-lang/noir/pull/6579) feat: Sync from aztec-packages (https://github.com/noir-lang/noir/pull/6576) --- .noir-sync-commit | 2 +- noir/noir-repo/acvm-repo/acvm_js/build.sh | 2 +- .../compiler/integration-tests/package.json | 2 +- .../compiler/noirc_evaluator/src/acir/mod.rs | 59 +- .../src/brillig/brillig_gen.rs | 54 +- .../src/brillig/brillig_gen/brillig_block.rs | 427 +++++---- .../compiler/noirc_evaluator/src/ssa.rs | 18 + .../check_for_underconstrained_values.rs | 4 +- .../src/ssa/ir/function_inserter.rs | 2 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 62 +- .../src/ssa/ir/instruction/call.rs | 44 +- .../noirc_evaluator/src/ssa/ir/printer.rs | 12 +- .../src/ssa/opt/constant_folding.rs | 842 ++++++++++++++++-- .../noirc_evaluator/src/ssa/opt/die.rs | 2 +- .../src/ssa/opt/flatten_cfg.rs | 391 +++----- .../src/ssa/opt/flatten_cfg/value_merger.rs | 73 +- .../src/ssa/opt/loop_invariant.rs | 378 ++++++++ .../noirc_evaluator/src/ssa/opt/mem2reg.rs | 17 +- .../noirc_evaluator/src/ssa/opt/mod.rs | 1 + .../src/ssa/opt/remove_enable_side_effects.rs | 2 + .../src/ssa/opt/remove_if_else.rs | 12 +- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 24 +- .../noirc_evaluator/src/ssa/parser/ast.rs | 7 + .../src/ssa/parser/into_ssa.rs | 41 +- .../noirc_evaluator/src/ssa/parser/lexer.rs | 46 +- .../noirc_evaluator/src/ssa/parser/mod.rs | 50 +- .../noirc_evaluator/src/ssa/parser/tests.rs | 37 + .../noirc_evaluator/src/ssa/parser/token.rs | 8 + .../src/ssa/ssa_gen/context.rs | 11 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 5 +- .../compiler/noirc_frontend/src/ast/mod.rs | 66 ++ .../noirc_frontend/src/ast/statement.rs | 7 + .../src/elaborator/expressions.rs | 12 +- .../noirc_frontend/src/elaborator/mod.rs | 7 + .../src/elaborator/statements.rs | 13 + .../noirc_frontend/src/elaborator/types.rs | 3 +- .../src/hir/comptime/interpreter/builtin.rs | 2 + .../src/hir/resolution/errors.rs | 9 + .../compiler/noirc_frontend/src/tests.rs | 71 +- .../noirc_frontend/src/tests/unused_items.rs | 6 +- .../noirc_frontend/src/tests/visibility.rs | 105 +++ noir/noir-repo/cspell.json | 2 + .../docs/docs/how_to/how-to-oracles.md | 2 +- .../docs/docs/noir/concepts/globals.md | 16 +- .../cryptographic_primitives/ec_primitives.md | 5 +- .../cryptographic_primitives/eddsa.mdx | 37 - .../docs/docs/noir/standard_library/mem.md | 30 + noir/noir-repo/noir_stdlib/src/bigint.nr | 12 +- .../noir_stdlib/src/collections/map.nr | 4 +- noir/noir-repo/noir_stdlib/src/ec/mod.nr | 8 +- noir/noir-repo/noir_stdlib/src/eddsa.nr | 76 -- noir/noir-repo/noir_stdlib/src/hash/sha256.nr | 18 +- noir/noir-repo/noir_stdlib/src/lib.nr | 1 - noir/noir-repo/noir_stdlib/src/mem.nr | 14 + .../bench_eddsa_poseidon/src/main.nr | 55 +- .../bench_poseidon2_hash_100/src/main.nr | 2 +- .../bench_poseidon2_hash_30/src/main.nr | 2 +- .../bench_poseidon_hash_100/src/main.nr | 2 +- .../bench_poseidon_hash_30/src/main.nr | 2 +- .../bench_poseidon_hash_100/src/main.nr | 2 +- .../bench_poseidon_hash_30/src/main.nr | 2 +- .../benchmarks/bench_sha256_100/src/main.nr | 2 +- .../benchmarks/bench_sha256_30/src/main.nr | 2 +- .../benchmarks/bench_sha256_long/src/main.nr | 2 +- .../assert_constant/src/main.nr | 10 +- .../comptime_globals_regression/src/main.nr | 2 +- .../comptime_module/src/main.nr | 2 +- .../numeric_generics_explicit/src/main.nr | 2 +- .../src/main.nr | 4 +- .../raw_string/src/main.nr | 2 +- .../static_assert/src/main.nr | 10 +- .../src/main.nr | 4 +- .../databus_mapping_regression/src/main.nr | 4 +- .../bench_2_to_17/src/main.nr | 2 +- .../execution_success/brillig_cow/src/main.nr | 2 +- .../brillig_cow_assign/src/main.nr | 2 +- .../brillig_cow_regression/src/main.nr | 2 +- .../Nargo.toml | 2 +- .../Prover.toml | 0 .../src/main.nr | 0 .../execution_success/eddsa/src/main.nr | 74 +- .../fmtstr_with_global/src/main.nr | 2 +- .../fold_2_to_17/src/main.nr | 2 +- .../fold_call_witness_condition/src/main.nr | 2 +- .../fold_numeric_generic_poseidon/src/main.nr | 4 +- .../global_consts/src/foo.nr | 2 +- .../global_consts/src/main.nr | 4 +- .../execution_success/hashmap/src/main.nr | 12 +- .../loop_invariant_regression/Nargo.toml | 7 + .../loop_invariant_regression/Prover.toml | 2 + .../loop_invariant_regression/src/main.nr | 13 + .../src/main.nr | 2 +- .../ram_blowup_regression/src/main.nr | 2 +- .../reference_counts/Nargo.toml | 7 + .../reference_counts/Prover.toml | 2 + .../reference_counts/src/main.nr | 40 + .../regression_2660/src/main.nr | 2 +- .../regression_5252/src/main.nr | 4 +- .../sha256_var_size_regression/src/main.nr | 2 +- .../execution_success/strings/src/main.nr | 2 +- .../struct_inputs/src/foo/bar.nr | 2 +- .../execution_success/uhashmap/src/main.nr | 12 +- .../test_libraries/diamond_deps_2/src/lib.nr | 2 +- .../tooling/debugger/ignored-tests.txt | 3 +- noir/noir-repo/tooling/nargo_cli/build.rs | 6 +- .../tooling/nargo_cli/src/cli/init_cmd.rs | 2 - .../nargo_fmt/src/formatter/expression.rs | 2 +- .../nargo_fmt/src/formatter/function.rs | 30 + .../tooling/nargo_toml/src/errors.rs | 2 + .../tooling/nargo_toml/src/semver.rs | 49 +- .../noir-repo/tooling/noirc_abi_wasm/build.sh | 2 +- noir/noir-repo/yarn.lock | 13 +- 112 files changed, 2636 insertions(+), 960 deletions(-) create mode 100644 noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs delete mode 100644 noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx delete mode 100644 noir/noir-repo/noir_stdlib/src/eddsa.nr rename noir/noir-repo/test_programs/execution_success/{brillig_unitialised_arrays => brillig_uninitialized_arrays}/Nargo.toml (58%) rename noir/noir-repo/test_programs/execution_success/{brillig_unitialised_arrays => brillig_uninitialized_arrays}/Prover.toml (100%) rename noir/noir-repo/test_programs/execution_success/{brillig_unitialised_arrays => brillig_uninitialized_arrays}/src/main.nr (100%) create mode 100644 noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Nargo.toml create mode 100644 noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Prover.toml create mode 100644 noir/noir-repo/test_programs/execution_success/loop_invariant_regression/src/main.nr create mode 100644 noir/noir-repo/test_programs/execution_success/reference_counts/Nargo.toml create mode 100644 noir/noir-repo/test_programs/execution_success/reference_counts/Prover.toml create mode 100644 noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr diff --git a/.noir-sync-commit b/.noir-sync-commit index 9bbde85e56b..9762308ebbf 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -68c32b4ffd9b069fe4b119327dbf4018c17ab9d4 +45eb7568d56b2d254453b85f236d554232aa5df9 diff --git a/noir/noir-repo/acvm-repo/acvm_js/build.sh b/noir/noir-repo/acvm-repo/acvm_js/build.sh index c07d2d8a4c1..16fb26e55db 100755 --- a/noir/noir-repo/acvm-repo/acvm_js/build.sh +++ b/noir/noir-repo/acvm-repo/acvm_js/build.sh @@ -25,7 +25,7 @@ function run_if_available { require_command jq require_command cargo require_command wasm-bindgen -#require_command wasm-opt +require_command wasm-opt self_path=$(dirname "$(readlink -f "$0")") pname=$(cargo read-manifest | jq -r '.name') diff --git a/noir/noir-repo/compiler/integration-tests/package.json b/noir/noir-repo/compiler/integration-tests/package.json index e33179f31e7..a9d437da792 100644 --- a/noir/noir-repo/compiler/integration-tests/package.json +++ b/noir/noir-repo/compiler/integration-tests/package.json @@ -13,7 +13,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "portal:../../../../barretenberg/ts", + "@aztec/bb.js": "0.63.1", "@noir-lang/noir_js": "workspace:*", "@noir-lang/noir_wasm": "workspace:*", "@nomicfoundation/hardhat-chai-matchers": "^2.0.0", diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs index 5c7899b5035..7274fe908d1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs @@ -24,12 +24,10 @@ mod big_int; mod brillig_directive; mod generated_acir; +use crate::brillig::brillig_gen::gen_brillig_for; use crate::brillig::{ brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, - brillig_ir::{ - artifact::{BrilligParameter, GeneratedBrillig}, - BrilligContext, - }, + brillig_ir::artifact::{BrilligParameter, GeneratedBrillig}, Brillig, }; use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport}; @@ -518,7 +516,7 @@ impl<'a> Context<'a> { let outputs: Vec = vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into()); - let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?; + let code = gen_brillig_for(main_func, arguments.clone(), brillig)?; // We specifically do not attempt execution of the brillig code being generated as this can result in it being // replaced with constraints on witnesses to the program outputs. @@ -878,8 +876,7 @@ impl<'a> Context<'a> { None, )? } else { - let code = - self.gen_brillig_for(func, arguments.clone(), brillig)?; + let code = gen_brillig_for(func, arguments.clone(), brillig)?; let generated_pointer = self.shared_context.new_generated_pointer(); let output_values = self.acir_context.brillig_call( @@ -999,47 +996,6 @@ impl<'a> Context<'a> { .collect() } - fn gen_brillig_for( - &self, - func: &Function, - arguments: Vec, - brillig: &Brillig, - ) -> Result, InternalError> { - // Create the entry point artifact - let mut entry_point = BrilligContext::new_entry_point_artifact( - arguments, - BrilligFunctionContext::return_values(func), - func.id(), - ); - entry_point.name = func.name().to_string(); - - // Link the entry point with all dependencies - while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { - let artifact = &brillig.find_by_label(unresolved_fn_label.clone()); - let artifact = match artifact { - Some(artifact) => artifact, - None => { - return Err(InternalError::General { - message: format!("Cannot find linked fn {unresolved_fn_label}"), - call_stack: CallStack::new(), - }) - } - }; - entry_point.link_with(artifact); - // Insert the range of opcode locations occupied by a procedure - if let Some(procedure_id) = &artifact.procedure { - let num_opcodes = entry_point.byte_code.len(); - let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len(); - // We subtract one as to keep the range inclusive on both ends - entry_point - .procedure_locations - .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1)); - } - } - // Generate the final bytecode - Ok(entry_point.finish()) - } - /// Handles an ArrayGet or ArraySet instruction. /// To set an index of the array (and create a new array in doing so), pass Some(value) for /// store_value. To just retrieve an index of the array, pass None for store_value. @@ -2806,6 +2762,13 @@ impl<'a> Context<'a> { Intrinsic::FieldLessThan => { unreachable!("FieldLessThan can only be called in unconstrained") } + Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => { + let zero = self.acir_context.add_constant(FieldElement::zero()); + Ok(vec![AcirValue::Var( + zero, + AcirType::NumericType(NumericType::Unsigned { bit_size: 32 }), + )]) + } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index 786a03031d6..ca4e783aa93 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -9,11 +9,17 @@ mod variable_liveness; use acvm::FieldElement; use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext}; -use super::brillig_ir::{ - artifact::{BrilligArtifact, Label}, - BrilligContext, +use super::{ + brillig_ir::{ + artifact::{BrilligArtifact, BrilligParameter, GeneratedBrillig, Label}, + BrilligContext, + }, + Brillig, +}; +use crate::{ + errors::InternalError, + ssa::ir::{dfg::CallStack, function::Function}, }; -use crate::ssa::ir::function::Function; /// Converting an SSA function into Brillig bytecode. pub(crate) fn convert_ssa_function( @@ -36,3 +42,43 @@ pub(crate) fn convert_ssa_function( artifact.name = func.name().to_string(); artifact } + +pub(crate) fn gen_brillig_for( + func: &Function, + arguments: Vec, + brillig: &Brillig, +) -> Result, InternalError> { + // Create the entry point artifact + let mut entry_point = BrilligContext::new_entry_point_artifact( + arguments, + FunctionContext::return_values(func), + func.id(), + ); + entry_point.name = func.name().to_string(); + + // Link the entry point with all dependencies + while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() { + let artifact = &brillig.find_by_label(unresolved_fn_label.clone()); + let artifact = match artifact { + Some(artifact) => artifact, + None => { + return Err(InternalError::General { + message: format!("Cannot find linked fn {unresolved_fn_label}"), + call_stack: CallStack::new(), + }) + } + }; + entry_point.link_with(artifact); + // Insert the range of opcode locations occupied by a procedure + if let Some(procedure_id) = &artifact.procedure { + let num_opcodes = entry_point.byte_code.len(); + let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len(); + // We subtract one as to keep the range inclusive on both ends + entry_point + .procedure_locations + .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1)); + } + } + // Generate the final bytecode + Ok(entry_point.finish()) +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 36e1ee90e11..1fa4985295a 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -402,210 +402,251 @@ impl<'block> BrilligBlock<'block> { let result_ids = dfg.instruction_results(instruction_id); self.convert_ssa_function_call(*func_id, arguments, dfg, result_ids); } - Value::Intrinsic(Intrinsic::BlackBox(bb_func)) => { - // Slices are represented as a tuple of (length, slice contents). - // We must check the inputs to determine if there are slices - // and make sure that we pass the correct inputs to the black box function call. - // The loop below only keeps the slice contents, so that - // setting up a black box function with slice inputs matches the expected - // number of arguments specified in the function signature. - let mut arguments_no_slice_len = Vec::new(); - for (i, arg) in arguments.iter().enumerate() { - if matches!(dfg.type_of_value(*arg), Type::Numeric(_)) { - if i < arguments.len() - 1 { - if !matches!(dfg.type_of_value(arguments[i + 1]), Type::Slice(_)) { - arguments_no_slice_len.push(*arg); - } + Value::Intrinsic(intrinsic) => { + // This match could be combined with the above but without it rust analyzer + // can't automatically insert any missing cases + match intrinsic { + Intrinsic::ArrayLen => { + let result_variable = self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + dfg.instruction_results(instruction_id)[0], + dfg, + ); + let param_id = arguments[0]; + // Slices are represented as a tuple in the form: (length, slice contents). + // Thus, we can expect the first argument to a field in the case of a slice + // or an array in the case of an array. + if let Type::Numeric(_) = dfg.type_of_value(param_id) { + let len_variable = self.convert_ssa_value(arguments[0], dfg); + let length = len_variable.extract_single_addr(); + self.brillig_context + .mov_instruction(result_variable.address, length.address); } else { - arguments_no_slice_len.push(*arg); + self.convert_ssa_array_len( + arguments[0], + result_variable.address, + dfg, + ); } - } else { - arguments_no_slice_len.push(*arg); } - } - - let function_arguments = - vecmap(&arguments_no_slice_len, |arg| self.convert_ssa_value(*arg, dfg)); - let function_results = dfg.instruction_results(instruction_id); - let function_results = vecmap(function_results, |result| { - self.allocate_external_call_result(*result, dfg) - }); - convert_black_box_call( - self.brillig_context, - bb_func, - &function_arguments, - &function_results, - ); - } - Value::Intrinsic(Intrinsic::ArrayLen) => { - let result_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - dfg.instruction_results(instruction_id)[0], - dfg, - ); - let param_id = arguments[0]; - // Slices are represented as a tuple in the form: (length, slice contents). - // Thus, we can expect the first argument to a field in the case of a slice - // or an array in the case of an array. - if let Type::Numeric(_) = dfg.type_of_value(param_id) { - let len_variable = self.convert_ssa_value(arguments[0], dfg); - let length = len_variable.extract_single_addr(); - self.brillig_context - .mov_instruction(result_variable.address, length.address); - } else { - self.convert_ssa_array_len(arguments[0], result_variable.address, dfg); - } - } - Value::Intrinsic(Intrinsic::AsSlice) => { - let source_variable = self.convert_ssa_value(arguments[0], dfg); - let result_ids = dfg.instruction_results(instruction_id); - let destination_len_variable = self.variables.define_single_addr_variable( - self.function_context, - self.brillig_context, - result_ids[0], - dfg, - ); - let destination_variable = self.variables.define_variable( - self.function_context, - self.brillig_context, - result_ids[1], - dfg, - ); - let destination_vector = destination_variable.extract_vector(); - let source_array = source_variable.extract_array(); - let element_size = dfg.type_of_value(arguments[0]).element_size(); - - let source_size_register = self - .brillig_context - .make_usize_constant_instruction(source_array.size.into()); - - // we need to explicitly set the destination_len_variable - self.brillig_context.codegen_usize_op( - source_size_register.address, - destination_len_variable.address, - BrilligBinaryOp::UnsignedDiv, - element_size, - ); - - self.brillig_context.codegen_initialize_vector( - destination_vector, - source_size_register, - None, - ); - - // Items - let vector_items_pointer = - self.brillig_context.codegen_make_vector_items_pointer(destination_vector); - let array_items_pointer = - self.brillig_context.codegen_make_array_items_pointer(source_array); - - self.brillig_context.codegen_mem_copy( - array_items_pointer, - vector_items_pointer, - source_size_register, - ); - - self.brillig_context.deallocate_single_addr(source_size_register); - self.brillig_context.deallocate_register(vector_items_pointer); - self.brillig_context.deallocate_register(array_items_pointer); - } - Value::Intrinsic( - Intrinsic::SlicePushBack - | Intrinsic::SlicePopBack - | Intrinsic::SlicePushFront - | Intrinsic::SlicePopFront - | Intrinsic::SliceInsert - | Intrinsic::SliceRemove, - ) => { - self.convert_ssa_slice_intrinsic_call( - dfg, - &dfg[dfg.resolve(*func)], - instruction_id, - arguments, - ); - } - Value::Intrinsic(Intrinsic::ToRadix(endianness)) => { - let results = dfg.instruction_results(instruction_id); - - let source = self.convert_ssa_single_addr_value(arguments[0], dfg); - let radix = self.convert_ssa_single_addr_value(arguments[1], dfg); - - let target_array = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_array(); - - self.brillig_context.codegen_to_radix( - source, - target_array, - radix, - matches!(endianness, Endian::Little), - false, - ); - } - Value::Intrinsic(Intrinsic::ToBits(endianness)) => { - let results = dfg.instruction_results(instruction_id); + Intrinsic::AsSlice => { + let source_variable = self.convert_ssa_value(arguments[0], dfg); + let result_ids = dfg.instruction_results(instruction_id); + let destination_len_variable = + self.variables.define_single_addr_variable( + self.function_context, + self.brillig_context, + result_ids[0], + dfg, + ); + let destination_variable = self.variables.define_variable( + self.function_context, + self.brillig_context, + result_ids[1], + dfg, + ); + let destination_vector = destination_variable.extract_vector(); + let source_array = source_variable.extract_array(); + let element_size = dfg.type_of_value(arguments[0]).element_size(); - let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + let source_size_register = self + .brillig_context + .make_usize_constant_instruction(source_array.size.into()); + + // we need to explicitly set the destination_len_variable + self.brillig_context.codegen_usize_op( + source_size_register.address, + destination_len_variable.address, + BrilligBinaryOp::UnsignedDiv, + element_size, + ); - let target_array = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_array(); + self.brillig_context.codegen_initialize_vector( + destination_vector, + source_size_register, + None, + ); - let two = self.brillig_context.make_usize_constant_instruction(2_usize.into()); + // Items + let vector_items_pointer = self + .brillig_context + .codegen_make_vector_items_pointer(destination_vector); + let array_items_pointer = + self.brillig_context.codegen_make_array_items_pointer(source_array); + + self.brillig_context.codegen_mem_copy( + array_items_pointer, + vector_items_pointer, + source_size_register, + ); - self.brillig_context.codegen_to_radix( - source, - target_array, - two, - matches!(endianness, Endian::Little), - true, - ); + self.brillig_context.deallocate_single_addr(source_size_register); + self.brillig_context.deallocate_register(vector_items_pointer); + self.brillig_context.deallocate_register(array_items_pointer); + } + Intrinsic::SlicePushBack + | Intrinsic::SlicePopBack + | Intrinsic::SlicePushFront + | Intrinsic::SlicePopFront + | Intrinsic::SliceInsert + | Intrinsic::SliceRemove => { + self.convert_ssa_slice_intrinsic_call( + dfg, + &dfg[dfg.resolve(*func)], + instruction_id, + arguments, + ); + } + Intrinsic::ToBits(endianness) => { + let results = dfg.instruction_results(instruction_id); + + let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + + let target_array = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_array(); + + let two = self + .brillig_context + .make_usize_constant_instruction(2_usize.into()); + + self.brillig_context.codegen_to_radix( + source, + target_array, + two, + matches!(endianness, Endian::Little), + true, + ); - self.brillig_context.deallocate_single_addr(two); - } + self.brillig_context.deallocate_single_addr(two); + } - // `Intrinsic::AsWitness` is used to provide hints to acir-gen on optimal expression splitting. - // It is then useless in the brillig runtime and so we can ignore it - Value::Intrinsic(Intrinsic::AsWitness) => (), - Value::Intrinsic(Intrinsic::FieldLessThan) => { - let lhs = self.convert_ssa_single_addr_value(arguments[0], dfg); - assert!(lhs.bit_size == FieldElement::max_num_bits()); - let rhs = self.convert_ssa_single_addr_value(arguments[1], dfg); - assert!(rhs.bit_size == FieldElement::max_num_bits()); - - let results = dfg.instruction_results(instruction_id); - let destination = self - .variables - .define_variable( - self.function_context, - self.brillig_context, - results[0], - dfg, - ) - .extract_single_addr(); - assert!(destination.bit_size == 1); + Intrinsic::ToRadix(endianness) => { + let results = dfg.instruction_results(instruction_id); + + let source = self.convert_ssa_single_addr_value(arguments[0], dfg); + let radix = self.convert_ssa_single_addr_value(arguments[1], dfg); + + let target_array = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_array(); + + self.brillig_context.codegen_to_radix( + source, + target_array, + radix, + matches!(endianness, Endian::Little), + false, + ); + } + Intrinsic::BlackBox(bb_func) => { + // Slices are represented as a tuple of (length, slice contents). + // We must check the inputs to determine if there are slices + // and make sure that we pass the correct inputs to the black box function call. + // The loop below only keeps the slice contents, so that + // setting up a black box function with slice inputs matches the expected + // number of arguments specified in the function signature. + let mut arguments_no_slice_len = Vec::new(); + for (i, arg) in arguments.iter().enumerate() { + if matches!(dfg.type_of_value(*arg), Type::Numeric(_)) { + if i < arguments.len() - 1 { + if !matches!( + dfg.type_of_value(arguments[i + 1]), + Type::Slice(_) + ) { + arguments_no_slice_len.push(*arg); + } + } else { + arguments_no_slice_len.push(*arg); + } + } else { + arguments_no_slice_len.push(*arg); + } + } - self.brillig_context.binary_instruction( - lhs, - rhs, - destination, - BrilligBinaryOp::LessThan, - ); + let function_arguments = vecmap(&arguments_no_slice_len, |arg| { + self.convert_ssa_value(*arg, dfg) + }); + let function_results = dfg.instruction_results(instruction_id); + let function_results = vecmap(function_results, |result| { + self.allocate_external_call_result(*result, dfg) + }); + convert_black_box_call( + self.brillig_context, + bb_func, + &function_arguments, + &function_results, + ); + } + // `Intrinsic::AsWitness` is used to provide hints to acir-gen on optimal expression splitting. + // It is then useless in the brillig runtime and so we can ignore it + Intrinsic::AsWitness => (), + Intrinsic::FieldLessThan => { + let lhs = self.convert_ssa_single_addr_value(arguments[0], dfg); + assert!(lhs.bit_size == FieldElement::max_num_bits()); + let rhs = self.convert_ssa_single_addr_value(arguments[1], dfg); + assert!(rhs.bit_size == FieldElement::max_num_bits()); + + let results = dfg.instruction_results(instruction_id); + let destination = self + .variables + .define_variable( + self.function_context, + self.brillig_context, + results[0], + dfg, + ) + .extract_single_addr(); + assert!(destination.bit_size == 1); + + self.brillig_context.binary_instruction( + lhs, + rhs, + destination, + BrilligBinaryOp::LessThan, + ); + } + Intrinsic::ArrayRefCount | Intrinsic::SliceRefCount => { + let array = self.convert_ssa_value(arguments[0], dfg); + let result = dfg.instruction_results(instruction_id)[0]; + + let destination = self.variables.define_variable( + self.function_context, + self.brillig_context, + result, + dfg, + ); + let destination = destination.extract_register(); + let array = array.extract_register(); + self.brillig_context.load_instruction(destination, array); + } + Intrinsic::FromField + | Intrinsic::AsField + | Intrinsic::IsUnconstrained + | Intrinsic::DerivePedersenGenerators + | Intrinsic::ApplyRangeConstraint + | Intrinsic::StrAsBytes + | Intrinsic::AssertConstant + | Intrinsic::StaticAssert + | Intrinsic::ArrayAsStrUnchecked => { + unreachable!("unsupported function call type {:?}", dfg[*func]) + } + } } - _ => { + Value::Instruction { .. } | Value::Param { .. } | Value::NumericConstant { .. } => { unreachable!("unsupported function call type {:?}", dfg[*func]) } }, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index 9e11441caf4..97c1760d87c 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -103,6 +103,7 @@ pub(crate) fn optimize_into_acir( Ssa::evaluate_static_assert_and_assert_constant, "After `static_assert` and `assert_constant`:", )? + .run_pass(Ssa::loop_invariant_code_motion, "After Loop Invariant Code Motion:") .try_run_pass(Ssa::unroll_loops_iteratively, "After Unrolling:")? .run_pass(Ssa::simplify_cfg, "After Simplifying (2nd):") .run_pass(Ssa::flatten_cfg, "After Flattening:") @@ -140,6 +141,23 @@ pub(crate) fn optimize_into_acir( ssa.to_brillig(options.enable_brillig_logging) }); + let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); + let ssa_gen_span_guard = ssa_gen_span.enter(); + + let ssa = SsaBuilder { + ssa, + print_ssa_passes: options.enable_ssa_logging, + print_codegen_timings: options.print_codegen_timings, + } + .run_pass( + |ssa| ssa.fold_constants_with_brillig(&brillig), + "After Constant Folding with Brillig:", + ) + .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:") + .finish(); + + drop(ssa_gen_span_guard); + let artifacts = time("SSA to ACIR", options.print_codegen_timings, || { ssa.into_acir(&brillig, options.expression_width) })?; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index cf884c98be9..7a4e336c33e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -205,16 +205,18 @@ impl Context { | Intrinsic::IsUnconstrained => {} Intrinsic::ArrayLen | Intrinsic::ArrayAsStrUnchecked + | Intrinsic::ArrayRefCount | Intrinsic::AsField | Intrinsic::AsSlice | Intrinsic::BlackBox(..) | Intrinsic::DerivePedersenGenerators | Intrinsic::FromField + | Intrinsic::SliceInsert | Intrinsic::SlicePushBack | Intrinsic::SlicePushFront | Intrinsic::SlicePopBack | Intrinsic::SlicePopFront - | Intrinsic::SliceInsert + | Intrinsic::SliceRefCount | Intrinsic::SliceRemove | Intrinsic::StaticAssert | Intrinsic::StrAsBytes diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs index 5e133072067..a0c23ad70aa 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs @@ -25,7 +25,7 @@ pub(crate) struct FunctionInserter<'f> { /// /// This is optional since caching arrays relies on the inserter inserting strictly /// in control-flow order. Otherwise, if arrays later in the program are cached first, - /// they may be refered to by instructions earlier in the program. + /// they may be referred to by instructions earlier in the program. array_cache: Option, /// If this pass is loop unrolling, store the block before the loop to optionally diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 936dc854c51..9958aadd443 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -11,11 +11,12 @@ use fxhash::FxHasher64; use iter_extended::vecmap; use noirc_frontend::hir_def::types::Type as HirType; -use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger; +use crate::ssa::{ir::function::RuntimeType, opt::flatten_cfg::value_merger::ValueMerger}; use super::{ basic_block::BasicBlockId, dfg::{CallStack, DataFlowGraph}, + function::Function, map::Id, types::{NumericType, Type}, value::{Value, ValueId}, @@ -70,6 +71,8 @@ pub(crate) enum Intrinsic { IsUnconstrained, DerivePedersenGenerators, FieldLessThan, + ArrayRefCount, + SliceRefCount, } impl std::fmt::Display for Intrinsic { @@ -99,6 +102,8 @@ impl std::fmt::Display for Intrinsic { Intrinsic::IsUnconstrained => write!(f, "is_unconstrained"), Intrinsic::DerivePedersenGenerators => write!(f, "derive_pedersen_generators"), Intrinsic::FieldLessThan => write!(f, "field_less_than"), + Intrinsic::ArrayRefCount => write!(f, "array_refcount"), + Intrinsic::SliceRefCount => write!(f, "slice_refcount"), } } } @@ -112,6 +117,10 @@ impl Intrinsic { Intrinsic::AssertConstant | Intrinsic::StaticAssert | Intrinsic::ApplyRangeConstraint + // Array & slice ref counts are treated as having side effects since they operate + // on hidden variables on otherwise identical array values. + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::AsWitness => true, // These apply a constraint that the input must fit into a specified number of limbs. @@ -170,6 +179,8 @@ impl Intrinsic { "is_unconstrained" => Some(Intrinsic::IsUnconstrained), "derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators), "field_less_than" => Some(Intrinsic::FieldLessThan), + "array_refcount" => Some(Intrinsic::ArrayRefCount), + "slice_refcount" => Some(Intrinsic::SliceRefCount), other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox), } @@ -269,15 +280,7 @@ pub(crate) enum Instruction { /// else_value /// } /// ``` - /// - /// Where we save the result of !then_condition so that we have the same - /// ValueId for it each time. - IfElse { - then_condition: ValueId, - then_value: ValueId, - else_condition: ValueId, - else_value: ValueId, - }, + IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId }, /// Creates a new array or slice. /// @@ -371,12 +374,12 @@ impl Instruction { } } - pub(crate) fn can_eliminate_if_unused(&self, dfg: &DataFlowGraph) -> bool { + pub(crate) fn can_eliminate_if_unused(&self, function: &Function) -> bool { use Instruction::*; match self { Binary(binary) => { if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) { - if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) { + if let Some(rhs) = function.dfg.get_numeric_constant(binary.rhs) { rhs != FieldElement::zero() } else { false @@ -395,15 +398,26 @@ impl Instruction { | ArraySet { .. } | MakeArray { .. } => true, + // Store instructions must be removed by DIE in acir code, any load + // instructions should already be unused by that point. + // + // Note that this check assumes that it is being performed after the flattening + // pass and after the last mem2reg pass. This is currently the case for the DIE + // pass where this check is done, but does mean that we cannot perform mem2reg + // after the DIE pass. + Store { .. } => { + matches!(function.runtime(), RuntimeType::Acir(_)) + && function.reachable_blocks().len() == 1 + } + Constrain(..) - | Store { .. } | EnableSideEffectsIf { .. } | IncrementRc { .. } | DecrementRc { .. } | RangeCheck { .. } => false, // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is. - Call { func, .. } => match dfg[*func] { + Call { func, .. } => match function.dfg[*func] { // Explicitly allows removal of unused ec operations, even if they can fail Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::MultiScalarMul)) | Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::EmbeddedCurveAdd)) => true, @@ -524,14 +538,11 @@ impl Instruction { assert_message: assert_message.clone(), } } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { - Instruction::IfElse { - then_condition: f(*then_condition), - then_value: f(*then_value), - else_condition: f(*else_condition), - else_value: f(*else_value), - } - } + Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse { + then_condition: f(*then_condition), + then_value: f(*then_value), + else_value: f(*else_value), + }, Instruction::MakeArray { elements, typ } => Instruction::MakeArray { elements: elements.iter().copied().map(f).collect(), typ: typ.clone(), @@ -590,10 +601,9 @@ impl Instruction { | Instruction::RangeCheck { value, .. } => { f(*value); } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { f(*then_condition); f(*then_value); - f(*else_condition); f(*else_value); } Instruction::MakeArray { elements, typ: _ } => { @@ -756,7 +766,7 @@ impl Instruction { None } } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let typ = dfg.type_of_value(*then_value); if let Some(constant) = dfg.get_numeric_constant(*then_condition) { @@ -775,13 +785,11 @@ impl Instruction { if matches!(&typ, Type::Numeric(_)) { let then_condition = *then_condition; - let else_condition = *else_condition; let result = ValueMerger::merge_numeric_values( dfg, block, then_condition, - else_condition, then_value, else_value, ); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index e1e967b9a43..4be37b3c626 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -368,6 +368,8 @@ pub(super) fn simplify_call( SimplifyResult::None } } + Intrinsic::ArrayRefCount => SimplifyResult::None, + Intrinsic::SliceRefCount => SimplifyResult::None, } } @@ -443,12 +445,8 @@ fn simplify_slice_push_back( let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None, call_stack); - let new_slice = value_merger.merge_values( - len_not_equals_capacity, - len_equals_capacity, - set_last_slice_value, - new_slice, - ); + let new_slice = + value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice); SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]) } @@ -810,7 +808,8 @@ fn simplify_derive_generators( results.push(is_infinite); } let len = results.len(); - let typ = Type::Array(vec![Type::field()].into(), len); + let typ = + Type::Array(vec![Type::field(), Type::field(), Type::unsigned(1)].into(), len / 3); let result = make_array(dfg, results.into(), typ, block, call_stack); SimplifyResult::SimplifiedTo(result) } else { @@ -820,3 +819,34 @@ fn simplify_derive_generators( unreachable!("Unexpected number of arguments to derive_generators"); } } + +#[cfg(test)] +mod tests { + use crate::ssa::{opt::assert_normalized_ssa_equals, Ssa}; + + #[test] + fn simplify_derive_generators_has_correct_type() { + let src = " + brillig(inline) fn main f0 { + b0(): + v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24] + + // This call was previously incorrectly simplified to something that returned `[Field; 3]` + v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1] + + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + + let expected = " + brillig(inline) fn main f0 { + b0(): + v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24] + v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1] + return v19 + } + "; + assert_normalized_ssa_equals(ssa, expected); + } +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index c44e7d8a388..6bebd21fe61 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -209,15 +209,11 @@ fn display_instruction_inner( Instruction::RangeCheck { value, max_bit_size, .. } => { writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,) } - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = show(*then_condition); let then_value = show(*then_value); - let else_condition = show(*else_condition); let else_value = show(*else_value); - writeln!( - f, - "if {then_condition} then {then_value} else if {else_condition} then {else_value}" - ) + writeln!(f, "if {then_condition} then {then_value} else {else_value}") } Instruction::MakeArray { elements, typ } => { write!(f, "make_array [")?; @@ -276,13 +272,13 @@ fn display_constrain_error( ) -> Result { match error { ConstrainError::StaticString(assert_message_string) => { - writeln!(f, " '{assert_message_string:?}'") + writeln!(f, ", {assert_message_string:?}") } ConstrainError::Dynamic(_, is_string, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload(*is_string, values, &function.dfg) { - writeln!(f, " '{}'", constant_string) + writeln!(f, ", {constant_string:?}") } else { writeln!(f, ", data {}", value_list(function, values)) } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 9f55e69868c..019bace33a3 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -19,21 +19,35 @@ //! //! This is the only pass which removes duplicated pure [`Instruction`]s however and so is needed when //! different blocks are merged, i.e. after the [`flatten_cfg`][super::flatten_cfg] pass. -use std::collections::HashSet; +use std::collections::{BTreeMap, HashSet, VecDeque}; -use acvm::{acir::AcirField, FieldElement}; +use acvm::{ + acir::AcirField, + brillig_vm::{MemoryValue, VMStatus, VM}, + FieldElement, +}; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use im::Vector; use iter_extended::vecmap; -use crate::ssa::{ - ir::{ - basic_block::BasicBlockId, - dfg::{DataFlowGraph, InsertInstructionResult}, - function::Function, - instruction::{Instruction, InstructionId}, - types::Type, - value::{Value, ValueId}, +use crate::{ + brillig::{ + brillig_gen::gen_brillig_for, + brillig_ir::{artifact::BrilligParameter, brillig_variable::get_bit_size_from_ssa_type}, + Brillig, + }, + ssa::{ + ir::{ + basic_block::BasicBlockId, + dfg::{DataFlowGraph, InsertInstructionResult}, + dom::DominatorTree, + function::{Function, FunctionId, RuntimeType}, + instruction::{Instruction, InstructionId}, + types::Type, + value::{Value, ValueId}, + }, + ssa_gen::Ssa, }, - ssa_gen::Ssa, }; use fxhash::FxHashMap as HashMap; @@ -44,7 +58,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn fold_constants(mut self) -> Ssa { for function in self.functions.values_mut() { - function.constant_fold(false); + function.constant_fold(false, None); } self } @@ -57,8 +71,69 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn fold_constants_using_constraints(mut self) -> Ssa { for function in self.functions.values_mut() { - function.constant_fold(true); + function.constant_fold(true, None); + } + self + } + + /// Performs constant folding on each instruction while also replacing calls to brillig functions + /// with all constant arguments by trying to evaluate those calls. + #[tracing::instrument(level = "trace", skip(self, brillig))] + pub(crate) fn fold_constants_with_brillig(mut self, brillig: &Brillig) -> Ssa { + // Collect all brillig functions so that later we can find them when processing a call instruction + let mut brillig_functions: BTreeMap = BTreeMap::new(); + for (func_id, func) in &self.functions { + if let RuntimeType::Brillig(..) = func.runtime() { + let cloned_function = Function::clone_with_id(*func_id, func); + brillig_functions.insert(*func_id, cloned_function); + }; + } + + let brillig_info = Some(BrilligInfo { brillig, brillig_functions: &brillig_functions }); + + for function in self.functions.values_mut() { + function.constant_fold(false, brillig_info); + } + + // It could happen that we inlined all calls to a given brillig function. + // In that case it's unused so we can remove it. This is what we check next. + self.remove_unused_brillig_functions(brillig_functions) + } + + fn remove_unused_brillig_functions( + mut self, + mut brillig_functions: BTreeMap, + ) -> Ssa { + // Remove from the above map functions that are called + for function in self.functions.values() { + for block_id in function.reachable_blocks() { + for instruction_id in function.dfg[block_id].instructions() { + let instruction = &function.dfg[*instruction_id]; + let Instruction::Call { func: func_id, arguments: _ } = instruction else { + continue; + }; + + let func_value = &function.dfg[*func_id]; + let Value::Function(func_id) = func_value else { continue }; + + brillig_functions.remove(func_id); + } + } } + + // The ones that remain are never called: let's remove them. + for func_id in brillig_functions.keys() { + // We never want to remove the main function (it could be `unconstrained` or it + // could have been turned into brillig if `--force-brillig` was given). + // We also don't want to remove entry points. + if self.main_id == *func_id || self.entry_point_to_generated_index.contains_key(func_id) + { + continue; + } + + self.functions.remove(func_id); + } + self } } @@ -66,11 +141,15 @@ impl Ssa { impl Function { /// The structure of this pass is simple: /// Go through each block and re-insert all instructions. - pub(crate) fn constant_fold(&mut self, use_constraint_info: bool) { - let mut context = Context { use_constraint_info, ..Default::default() }; - context.block_queue.push(self.entry_block()); + pub(crate) fn constant_fold( + &mut self, + use_constraint_info: bool, + brillig_info: Option, + ) { + let mut context = Context::new(self, use_constraint_info, brillig_info); + context.block_queue.push_back(self.entry_block()); - while let Some(block) = context.block_queue.pop() { + while let Some(block) = context.block_queue.pop_front() { if context.visited_blocks.contains(&block) { continue; } @@ -81,34 +160,69 @@ impl Function { } } -#[derive(Default)] -struct Context { +struct Context<'a> { use_constraint_info: bool, + brillig_info: Option>, /// Maps pre-folded ValueIds to the new ValueIds obtained by re-inserting the instruction. visited_blocks: HashSet, - block_queue: Vec, + block_queue: VecDeque, + + /// Contains sets of values which are constrained to be equivalent to each other. + /// + /// The mapping's structure is `side_effects_enabled_var => (constrained_value => simplified_value)`. + /// + /// We partition the maps of constrained values according to the side-effects flag at the point + /// at which the values are constrained. This prevents constraints which are only sometimes enforced + /// being used to modify the rest of the program. + constraint_simplification_mappings: HashMap>, + + // Cache of instructions without any side-effects along with their outputs. + cached_instruction_results: InstructionResultCache, + + dom: DominatorTree, +} + +#[derive(Copy, Clone)] +pub(crate) struct BrilligInfo<'a> { + brillig: &'a Brillig, + brillig_functions: &'a BTreeMap, } /// HashMap from (Instruction, side_effects_enabled_var) to the results of the instruction. /// Stored as a two-level map to avoid cloning Instructions during the `.get` call. -type InstructionResultCache = HashMap, Vec>>; +/// +/// In addition to each result, the original BasicBlockId is stored as well. This allows us +/// to deduplicate instructions across blocks as long as the new block dominates the original. +type InstructionResultCache = HashMap, ResultCache>>; + +/// Records the results of all duplicate [`Instruction`]s along with the blocks in which they sit. +/// +/// For more information see [`InstructionResultCache`]. +#[derive(Default)] +struct ResultCache { + result: Option<(BasicBlockId, Vec)>, +} + +impl<'brillig> Context<'brillig> { + fn new( + function: &Function, + use_constraint_info: bool, + brillig_info: Option>, + ) -> Self { + Self { + use_constraint_info, + brillig_info, + visited_blocks: Default::default(), + block_queue: Default::default(), + constraint_simplification_mappings: Default::default(), + cached_instruction_results: Default::default(), + dom: DominatorTree::with_function(function), + } + } -impl Context { fn fold_constants_in_block(&mut self, function: &mut Function, block: BasicBlockId) { let instructions = function.dfg[block].take_instructions(); - // Cache of instructions without any side-effects along with their outputs. - let mut cached_instruction_results = HashMap::default(); - - // Contains sets of values which are constrained to be equivalent to each other. - // - // The mapping's structure is `side_effects_enabled_var => (constrained_value => simplified_value)`. - // - // We partition the maps of constrained values according to the side-effects flag at the point - // at which the values are constrained. This prevents constraints which are only sometimes enforced - // being used to modify the rest of the program. - let mut constraint_simplification_mappings: HashMap> = - HashMap::default(); let mut side_effects_enabled_var = function.dfg.make_constant(FieldElement::one(), Type::bool()); @@ -117,8 +231,6 @@ impl Context { &mut function.dfg, block, instruction_id, - &mut cached_instruction_results, - &mut constraint_simplification_mappings, &mut side_effects_enabled_var, ); } @@ -126,29 +238,54 @@ impl Context { } fn fold_constants_into_instruction( - &self, + &mut self, dfg: &mut DataFlowGraph, - block: BasicBlockId, + mut block: BasicBlockId, id: InstructionId, - instruction_result_cache: &mut InstructionResultCache, - constraint_simplification_mappings: &mut HashMap>, side_effects_enabled_var: &mut ValueId, ) { - let constraint_simplification_mapping = - constraint_simplification_mappings.entry(*side_effects_enabled_var).or_default(); + let constraint_simplification_mapping = self.get_constraint_map(*side_effects_enabled_var); let instruction = Self::resolve_instruction(id, dfg, constraint_simplification_mapping); let old_results = dfg.instruction_results(id).to_vec(); // If a copy of this instruction exists earlier in the block, then reuse the previous results. - if let Some(cached_results) = - Self::get_cached(dfg, instruction_result_cache, &instruction, *side_effects_enabled_var) + if let Some(cache_result) = + self.get_cached(dfg, &instruction, *side_effects_enabled_var, block) { - Self::replace_result_ids(dfg, &old_results, cached_results); - return; + match cache_result { + CacheResult::Cached(cached) => { + Self::replace_result_ids(dfg, &old_results, cached); + return; + } + CacheResult::NeedToHoistToCommonBlock(dominator, _cached) => { + // Just change the block to insert in the common dominator instead. + // This will only move the current instance of the instruction right now. + // When constant folding is run a second time later on, it'll catch + // that the previous instance can be deduplicated to this instance. + block = dominator; + } + } } - // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. - let new_results = Self::push_instruction(id, instruction.clone(), &old_results, block, dfg); + let new_results = + // First try to inline a call to a brillig function with all constant arguments. + Self::try_inline_brillig_call_with_all_constants( + &instruction, + &old_results, + block, + dfg, + self.brillig_info, + ) + .unwrap_or_else(|| { + // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. + Self::push_instruction( + id, + instruction.clone(), + &old_results, + block, + dfg, + ) + }); Self::replace_result_ids(dfg, &old_results, &new_results); @@ -156,9 +293,8 @@ impl Context { instruction.clone(), new_results, dfg, - instruction_result_cache, - constraint_simplification_mapping, *side_effects_enabled_var, + block, ); // If we just inserted an `Instruction::EnableSideEffectsIf`, we need to update `side_effects_enabled_var` @@ -229,13 +365,12 @@ impl Context { } fn cache_instruction( - &self, + &mut self, instruction: Instruction, instruction_results: Vec, dfg: &DataFlowGraph, - instruction_result_cache: &mut InstructionResultCache, - constraint_simplification_mapping: &mut HashMap, side_effects_enabled_var: ValueId, + block: BasicBlockId, ) { if self.use_constraint_info { // If the instruction was a constraint, then create a link between the two `ValueId`s @@ -248,18 +383,18 @@ impl Context { // Prefer replacing with constants where possible. (Value::NumericConstant { .. }, _) => { - constraint_simplification_mapping.insert(rhs, lhs); + self.get_constraint_map(side_effects_enabled_var).insert(rhs, lhs); } (_, Value::NumericConstant { .. }) => { - constraint_simplification_mapping.insert(lhs, rhs); + self.get_constraint_map(side_effects_enabled_var).insert(lhs, rhs); } // Otherwise prefer block parameters over instruction results. // This is as block parameters are more likely to be a single witness rather than a full expression. (Value::Param { .. }, Value::Instruction { .. }) => { - constraint_simplification_mapping.insert(rhs, lhs); + self.get_constraint_map(side_effects_enabled_var).insert(rhs, lhs); } (Value::Instruction { .. }, Value::Param { .. }) => { - constraint_simplification_mapping.insert(lhs, rhs); + self.get_constraint_map(side_effects_enabled_var).insert(lhs, rhs); } (_, _) => (), } @@ -273,13 +408,22 @@ impl Context { self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg); let predicate = use_predicate.then_some(side_effects_enabled_var); - instruction_result_cache + self.cached_instruction_results .entry(instruction) .or_default() - .insert(predicate, instruction_results); + .entry(predicate) + .or_default() + .cache(block, instruction_results); } } + fn get_constraint_map( + &mut self, + side_effects_enabled_var: ValueId, + ) -> &mut HashMap { + self.constraint_simplification_mappings.entry(side_effects_enabled_var).or_default() + } + /// Replaces a set of [`ValueId`]s inside the [`DataFlowGraph`] with another. fn replace_result_ids( dfg: &mut DataFlowGraph, @@ -291,24 +435,256 @@ impl Context { } } - fn get_cached<'a>( + fn get_cached( + &mut self, dfg: &DataFlowGraph, - instruction_result_cache: &'a mut InstructionResultCache, instruction: &Instruction, side_effects_enabled_var: ValueId, - ) -> Option<&'a Vec> { - let results_for_instruction = instruction_result_cache.get(instruction); + block: BasicBlockId, + ) -> Option { + let results_for_instruction = self.cached_instruction_results.get(instruction)?; + + let predicate = self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg); + let predicate = predicate.then_some(side_effects_enabled_var); + + results_for_instruction.get(&predicate)?.get(block, &mut self.dom) + } + + /// Checks if the given instruction is a call to a brillig function with all constant arguments. + /// If so, we can try to evaluate that function and replace the results with the evaluation results. + fn try_inline_brillig_call_with_all_constants( + instruction: &Instruction, + old_results: &[ValueId], + block: BasicBlockId, + dfg: &mut DataFlowGraph, + brillig_info: Option, + ) -> Option> { + let evaluation_result = Self::evaluate_const_brillig_call( + instruction, + brillig_info?.brillig, + brillig_info?.brillig_functions, + dfg, + ); + + match evaluation_result { + EvaluationResult::NotABrilligCall | EvaluationResult::CannotEvaluate(_) => None, + EvaluationResult::Evaluated(memory_values) => { + let mut memory_index = 0; + let new_results = vecmap(old_results, |old_result| { + let typ = dfg.type_of_value(*old_result); + Self::new_value_for_type_and_memory_values( + typ, + block, + &memory_values, + &mut memory_index, + dfg, + ) + }); + Some(new_results) + } + } + } - // See if there's a cached version with no predicate first - if let Some(results) = results_for_instruction.and_then(|map| map.get(&None)) { - return Some(results); + /// Tries to evaluate an instruction if it's a call that points to a brillig function, + /// and all its arguments are constant. + /// We do this by directly executing the function with a brillig VM. + fn evaluate_const_brillig_call( + instruction: &Instruction, + brillig: &Brillig, + brillig_functions: &BTreeMap, + dfg: &mut DataFlowGraph, + ) -> EvaluationResult { + let Instruction::Call { func: func_id, arguments } = instruction else { + return EvaluationResult::NotABrilligCall; + }; + + let func_value = &dfg[*func_id]; + let Value::Function(func_id) = func_value else { + return EvaluationResult::NotABrilligCall; + }; + + let Some(func) = brillig_functions.get(func_id) else { + return EvaluationResult::NotABrilligCall; + }; + + if !arguments.iter().all(|argument| dfg.is_constant(*argument)) { + return EvaluationResult::CannotEvaluate(*func_id); + } + + let mut brillig_arguments = Vec::new(); + for argument in arguments { + let typ = dfg.type_of_value(*argument); + let Some(parameter) = type_to_brillig_parameter(&typ) else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + brillig_arguments.push(parameter); + } + + // Check that return value types are supported by brillig + for return_id in func.returns().iter() { + let typ = func.dfg.type_of_value(*return_id); + if type_to_brillig_parameter(&typ).is_none() { + return EvaluationResult::CannotEvaluate(*func_id); + } + } + + let Ok(generated_brillig) = gen_brillig_for(func, brillig_arguments, brillig) else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + + let mut calldata = Vec::new(); + for argument in arguments { + value_id_to_calldata(*argument, dfg, &mut calldata); + } + + let bytecode = &generated_brillig.byte_code; + let foreign_call_results = Vec::new(); + let black_box_solver = Bn254BlackBoxSolver; + let profiling_active = false; + let mut vm = + VM::new(calldata, bytecode, foreign_call_results, &black_box_solver, profiling_active); + let vm_status: VMStatus<_> = vm.process_opcodes(); + let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { + return EvaluationResult::CannotEvaluate(*func_id); + }; + + let memory = + vm.get_memory()[return_data_offset..(return_data_offset + return_data_size)].to_vec(); + + EvaluationResult::Evaluated(memory) + } + + /// Creates a new value inside this function by reading it from `memory_values` starting at + /// `memory_index` depending on the given Type: if it's an array multiple values will be read + /// and a new `make_array` instruction will be created. + fn new_value_for_type_and_memory_values( + typ: Type, + block_id: BasicBlockId, + memory_values: &[MemoryValue], + memory_index: &mut usize, + dfg: &mut DataFlowGraph, + ) -> ValueId { + match typ { + Type::Numeric(_) => { + let memory = memory_values[*memory_index]; + *memory_index += 1; + + let field_value = match memory { + MemoryValue::Field(field_value) => field_value, + MemoryValue::Integer(u128_value, _) => u128_value.into(), + }; + dfg.make_constant(field_value, typ) + } + Type::Array(types, length) => { + let mut new_array_values = Vector::new(); + for _ in 0..length { + for typ in types.iter() { + let new_value = Self::new_value_for_type_and_memory_values( + typ.clone(), + block_id, + memory_values, + memory_index, + dfg, + ); + new_array_values.push_back(new_value); + } + } + + let instruction = Instruction::MakeArray { + elements: new_array_values, + typ: Type::Array(types, length), + }; + let instruction_id = dfg.make_instruction(instruction, None); + dfg[block_id].instructions_mut().push(instruction_id); + *dfg.instruction_results(instruction_id).first().unwrap() + } + Type::Reference(_) => { + panic!("Unexpected reference type in brillig function result") + } + Type::Slice(_) => { + panic!("Unexpected slice type in brillig function result") + } + Type::Function => { + panic!("Unexpected function type in brillig function result") + } } + } +} + +impl ResultCache { + /// Records that an `Instruction` in block `block` produced the result values `results`. + fn cache(&mut self, block: BasicBlockId, results: Vec) { + if self.result.is_none() { + self.result = Some((block, results)); + } + } + + /// Returns a set of [`ValueId`]s produced from a copy of this [`Instruction`] which sits + /// within a block which dominates `block`. + /// + /// We require that the cached instruction's block dominates `block` in order to avoid + /// cycles causing issues (e.g. two instructions being replaced with the results of each other + /// such that neither instruction exists anymore.) + fn get(&self, block: BasicBlockId, dom: &mut DominatorTree) -> Option { + self.result.as_ref().map(|(origin_block, results)| { + if dom.dominates(*origin_block, block) { + CacheResult::Cached(results) + } else { + // Insert a copy of this instruction in the common dominator + let dominator = dom.common_dominator(*origin_block, block); + CacheResult::NeedToHoistToCommonBlock(dominator, results) + } + }) + } +} + +enum CacheResult<'a> { + Cached(&'a [ValueId]), + NeedToHoistToCommonBlock(BasicBlockId, &'a [ValueId]), +} + +/// Result of trying to evaluate an instruction (any instruction) in this pass. +enum EvaluationResult { + /// Nothing was done because the instruction wasn't a call to a brillig function, + /// or some arguments to it were not constants. + NotABrilligCall, + /// The instruction was a call to a brillig function, but we couldn't evaluate it. + /// This can occur in the situation where the brillig function reaches a "trap" or a foreign call opcode. + CannotEvaluate(FunctionId), + /// The instruction was a call to a brillig function and we were able to evaluate it, + /// returning evaluation memory values. + Evaluated(Vec>), +} + +/// Similar to FunctionContext::ssa_type_to_parameter but never panics and disallows reference types. +pub(crate) fn type_to_brillig_parameter(typ: &Type) -> Option { + match typ { + Type::Numeric(_) => Some(BrilligParameter::SingleAddr(get_bit_size_from_ssa_type(typ))), + Type::Array(item_type, size) => { + let mut parameters = Vec::with_capacity(item_type.len()); + for item_typ in item_type.iter() { + parameters.push(type_to_brillig_parameter(item_typ)?); + } + Some(BrilligParameter::Array(parameters, *size)) + } + _ => None, + } +} - let predicate = - instruction.requires_acir_gen_predicate(dfg).then_some(side_effects_enabled_var); +fn value_id_to_calldata(value_id: ValueId, dfg: &DataFlowGraph, calldata: &mut Vec) { + if let Some(value) = dfg.get_numeric_constant(value_id) { + calldata.push(value); + return; + } - results_for_instruction.and_then(|map| map.get(&predicate)) + if let Some((values, _type)) = dfg.get_array_constant(value_id) { + for value in values { + value_id_to_calldata(value, dfg, calldata); + } + return; } + + panic!("Expected ValueId to be numeric constant or array constant"); } #[cfg(test)] @@ -547,22 +923,32 @@ mod test { // Regression for #4600 #[test] fn array_get_regression() { + // fn main f0 { + // b0(v0: u1, v1: u64): + // enable_side_effects_if v0 + // v2 = make_array [Field 0, Field 1] + // v3 = array_get v2, index v1 + // v4 = not v0 + // enable_side_effects_if v4 + // v5 = array_get v2, index v1 + // } + // // We want to make sure after constant folding both array_gets remain since they are // under different enable_side_effects_if contexts and thus one may be disabled while // the other is not. If one is removed, it is possible e.g. v4 is replaced with v2 which // is disabled (only gets from index 0) and thus returns the wrong result. let src = " - acir(inline) fn main f0 { - b0(v0: u1, v1: u64): - enable_side_effects v0 - v4 = make_array [Field 0, Field 1] : [Field; 2] - v5 = array_get v4, index v1 -> Field - v6 = not v0 - enable_side_effects v6 - v7 = array_get v4, index v1 -> Field - return - } - "; + acir(inline) fn main f0 { + b0(v0: u1, v1: u64): + enable_side_effects v0 + v4 = make_array [Field 0, Field 1] : [Field; 2] + v5 = array_get v4, index v1 -> Field + v6 = not v0 + enable_side_effects v6 + v7 = array_get v4, index v1 -> Field + return + } + "; let ssa = Ssa::from_str(src).unwrap(); // Expected output is unchanged @@ -620,14 +1006,14 @@ mod test { assert_normalized_ssa_equals(ssa, expected); } - // This test currently fails. It being fixed will address the issue https://github.com/noir-lang/noir/issues/5756 #[test] - #[should_panic] fn constant_array_deduplication() { // fn main f0 { // b0(v0: u64): - // v5 = call keccakf1600([v0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0]) - // v6 = call keccakf1600([v0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0]) + // v1 = make_array [v0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0] + // v2 = make_array [v0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0] + // v5 = call keccakf1600(v1) + // v6 = call keccakf1600(v2) // } // // Here we're checking a situation where two identical arrays are being initialized twice and being assigned separate `ValueId`s. @@ -647,12 +1033,13 @@ mod test { let array1 = builder.insert_make_array(array_contents.clone(), typ.clone()); let array2 = builder.insert_make_array(array_contents, typ.clone()); - assert_eq!(array1, array2, "arrays were assigned different value ids"); + assert_ne!(array1, array2, "arrays were not assigned different value ids"); let keccakf1600 = builder.import_intrinsic("keccakf1600").expect("keccakf1600 intrinsic should exist"); let _v10 = builder.insert_call(keccakf1600, vec![array1], vec![typ.clone()]); let _v11 = builder.insert_call(keccakf1600, vec![array2], vec![typ.clone()]); + builder.terminate_with_return(Vec::new()); let mut ssa = builder.finish(); ssa.normalize_ids(); @@ -662,8 +1049,13 @@ mod test { let main = ssa.main(); let instructions = main.dfg[main.entry_block()].instructions(); let starting_instruction_count = instructions.len(); - assert_eq!(starting_instruction_count, 2); + assert_eq!(starting_instruction_count, 4); + // fn main f0 { + // b0(v0: u64): + // v1 = make_array [v0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0, u64 0] + // v5 = call keccakf1600(v1) + // } let ssa = ssa.fold_constants(); println!("{ssa}"); @@ -671,6 +1063,282 @@ mod test { let main = ssa.main(); let instructions = main.dfg[main.entry_block()].instructions(); let ending_instruction_count = instructions.len(); - assert_eq!(ending_instruction_count, 1); + assert_eq!(ending_instruction_count, 2); + } + + #[test] + fn deduplicate_across_blocks() { + // fn main f0 { + // b0(v0: u1): + // v1 = not v0 + // jmp b1() + // b1(): + // v2 = not v0 + // return v2 + // } + let main_id = Id::test_new(0); + + // Compiling main + let mut builder = FunctionBuilder::new("main".into(), main_id); + let b1 = builder.insert_block(); + + let v0 = builder.add_parameter(Type::bool()); + let _v1 = builder.insert_not(v0); + builder.terminate_with_jmp(b1, Vec::new()); + + builder.switch_to_block(b1); + let v2 = builder.insert_not(v0); + builder.terminate_with_return(vec![v2]); + + let ssa = builder.finish(); + let main = ssa.main(); + assert_eq!(main.dfg[main.entry_block()].instructions().len(), 1); + assert_eq!(main.dfg[b1].instructions().len(), 1); + + // Expected output: + // + // fn main f0 { + // b0(v0: u1): + // v1 = not v0 + // jmp b1() + // b1(): + // return v1 + // } + let ssa = ssa.fold_constants_using_constraints(); + let main = ssa.main(); + assert_eq!(main.dfg[main.entry_block()].instructions().len(), 1); + assert_eq!(main.dfg[b1].instructions().len(), 0); + } + + #[test] + fn deduplicate_across_non_dominated_blocks() { + let src = " + brillig(inline) fn main f0 { + b0(v0: u32): + v2 = lt u32 1000, v0 + jmpif v2 then: b1, else: b2 + b1(): + v4 = add v0, u32 1 + v5 = lt v0, v4 + constrain v5 == u1 1 + jmp b2() + b2(): + v7 = lt u32 1000, v0 + jmpif v7 then: b3, else: b4 + b3(): + v8 = add v0, u32 1 + v9 = lt v0, v8 + constrain v9 == u1 1 + jmp b4() + b4(): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + + // v4 has been hoisted, although: + // - v5 has not yet been removed since it was encountered earlier in the program + // - v8 hasn't been recognized as a duplicate of v6 yet since they still reference v4 and + // v5 respectively + let expected = " + brillig(inline) fn main f0 { + b0(v0: u32): + v2 = lt u32 1000, v0 + v4 = add v0, u32 1 + jmpif v2 then: b1, else: b2 + b1(): + v5 = add v0, u32 1 + v6 = lt v0, v5 + constrain v6 == u1 1 + jmp b2() + b2(): + jmpif v2 then: b3, else: b4 + b3(): + v8 = lt v0, v4 + constrain v8 == u1 1 + jmp b4() + b4(): + return + } + "; + + let ssa = ssa.fold_constants_using_constraints(); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_without_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1() -> Field + return v0 + } + + brillig(inline) fn one f1 { + b0(): + v0 = add Field 2, Field 3 + return v0 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_two_field_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, Field 3) -> Field + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: Field): + v2 = add v0, v1 + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_two_i32_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(i32 2, i32 3) -> i32 + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: i32, v1: i32): + v2 = add v0, v1 + return v2 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + return i32 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_array_return() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, Field 3, Field 4) -> [Field; 3] + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: Field, v2: Field): + v3 = make_array [v0, v1, v2] : [Field; 3] + return v3 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v3 = make_array [Field 2, Field 3, Field 4] : [Field; 3] + return v3 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_composite_array_return() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = call f1(Field 2, i32 3, Field 4, i32 5) -> [(Field, i32); 2] + return v0 + } + + brillig(inline) fn one f1 { + b0(v0: Field, v1: i32, v2: i32, v3: Field): + v4 = make_array [v0, v1, v2, v3] : [(Field, i32); 2] + return v4 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v4 = make_array [Field 2, i32 3, Field 4, i32 5] : [(Field, i32); 2] + return v4 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn inlines_brillig_call_with_array_arguments() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = make_array [Field 2, Field 3] : [Field; 2] + v1 = call f1(v0) -> Field + return v1 + } + + brillig(inline) fn one f1 { + b0(v0: [Field; 2]): + inc_rc v0 + v2 = array_get v0, index u32 0 -> Field + v4 = array_get v0, index u32 1 -> Field + v5 = add v2, v4 + dec_rc v0 + return v5 + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let brillig = ssa.to_brillig(false); + + let expected = " + acir(inline) fn main f0 { + b0(): + v2 = make_array [Field 2, Field 3] : [Field; 2] + return Field 5 + } + "; + let ssa = ssa.fold_constants_with_brillig(&brillig); + assert_normalized_ssa_equals(ssa, expected); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs index 666a8e32246..8d3fa9cc615 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -172,7 +172,7 @@ impl Context { fn is_unused(&self, instruction_id: InstructionId, function: &Function) -> bool { let instruction = &function.dfg[instruction_id]; - if instruction.can_eliminate_if_unused(&function.dfg) { + if instruction.can_eliminate_if_unused(function) { let results = function.dfg.instruction_results(instruction_id); results.iter().all(|result| !self.used_values.contains(result)) } else if let Instruction::Call { func, arguments } = instruction { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index a2b8e20d20f..61a93aee58d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -132,7 +132,6 @@ //! v12 = add v10, v11 //! store v12 at v5 (new store) use fxhash::FxHashMap as HashMap; -use std::collections::{BTreeMap, HashSet}; use acvm::{acir::AcirField, acir::BlackBoxFunc, FieldElement}; use iter_extended::vecmap; @@ -186,18 +185,6 @@ struct Context<'f> { /// Maps start of branch -> end of branch branch_ends: HashMap, - /// Maps an address to the old and new value of the element at that address - /// These only hold stores for one block at a time and is cleared - /// between inlining of branches. - store_values: HashMap, - - /// Stores all allocations local to the current branch. - /// Since these branches are local to the current branch (ie. only defined within one branch of - /// an if expression), they should not be merged with their previous value or stored value in - /// the other branch since there is no such value. The ValueId here is that which is returned - /// by the allocate instruction. - local_allocations: HashSet, - /// A stack of each jmpif condition that was taken to reach a particular point in the program. /// When two branches are merged back into one, this constitutes a join point, and is analogous /// to the rest of the program after an if statement. When such a join point / end block is @@ -216,13 +203,6 @@ struct Context<'f> { arguments_stack: Vec>, } -#[derive(Clone)] -pub(crate) struct Store { - old_value: ValueId, - new_value: ValueId, - call_stack: CallStack, -} - #[derive(Clone)] struct ConditionalBranch { // Contains the last processed block during the processing of the branch. @@ -231,10 +211,6 @@ struct ConditionalBranch { old_condition: ValueId, // The condition of the branch condition: ValueId, - // The store values accumulated when processing the branch - store_values: HashMap, - // The allocations accumulated when processing the branch - local_allocations: HashSet, } struct ConditionalContext { @@ -263,8 +239,6 @@ fn flatten_function_cfg(function: &mut Function, no_predicates: &HashMap Context<'f> { // If this is not a separate variable, clippy gets confused and says the to_vec is // unnecessary, when removing it actually causes an aliasing/mutability error. let instructions = self.inserter.function.dfg[block].instructions().to_vec(); + let mut previous_allocate_result = None; + for instruction in instructions.iter() { if self.is_no_predicate(no_predicates, instruction) { // disable side effect for no_predicate functions @@ -356,10 +332,10 @@ impl<'f> Context<'f> { None, im::Vector::new(), ); - self.push_instruction(*instruction); + self.push_instruction(*instruction, &mut previous_allocate_result); self.insert_current_side_effects_enabled(); } else { - self.push_instruction(*instruction); + self.push_instruction(*instruction, &mut previous_allocate_result); } } } @@ -429,13 +405,9 @@ impl<'f> Context<'f> { let old_condition = *condition; let then_condition = self.inserter.resolve(old_condition); - let old_stores = std::mem::take(&mut self.store_values); - let old_allocations = std::mem::take(&mut self.local_allocations); let branch = ConditionalBranch { old_condition, condition: self.link_condition(then_condition), - store_values: old_stores, - local_allocations: old_allocations, last_block: *then_destination, }; let cond_context = ConditionalContext { @@ -463,21 +435,11 @@ impl<'f> Context<'f> { ); let else_condition = self.link_condition(else_condition); - // Make sure the else branch sees the previous values of each store - // rather than any values created in the 'then' branch. - let old_stores = std::mem::take(&mut cond_context.then_branch.store_values); - cond_context.then_branch.store_values = std::mem::take(&mut self.store_values); - self.undo_stores_in_then_branch(&cond_context.then_branch.store_values); - - let old_allocations = std::mem::take(&mut self.local_allocations); let else_branch = ConditionalBranch { old_condition: cond_context.then_branch.old_condition, condition: else_condition, - store_values: old_stores, - local_allocations: old_allocations, last_block: *block, }; - cond_context.then_branch.local_allocations.clear(); cond_context.else_branch = Some(else_branch); self.condition_stack.push(cond_context); @@ -499,10 +461,7 @@ impl<'f> Context<'f> { } let mut else_branch = cond_context.else_branch.unwrap(); - let stores_in_branch = std::mem::replace(&mut self.store_values, else_branch.store_values); - self.local_allocations = std::mem::take(&mut else_branch.local_allocations); else_branch.last_block = *block; - else_branch.store_values = stores_in_branch; cond_context.else_branch = Some(else_branch); // We must remember to reset whether side effects are enabled when both branches @@ -560,7 +519,6 @@ impl<'f> Context<'f> { let instruction = Instruction::IfElse { then_condition: cond_context.then_branch.condition, then_value: then_arg, - else_condition: cond_context.else_branch.as_ref().unwrap().condition, else_value: else_arg, }; let call_stack = cond_context.call_stack.clone(); @@ -571,8 +529,6 @@ impl<'f> Context<'f> { .first() }); - let call_stack = cond_context.call_stack; - self.merge_stores(cond_context.then_branch, cond_context.else_branch, call_stack); self.arguments_stack.pop(); self.arguments_stack.pop(); self.arguments_stack.push(args); @@ -627,130 +583,45 @@ impl<'f> Context<'f> { self.insert_instruction_with_typevars(enable_side_effects, None, call_stack); } - /// Merge any store instructions found in each branch. - /// - /// This function relies on the 'then' branch being merged before the 'else' branch of a jmpif - /// instruction. If this ordering is changed, the ordering that store values are merged within - /// this function also needs to be changed to reflect that. - fn merge_stores( - &mut self, - then_branch: ConditionalBranch, - else_branch: Option, - call_stack: CallStack, - ) { - // Address -> (then_value, else_value, value_before_the_if) - let mut new_map = BTreeMap::new(); - - for (address, store) in then_branch.store_values { - new_map.insert(address, (store.new_value, store.old_value, store.old_value)); - } - - if else_branch.is_some() { - for (address, store) in else_branch.clone().unwrap().store_values { - if let Some(entry) = new_map.get_mut(&address) { - entry.1 = store.new_value; - } else { - new_map.insert(address, (store.old_value, store.new_value, store.old_value)); - } - } - } - - let then_condition = then_branch.condition; - let else_condition = if let Some(branch) = else_branch { - branch.condition - } else { - self.inserter.function.dfg.make_constant(FieldElement::zero(), Type::bool()) - }; - let block = self.inserter.function.entry_block(); - - // Merging must occur in a separate loop as we cannot borrow `self` as mutable while `value_merger` does - let mut new_values = HashMap::default(); - for (address, (then_case, else_case, _)) in &new_map { - let instruction = Instruction::IfElse { - then_condition, - then_value: *then_case, - else_condition, - else_value: *else_case, - }; - let dfg = &mut self.inserter.function.dfg; - let value = dfg - .insert_instruction_and_results(instruction, block, None, call_stack.clone()) - .first(); - - new_values.insert(address, value); - } - - // Replace stores with new merged values - for (address, (_, _, old_value)) in &new_map { - let value = new_values[address]; - let address = *address; - self.insert_instruction_with_typevars( - Instruction::Store { address, value }, - None, - call_stack.clone(), - ); - - if let Some(store) = self.store_values.get_mut(&address) { - store.new_value = value; - } else { - self.store_values.insert( - address, - Store { - old_value: *old_value, - new_value: value, - call_stack: call_stack.clone(), - }, - ); - } - } - } - - fn remember_store(&mut self, address: ValueId, new_value: ValueId, call_stack: CallStack) { - if !self.local_allocations.contains(&address) { - if let Some(store_value) = self.store_values.get_mut(&address) { - store_value.new_value = new_value; - } else { - let load = Instruction::Load { address }; - - let load_type = Some(vec![self.inserter.function.dfg.type_of_value(new_value)]); - let old_value = self - .insert_instruction_with_typevars(load.clone(), load_type, call_stack.clone()) - .first(); - - self.store_values.insert(address, Store { old_value, new_value, call_stack }); - } - } - } - /// Push the given instruction to the end of the entry block of the current function. /// /// Note that each ValueId of the instruction will be mapped via self.inserter.resolve. /// As a result, the instruction that will be pushed will actually be a new instruction /// with a different InstructionId from the original. The results of the given instruction /// will also be mapped to the results of the new instruction. - fn push_instruction(&mut self, id: InstructionId) -> Vec { + /// + /// `previous_allocate_result` should only be set to the result of an allocate instruction + /// if that instruction was the instruction immediately previous to this one - if there are + /// any instructions in between it should be None. + fn push_instruction( + &mut self, + id: InstructionId, + previous_allocate_result: &mut Option, + ) { let (instruction, call_stack) = self.inserter.map_instruction(id); - let instruction = self.handle_instruction_side_effects(instruction, call_stack.clone()); - let is_allocate = matches!(instruction, Instruction::Allocate); + let instruction = self.handle_instruction_side_effects( + instruction, + call_stack.clone(), + *previous_allocate_result, + ); + let instruction_is_allocate = matches!(&instruction, Instruction::Allocate); let entry = self.inserter.function.entry_block(); let results = self.inserter.push_instruction_value(instruction, id, entry, call_stack); - - // Remember an allocate was created local to this branch so that we do not try to merge store - // values across branches for it later. - if is_allocate { - self.local_allocations.insert(results.first()); - } - - results.results().into_owned() + *previous_allocate_result = instruction_is_allocate.then(|| results.first()); } /// If we are currently in a branch, we need to modify constrain instructions /// to multiply them by the branch's condition (see optimization #1 in the module comment). + /// + /// `previous_allocate_result` should only be set to the result of an allocate instruction + /// if that instruction was the instruction immediately previous to this one - if there are + /// any instructions in between it should be None. fn handle_instruction_side_effects( &mut self, instruction: Instruction, call_stack: CallStack, + previous_allocate_result: Option, ) -> Instruction { if let Some(condition) = self.get_last_condition() { match instruction { @@ -779,8 +650,32 @@ impl<'f> Context<'f> { Instruction::Constrain(lhs, rhs, message) } Instruction::Store { address, value } => { - self.remember_store(address, value, call_stack); - Instruction::Store { address, value } + // If this instruction immediately follows an allocate, and stores to that + // address there is no previous value to load and we don't need a merge anyway. + if Some(address) == previous_allocate_result { + Instruction::Store { address, value } + } else { + // Instead of storing `value`, store `if condition { value } else { previous_value }` + let typ = self.inserter.function.dfg.type_of_value(value); + let load = Instruction::Load { address }; + let previous_value = self + .insert_instruction_with_typevars( + load, + Some(vec![typ]), + call_stack.clone(), + ) + .first(); + + let instruction = Instruction::IfElse { + then_condition: condition, + then_value: value, + + else_value: previous_value, + }; + + let updated_value = self.insert_instruction(instruction, call_stack); + Instruction::Store { address, value: updated_value } + } } Instruction::RangeCheck { value, max_bit_size, assert_message } => { // Replace value with `value * predicate` to zero out value when predicate is inactive. @@ -902,22 +797,10 @@ impl<'f> Context<'f> { call_stack, ) } - - fn undo_stores_in_then_branch(&mut self, store_values: &HashMap) { - for (address, store) in store_values { - let address = *address; - let value = store.old_value; - let instruction = Instruction::Store { address, value }; - // Considering the location of undoing a store to be the same as the original store. - self.insert_instruction_with_typevars(instruction, None, store.call_stack.clone()); - } - } } #[cfg(test)] mod test { - use std::sync::Arc; - use acvm::acir::AcirField; use crate::ssa::{ @@ -958,11 +841,9 @@ mod test { v1 = not v0 enable_side_effects u1 1 v3 = cast v0 as Field - v4 = cast v1 as Field - v6 = mul v3, Field 3 - v8 = mul v4, Field 4 - v9 = add v6, v8 - return v9 + v5 = mul v3, Field -1 + v7 = add Field 4, v5 + return v7 } "; @@ -1022,16 +903,13 @@ mod test { b0(v0: u1, v1: &mut Field): enable_side_effects v0 v2 = load v1 -> Field - store Field 5 at v1 - v4 = not v0 - store v2 at v1 + v3 = cast v0 as Field + v5 = sub Field 5, v2 + v6 = mul v3, v5 + v7 = add v2, v6 + store v7 at v1 + v8 = not v0 enable_side_effects u1 1 - v6 = cast v0 as Field - v7 = cast v4 as Field - v8 = mul v6, Field 5 - v9 = mul v7, v2 - v10 = add v8, v9 - store v10 at v1 return } "; @@ -1062,19 +940,20 @@ mod test { b0(v0: u1, v1: &mut Field): enable_side_effects v0 v2 = load v1 -> Field - store Field 5 at v1 - v4 = not v0 - store v2 at v1 - enable_side_effects v4 - v5 = load v1 -> Field - store Field 6 at v1 + v3 = cast v0 as Field + v5 = sub Field 5, v2 + v6 = mul v3, v5 + v7 = add v2, v6 + store v7 at v1 + v8 = not v0 + enable_side_effects v8 + v9 = load v1 -> Field + v10 = cast v8 as Field + v12 = sub Field 6, v9 + v13 = mul v10, v12 + v14 = add v9, v13 + store v14 at v1 enable_side_effects u1 1 - v8 = cast v0 as Field - v9 = cast v4 as Field - v10 = mul v8, Field 5 - v11 = mul v9, Field 6 - v12 = add v10, v11 - store v12 at v1 return } "; @@ -1242,7 +1121,7 @@ mod test { }; let merged_values = get_all_constants_reachable_from_instruction(&main.dfg, ret); - assert_eq!(merged_values, vec![3, 5, 6]); + assert_eq!(merged_values, vec![1, 3, 5, 6]); } #[test] @@ -1380,63 +1259,73 @@ mod test { fn should_not_merge_incorrectly_to_false() { // Regression test for #1792 // Tests that it does not simplify a true constraint an always-false constraint - // acir(inline) fn main f1 { - // b0(v0: [u8; 2]): - // v5 = array_get v0, index u8 0 - // v6 = cast v5 as u32 - // v8 = truncate v6 to 1 bits, max_bit_size: 32 - // v9 = cast v8 as u1 - // v10 = allocate - // store u8 0 at v10 - // jmpif v9 then: b2, else: b3 - // b2(): - // v12 = cast v5 as Field - // v13 = add v12, Field 1 - // store v13 at v10 - // jmp b4() - // b4(): - // constrain v9 == u1 1 - // return - // b3(): - // store u8 0 at v10 - // jmp b4() - // } - let main_id = Id::test_new(1); - let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.insert_block(); // b0 - let b1 = builder.insert_block(); - let b2 = builder.insert_block(); - let b3 = builder.insert_block(); - let element_type = Arc::new(vec![Type::unsigned(8)]); - let array_type = Type::Array(element_type.clone(), 2); - let array = builder.add_parameter(array_type); - let zero = builder.numeric_constant(0_u128, Type::unsigned(8)); - let v5 = builder.insert_array_get(array, zero, Type::unsigned(8)); - let v6 = builder.insert_cast(v5, Type::unsigned(32)); - let i_two = builder.numeric_constant(2_u128, Type::unsigned(32)); - let v8 = builder.insert_binary(v6, BinaryOp::Mod, i_two); - let v9 = builder.insert_cast(v8, Type::bool()); - let v10 = builder.insert_allocate(Type::field()); - builder.insert_store(v10, zero); - builder.terminate_with_jmpif(v9, b1, b2); - builder.switch_to_block(b1); - let one = builder.field_constant(1_u128); - let v5b = builder.insert_cast(v5, Type::field()); - let v13: Id = builder.insert_binary(v5b, BinaryOp::Add, one); - let v14 = builder.insert_cast(v13, Type::unsigned(8)); - builder.insert_store(v10, v14); - builder.terminate_with_jmp(b3, vec![]); - builder.switch_to_block(b2); - builder.insert_store(v10, zero); - builder.terminate_with_jmp(b3, vec![]); - builder.switch_to_block(b3); - let v_true = builder.numeric_constant(true, Type::bool()); - let v12 = builder.insert_binary(v9, BinaryOp::Eq, v_true); - builder.insert_constrain(v12, v_true, None); - builder.terminate_with_return(vec![]); - let ssa = builder.finish(); + let src = " + acir(inline) fn main f0 { + b0(v0: [u8; 2]): + v2 = array_get v0, index u8 0 -> u8 + v3 = cast v2 as u32 + v4 = truncate v3 to 1 bits, max_bit_size: 32 + v5 = cast v4 as u1 + v6 = allocate -> &mut Field + store u8 0 at v6 + jmpif v5 then: b2, else: b1 + b2(): + v7 = cast v2 as Field + v9 = add v7, Field 1 + v10 = cast v9 as u8 + store v10 at v6 + jmp b3() + b3(): + constrain v5 == u1 1 + return + b1(): + store u8 0 at v6 + jmp b3() + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: [u8; 2]): + v2 = array_get v0, index u8 0 -> u8 + v3 = cast v2 as u32 + v4 = truncate v3 to 1 bits, max_bit_size: 32 + v5 = cast v4 as u1 + v6 = allocate -> &mut Field + store u8 0 at v6 + enable_side_effects v5 + v7 = cast v2 as Field + v9 = add v7, Field 1 + v10 = cast v9 as u8 + v11 = load v6 -> u8 + v12 = cast v4 as Field + v13 = cast v11 as Field + v14 = sub v9, v13 + v15 = mul v12, v14 + v16 = add v13, v15 + v17 = cast v16 as u8 + store v17 at v6 + v18 = not v5 + enable_side_effects v18 + v19 = load v6 -> u8 + v20 = cast v18 as Field + v21 = cast v19 as Field + v23 = sub Field 0, v21 + v24 = mul v20, v23 + v25 = add v21, v24 + v26 = cast v25 as u8 + store v26 at v6 + enable_side_effects u1 1 + constrain v5 == u1 1 + return + } + "; + let flattened_ssa = ssa.flatten_cfg(); let main = flattened_ssa.main(); + // Now assert that there is not an always-false constraint after flattening: let mut constrain_count = 0; for instruction in main.dfg[main.entry_block()].instructions() { @@ -1450,6 +1339,8 @@ mod test { } } assert_eq!(constrain_count, 1); + + assert_normalized_ssa_equals(flattened_ssa, expected); } #[test] diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index bee58278aa8..8ea26d4e96d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -45,7 +45,7 @@ impl<'a> ValueMerger<'a> { /// Merge two values a and b from separate basic blocks to a single value. /// If these two values are numeric, the result will be - /// `then_condition * then_value + else_condition * else_value`. + /// `then_condition * (then_value - else_value) + else_value`. /// Otherwise, if the values being merged are arrays, a new array will be made /// recursively from combining each element of both input arrays. /// @@ -54,7 +54,6 @@ impl<'a> ValueMerger<'a> { pub(crate) fn merge_values( &mut self, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -70,15 +69,14 @@ impl<'a> ValueMerger<'a> { self.dfg, self.block, then_condition, - else_condition, then_value, else_value, ), typ @ Type::Array(_, _) => { - self.merge_array_values(typ, then_condition, else_condition, then_value, else_value) + self.merge_array_values(typ, then_condition, then_value, else_value) } typ @ Type::Slice(_) => { - self.merge_slice_values(typ, then_condition, else_condition, then_value, else_value) + self.merge_slice_values(typ, then_condition, then_value, else_value) } Type::Reference(_) => panic!("Cannot return references from an if expression"), Type::Function => panic!("Cannot return functions from an if expression"), @@ -86,12 +84,11 @@ impl<'a> ValueMerger<'a> { } /// Merge two numeric values a and b from separate basic blocks to a single value. This - /// function would return the result of `if c { a } else { b }` as `c*a + (!c)*b`. + /// function would return the result of `if c { a } else { b }` as `c * (a-b) + b`. pub(crate) fn merge_numeric_values( dfg: &mut DataFlowGraph, block: BasicBlockId, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -114,31 +111,38 @@ impl<'a> ValueMerger<'a> { // We must cast the bool conditions to the actual numeric type used by each value. let then_condition = dfg .insert_instruction_and_results( - Instruction::Cast(then_condition, then_type), - block, - None, - call_stack.clone(), - ) - .first(); - let else_condition = dfg - .insert_instruction_and_results( - Instruction::Cast(else_condition, else_type), + Instruction::Cast(then_condition, Type::field()), block, None, call_stack.clone(), ) .first(); - let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); - let then_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let then_field = Instruction::Cast(then_value, Type::field()); + let then_field_value = + dfg.insert_instruction_and_results(then_field, block, None, call_stack.clone()).first(); - let mul = Instruction::binary(BinaryOp::Mul, else_condition, else_value); - let else_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let else_field = Instruction::Cast(else_value, Type::field()); + let else_field_value = + dfg.insert_instruction_and_results(else_field, block, None, call_stack.clone()).first(); + + let diff = Instruction::binary(BinaryOp::Sub, then_field_value, else_field_value); + let diff_value = + dfg.insert_instruction_and_results(diff, block, None, call_stack.clone()).first(); + + let conditional_diff = Instruction::binary(BinaryOp::Mul, then_condition, diff_value); + let conditional_diff_value = dfg + .insert_instruction_and_results(conditional_diff, block, None, call_stack.clone()) + .first(); + + let merged_field = + Instruction::binary(BinaryOp::Add, else_field_value, conditional_diff_value); + let merged_field_value = dfg + .insert_instruction_and_results(merged_field, block, None, call_stack.clone()) + .first(); - let add = Instruction::binary(BinaryOp::Add, then_value, else_value); - dfg.insert_instruction_and_results(add, block, None, call_stack).first() + let merged = Instruction::Cast(merged_field_value, then_type); + dfg.insert_instruction_and_results(merged, block, None, call_stack).first() } /// Given an if expression that returns an array: `if c { array1 } else { array2 }`, @@ -148,7 +152,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, ) -> ValueId { @@ -163,7 +166,6 @@ impl<'a> ValueMerger<'a> { if let Some(result) = self.try_merge_only_changed_indices( then_condition, - else_condition, then_value, else_value, actual_length, @@ -193,12 +195,7 @@ impl<'a> ValueMerger<'a> { let then_element = get_element(then_value, typevars.clone()); let else_element = get_element(else_value, typevars); - merged.push_back(self.merge_values( - then_condition, - else_condition, - then_element, - else_element, - )); + merged.push_back(self.merge_values(then_condition, then_element, else_element)); } } @@ -211,7 +208,6 @@ impl<'a> ValueMerger<'a> { &mut self, typ: Type, then_condition: ValueId, - else_condition: ValueId, then_value_id: ValueId, else_value_id: ValueId, ) -> ValueId { @@ -269,12 +265,7 @@ impl<'a> ValueMerger<'a> { let else_element = get_element(else_value_id, typevars, else_len * element_types.len()); - merged.push_back(self.merge_values( - then_condition, - else_condition, - then_element, - else_element, - )); + merged.push_back(self.merge_values(then_condition, then_element, else_element)); } } @@ -323,7 +314,6 @@ impl<'a> ValueMerger<'a> { fn try_merge_only_changed_indices( &mut self, then_condition: ValueId, - else_condition: ValueId, then_value: ValueId, else_value: ValueId, array_length: usize, @@ -407,8 +397,7 @@ impl<'a> ValueMerger<'a> { let then_element = get_element(then_value, typevars.clone()); let else_element = get_element(else_value, typevars); - let value = - self.merge_values(then_condition, else_condition, then_element, else_element); + let value = self.merge_values(then_condition, then_element, else_element); array = self.insert_array_set(array, index, value, Some(condition)).first(); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs new file mode 100644 index 00000000000..14233ca73e5 --- /dev/null +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -0,0 +1,378 @@ +//! The loop invariant code motion pass moves code from inside a loop to before the loop +//! if that code will always have the same result on every iteration of the loop. +//! +//! To identify a loop invariant, check whether all of an instruction's values are: +//! - Outside of the loop +//! - Constant +//! - Already marked as loop invariants +//! +//! We also check that we are not hoisting instructions with side effects. +use fxhash::FxHashSet as HashSet; + +use crate::ssa::{ + ir::{ + basic_block::BasicBlockId, + function::{Function, RuntimeType}, + function_inserter::FunctionInserter, + instruction::InstructionId, + value::ValueId, + }, + Ssa, +}; + +use super::unrolling::{Loop, Loops}; + +impl Ssa { + #[tracing::instrument(level = "trace", skip(self))] + pub(crate) fn loop_invariant_code_motion(mut self) -> Ssa { + let brillig_functions = self + .functions + .iter_mut() + .filter(|(_, func)| matches!(func.runtime(), RuntimeType::Brillig(_))); + + for (_, function) in brillig_functions { + function.loop_invariant_code_motion(); + } + + self + } +} + +impl Function { + fn loop_invariant_code_motion(&mut self) { + Loops::find_all(self).hoist_loop_invariants(self); + } +} + +impl Loops { + fn hoist_loop_invariants(self, function: &mut Function) { + let mut context = LoopInvariantContext::new(function); + + for loop_ in self.yet_to_unroll.iter() { + let Ok(pre_header) = loop_.get_pre_header(context.inserter.function, &self.cfg) else { + // If the loop does not have a preheader we skip hoisting loop invariants for this loop + continue; + }; + context.hoist_loop_invariants(loop_, pre_header); + } + + context.map_dependent_instructions(); + } +} + +struct LoopInvariantContext<'f> { + inserter: FunctionInserter<'f>, + defined_in_loop: HashSet, + loop_invariants: HashSet, +} + +impl<'f> LoopInvariantContext<'f> { + fn new(function: &'f mut Function) -> Self { + Self { + inserter: FunctionInserter::new(function), + defined_in_loop: HashSet::default(), + loop_invariants: HashSet::default(), + } + } + + fn hoist_loop_invariants(&mut self, loop_: &Loop, pre_header: BasicBlockId) { + self.set_values_defined_in_loop(loop_); + + for block in loop_.blocks.iter() { + for instruction_id in self.inserter.function.dfg[*block].take_instructions() { + let hoist_invariant = self.can_hoist_invariant(instruction_id); + + if hoist_invariant { + self.inserter.push_instruction(instruction_id, pre_header); + } else { + self.inserter.push_instruction(instruction_id, *block); + } + + self.update_values_defined_in_loop_and_invariants(instruction_id, hoist_invariant); + } + } + } + + /// Gather the variables declared within the loop + fn set_values_defined_in_loop(&mut self, loop_: &Loop) { + for block in loop_.blocks.iter() { + let params = self.inserter.function.dfg.block_parameters(*block); + self.defined_in_loop.extend(params); + for instruction_id in self.inserter.function.dfg[*block].instructions() { + let results = self.inserter.function.dfg.instruction_results(*instruction_id); + self.defined_in_loop.extend(results); + } + } + } + + /// Update any values defined in the loop and loop invariants after a + /// analyzing and re-inserting a loop's instruction. + fn update_values_defined_in_loop_and_invariants( + &mut self, + instruction_id: InstructionId, + hoist_invariant: bool, + ) { + let results = self.inserter.function.dfg.instruction_results(instruction_id).to_vec(); + // We will have new IDs after pushing instructions. + // We should mark the resolved result IDs as also being defined within the loop. + let results = + results.into_iter().map(|value| self.inserter.resolve(value)).collect::>(); + self.defined_in_loop.extend(results.iter()); + + // We also want the update result IDs when we are marking loop invariants as we may not + // be going through the blocks of the loop in execution order + if hoist_invariant { + // Track already found loop invariants + self.loop_invariants.extend(results.iter()); + } + } + + fn can_hoist_invariant(&mut self, instruction_id: InstructionId) -> bool { + let mut is_loop_invariant = true; + // The list of blocks for a nested loop contain any inner loops as well. + // We may have already re-inserted new instructions if two loops share blocks + // so we need to map all the values in the instruction which we want to check. + let (instruction, _) = self.inserter.map_instruction(instruction_id); + instruction.for_each_value(|value| { + // If an instruction value is defined in the loop and not already a loop invariant + // the instruction results are not loop invariants. + // + // We are implicitly checking whether the values are constant as well. + // The set of values defined in the loop only contains instruction results and block parameters + // which cannot be constants. + is_loop_invariant &= + !self.defined_in_loop.contains(&value) || self.loop_invariants.contains(&value); + }); + is_loop_invariant && instruction.can_be_deduplicated(&self.inserter.function.dfg, false) + } + + fn map_dependent_instructions(&mut self) { + let blocks = self.inserter.function.reachable_blocks(); + for block in blocks { + for instruction_id in self.inserter.function.dfg[block].take_instructions() { + self.inserter.push_instruction(instruction_id, block); + } + self.inserter.map_terminator_in_place(block); + } + } +} + +#[cfg(test)] +mod test { + use crate::ssa::opt::assert_normalized_ssa_equals; + use crate::ssa::Ssa; + + #[test] + fn simple_loop_invariant_code_motion() { + let src = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + jmp b1(u32 0) + b1(v2: u32): + v5 = lt v2, u32 4 + jmpif v5 then: b3, else: b2 + b3(): + v6 = mul v0, v1 + constrain v6 == u32 6 + v8 = add v2, u32 1 + jmp b1(v8) + b2(): + return + } + "; + + let mut ssa = Ssa::from_str(src).unwrap(); + let main = ssa.main_mut(); + + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 0); // The final return is not counted + + // `v6 = mul v0, v1` in b3 should now be `v3 = mul v0, v1` in b0 + let expected = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v3 = mul v0, v1 + jmp b1(u32 0) + b1(v2: u32): + v6 = lt v2, u32 4 + jmpif v6 then: b3, else: b2 + b3(): + constrain v3 == u32 6 + v9 = add v2, u32 1 + jmp b1(v9) + b2(): + return + } + "; + + let ssa = ssa.loop_invariant_code_motion(); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn nested_loop_invariant_code_motion() { + // Check that a loop invariant in the inner loop of a nested loop + // is hoisted to the parent loop's pre-header block. + let src = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + jmp b1(u32 0) + b1(v2: u32): + v6 = lt v2, u32 4 + jmpif v6 then: b3, else: b2 + b3(): + jmp b4(u32 0) + b4(v3: u32): + v7 = lt v3, u32 4 + jmpif v7 then: b6, else: b5 + b6(): + v10 = mul v0, v1 + constrain v10 == u32 6 + v12 = add v3, u32 1 + jmp b4(v12) + b5(): + v9 = add v2, u32 1 + jmp b1(v9) + b2(): + return + } + "; + + let mut ssa = Ssa::from_str(src).unwrap(); + let main = ssa.main_mut(); + + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 0); // The final return is not counted + + // `v10 = mul v0, v1` in b6 should now be `v4 = mul v0, v1` in b0 + let expected = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v4 = mul v0, v1 + jmp b1(u32 0) + b1(v2: u32): + v7 = lt v2, u32 4 + jmpif v7 then: b3, else: b2 + b3(): + jmp b4(u32 0) + b4(v3: u32): + v8 = lt v3, u32 4 + jmpif v8 then: b6, else: b5 + b6(): + constrain v4 == u32 6 + v12 = add v3, u32 1 + jmp b4(v12) + b5(): + v10 = add v2, u32 1 + jmp b1(v10) + b2(): + return + } + "; + + let ssa = ssa.loop_invariant_code_motion(); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn hoist_invariant_with_invariant_as_argument() { + // Check that an instruction which has arguments defined in the loop + // but which are already marked loop invariants is still hoisted to the preheader. + // + // For example, in b3 we have the following instructions: + // ```text + // v6 = mul v0, v1 + // v7 = mul v6, v0 + // ``` + // `v6` should be marked a loop invariants as `v0` and `v1` are both declared outside of the loop. + // As we will be hoisting `v6 = mul v0, v1` to the loop preheader we know that we can also + // hoist `v7 = mul v6, v0`. + let src = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + jmp b1(u32 0) + b1(v2: u32): + v5 = lt v2, u32 4 + jmpif v5 then: b3, else: b2 + b3(): + v6 = mul v0, v1 + v7 = mul v6, v0 + v8 = eq v7, u32 12 + constrain v7 == u32 12 + v9 = add v2, u32 1 + jmp b1(v9) + b2(): + return + } + "; + + let mut ssa = Ssa::from_str(src).unwrap(); + let main = ssa.main_mut(); + + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 0); // The final return is not counted + + let expected = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v3 = mul v0, v1 + v4 = mul v3, v0 + v6 = eq v4, u32 12 + jmp b1(u32 0) + b1(v2: u32): + v9 = lt v2, u32 4 + jmpif v9 then: b3, else: b2 + b3(): + constrain v4 == u32 12 + v11 = add v2, u32 1 + jmp b1(v11) + b2(): + return + } + "; + + let ssa = ssa.loop_invariant_code_motion(); + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn do_not_hoist_instructions_with_side_effects() { + // In `v12 = load v5` in `b3`, `v5` is defined outside the loop. + // However, as the instruction has side effects, we want to make sure + // we do not hoist the instruction to the loop preheader. + let src = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v4 = make_array [u32 0, u32 0, u32 0, u32 0, u32 0] : [u32; 5] + inc_rc v4 + v5 = allocate -> &mut [u32; 5] + store v4 at v5 + jmp b1(u32 0) + b1(v2: u32): + v7 = lt v2, u32 4 + jmpif v7 then: b3, else: b2 + b3(): + v12 = load v5 -> [u32; 5] + v13 = array_set v12, index v0, value v1 + store v13 at v5 + v15 = add v2, u32 1 + jmp b1(v15) + b2(): + v8 = load v5 -> [u32; 5] + v10 = array_get v8, index u32 2 -> u32 + constrain v10 == u32 3 + return + } + "; + + let mut ssa = Ssa::from_str(src).unwrap(); + let main = ssa.main_mut(); + + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 4); // The final return is not counted + + let ssa = ssa.loop_invariant_code_motion(); + // The code should be unchanged + assert_normalized_ssa_equals(ssa, src); + } +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 0690dbbf204..77133d7d88d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -415,11 +415,13 @@ impl<'f> PerFunctionContext<'f> { let address = self.inserter.function.dfg.resolve(*address); let value = self.inserter.function.dfg.resolve(*value); + // FIXME: This causes errors in the sha256 tests + // // If there was another store to this instruction without any (unremoved) loads or // function calls in-between, we can remove the previous store. - if let Some(last_store) = references.last_stores.get(&address) { - self.instructions_to_remove.insert(*last_store); - } + // if let Some(last_store) = references.last_stores.get(&address) { + // self.instructions_to_remove.insert(*last_store); + // } if self.inserter.function.dfg.value_is_reference(value) { if let Some(expression) = references.expressions.get(&value) { @@ -894,16 +896,19 @@ mod tests { // We would need to track whether the store where `v9` is the store value gets removed to know whether // to remove it. assert_eq!(count_stores(main.entry_block(), &main.dfg), 1); + // The first store in b1 is removed since there is another store to the same reference // in the same block, and the store is not needed before the later store. // The rest of the stores are also removed as no loads are done within any blocks // to the stored values. - assert_eq!(count_stores(b1, &main.dfg), 0); + // + // NOTE: This store is not removed due to the FIXME when handling Instruction::Store. + assert_eq!(count_stores(b1, &main.dfg), 1); let b1_instructions = main.dfg[b1].instructions(); - // We expect the last eq to be optimized out - assert_eq!(b1_instructions.len(), 0); + // We expect the last eq to be optimized out, only the store from above remains + assert_eq!(b1_instructions.len(), 1); } #[test] diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs index 10e86c6601a..06481a12f60 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -12,6 +12,7 @@ mod defunctionalize; mod die; pub(crate) mod flatten_cfg; mod inlining; +mod loop_invariant; mod mem2reg; mod normalize_value_ids; mod rc; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs index 0517f9ef89f..f735d9300ce 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs @@ -180,6 +180,8 @@ impl Context { | Intrinsic::AsWitness | Intrinsic::IsUnconstrained | Intrinsic::DerivePedersenGenerators + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::FieldLessThan => false, }, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index c387e0b6234..8e25c3f0a35 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -66,10 +66,9 @@ impl Context { for instruction in instructions { match &function.dfg[instruction] { - Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + Instruction::IfElse { then_condition, then_value, else_value } => { let then_condition = *then_condition; let then_value = *then_value; - let else_condition = *else_condition; let else_value = *else_value; let typ = function.dfg.type_of_value(then_value); @@ -85,12 +84,7 @@ impl Context { call_stack, ); - let value = value_merger.merge_values( - then_condition, - else_condition, - then_value, - else_value, - ); + let value = value_merger.merge_values(then_condition, then_value, else_value); let _typ = function.dfg.type_of_value(value); let results = function.dfg.instruction_results(instruction); @@ -238,6 +232,8 @@ fn slice_capacity_change( | Intrinsic::DerivePedersenGenerators | Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) + | Intrinsic::ArrayRefCount + | Intrinsic::SliceRefCount | Intrinsic::FieldLessThan => SizeChange::None, } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 89f1b2b2d7d..777c16dacd1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -18,8 +18,6 @@ //! //! When unrolling ACIR code, we remove reference count instructions because they are //! only used by Brillig bytecode. -use std::collections::HashSet; - use acvm::{acir::AcirField, FieldElement}; use crate::{ @@ -39,7 +37,7 @@ use crate::{ ssa_gen::Ssa, }, }; -use fxhash::FxHashMap as HashMap; +use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; impl Ssa { /// Loop unrolling can return errors, since ACIR functions need to be fully unrolled. @@ -84,7 +82,7 @@ impl Function { } } -struct Loop { +pub(super) struct Loop { /// The header block of a loop is the block which dominates all the /// other blocks in the loop. header: BasicBlockId, @@ -94,17 +92,17 @@ struct Loop { back_edge_start: BasicBlockId, /// All the blocks contained within the loop, including `header` and `back_edge_start`. - blocks: HashSet, + pub(super) blocks: HashSet, } -struct Loops { +pub(super) struct Loops { /// The loops that failed to be unrolled so that we do not try to unroll them again. /// Each loop is identified by its header block id. failed_to_unroll: HashSet, - yet_to_unroll: Vec, + pub(super) yet_to_unroll: Vec, modified_blocks: HashSet, - cfg: ControlFlowGraph, + pub(super) cfg: ControlFlowGraph, } impl Loops { @@ -136,7 +134,7 @@ impl Loops { /// loop_end loop_body /// ``` /// `loop_entry` has two predecessors: `main` and `loop_body`, and it dominates `loop_body`. - fn find_all(function: &Function) -> Self { + pub(super) fn find_all(function: &Function) -> Self { let cfg = ControlFlowGraph::with_function(function); let post_order = PostOrder::with_function(function); let mut dom_tree = DominatorTree::with_cfg_and_post_order(&cfg, &post_order); @@ -163,9 +161,9 @@ impl Loops { loops.sort_by_key(|loop_| loop_.blocks.len()); Self { - failed_to_unroll: HashSet::new(), + failed_to_unroll: HashSet::default(), yet_to_unroll: loops, - modified_blocks: HashSet::new(), + modified_blocks: HashSet::default(), cfg, } } @@ -209,7 +207,7 @@ impl Loop { back_edge_start: BasicBlockId, cfg: &ControlFlowGraph, ) -> Self { - let mut blocks = HashSet::new(); + let mut blocks = HashSet::default(); blocks.insert(header); let mut insert = |block, stack: &mut Vec| { @@ -393,7 +391,7 @@ impl Loop { /// The loop pre-header is the block that comes before the loop begins. Generally a header block /// is expected to have 2 predecessors: the pre-header and the final block of the loop which jumps /// back to the beginning. Other predecessors can come from `break` or `continue`. - fn get_pre_header( + pub(super) fn get_pre_header( &self, function: &Function, cfg: &ControlFlowGraph, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/ast.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/ast.rs index a34b7fd70d3..6c7608a2f16 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/ast.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/ast.rs @@ -89,6 +89,7 @@ pub(crate) enum ParsedInstruction { Constrain { lhs: ParsedValue, rhs: ParsedValue, + assert_message: Option, }, DecrementRc { value: ParsedValue, @@ -129,6 +130,12 @@ pub(crate) enum ParsedInstruction { }, } +#[derive(Debug)] +pub(crate) enum AssertMessage { + Static(String), + Dynamic(Vec), +} + #[derive(Debug)] pub(crate) enum ParsedTerminator { Jmp { destination: Identifier, arguments: Vec }, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 552ac0781c7..e78cbbd75a1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -1,13 +1,18 @@ use std::collections::HashMap; +use acvm::acir::circuit::ErrorSelector; + use crate::ssa::{ function_builder::FunctionBuilder, - ir::{basic_block::BasicBlockId, function::FunctionId, value::ValueId}, + ir::{ + basic_block::BasicBlockId, function::FunctionId, instruction::ConstrainError, + value::ValueId, + }, }; use super::{ - Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedSsa, ParsedTerminator, - ParsedValue, RuntimeType, Ssa, SsaError, + ast::AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedSsa, + ParsedTerminator, ParsedValue, RuntimeType, Ssa, SsaError, }; impl ParsedSsa { @@ -31,6 +36,8 @@ struct Translator { /// passes already which replaced some of the original IDs. The translator /// will recreate the SSA step by step, which can result in a new ID layout. variables: HashMap>, + + error_selector_counter: u64, } impl Translator { @@ -64,8 +71,13 @@ impl Translator { functions.insert(function.internal_name.clone(), function_id); } - let mut translator = - Self { builder, functions, variables: HashMap::new(), blocks: HashMap::new() }; + let mut translator = Self { + builder, + functions, + variables: HashMap::new(), + blocks: HashMap::new(), + error_selector_counter: 0, + }; translator.translate_function_body(main_function)?; Ok(translator) @@ -198,10 +210,25 @@ impl Translator { let value_id = self.builder.insert_cast(lhs, typ); self.define_variable(target, value_id)?; } - ParsedInstruction::Constrain { lhs, rhs } => { + ParsedInstruction::Constrain { lhs, rhs, assert_message } => { let lhs = self.translate_value(lhs)?; let rhs = self.translate_value(rhs)?; - self.builder.insert_constrain(lhs, rhs, None); + let assert_message = match assert_message { + Some(AssertMessage::Static(string)) => { + Some(ConstrainError::StaticString(string)) + } + Some(AssertMessage::Dynamic(values)) => { + let error_selector = ErrorSelector::new(self.error_selector_counter); + self.error_selector_counter += 1; + + let is_string_type = false; + let values = self.translate_values(values)?; + + Some(ConstrainError::Dynamic(error_selector, is_string_type, values)) + } + None => None, + }; + self.builder.insert_constrain(lhs, rhs, assert_message); } ParsedInstruction::DecrementRc { value } => { let value = self.translate_value(value)?; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs index 4c90475be74..d89bc1e9e28 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/lexer.rs @@ -61,6 +61,7 @@ impl<'a> Lexer<'a> { Some('&') => self.single_char_token(Token::Ampersand), Some('-') if self.peek_char() == Some('>') => self.double_char_token(Token::Arrow), Some('-') => self.single_char_token(Token::Dash), + Some('"') => self.eat_string_literal(), Some(ch) if ch.is_ascii_alphanumeric() || ch == '_' => self.eat_alpha_numeric(ch), Some(char) => Err(LexerError::UnexpectedCharacter { char, @@ -177,6 +178,41 @@ impl<'a> Lexer<'a> { Ok(integer_token.into_span(start, end)) } + fn eat_string_literal(&mut self) -> SpannedTokenResult { + let start = self.position; + let mut string = String::new(); + + while let Some(next) = self.next_char() { + let char = match next { + '"' => break, + '\\' => match self.next_char() { + Some('r') => '\r', + Some('n') => '\n', + Some('t') => '\t', + Some('0') => '\0', + Some('"') => '"', + Some('\\') => '\\', + Some(escaped) => { + let span = Span::inclusive(start, self.position); + return Err(LexerError::InvalidEscape { escaped, span }); + } + None => { + let span = Span::inclusive(start, self.position); + return Err(LexerError::UnterminatedStringLiteral { span }); + } + }, + other => other, + }; + + string.push(char); + } + + let str_literal_token = Token::Str(string); + + let end = self.position; + Ok(str_literal_token.into_span(start, end)) + } + fn eat_while bool>( &mut self, initial_char: Option, @@ -247,6 +283,12 @@ pub(crate) enum LexerError { InvalidIntegerLiteral { span: Span, found: String }, #[error("Integer literal too large")] IntegerLiteralTooLarge { span: Span, limit: String }, + #[error("Unterminated string literal")] + UnterminatedStringLiteral { span: Span }, + #[error( + "'\\{escaped}' is not a valid escape sequence. Use '\\' for a literal backslash character." + )] + InvalidEscape { escaped: char, span: Span }, } impl LexerError { @@ -254,7 +296,9 @@ impl LexerError { match self { LexerError::UnexpectedCharacter { span, .. } | LexerError::InvalidIntegerLiteral { span, .. } - | LexerError::IntegerLiteralTooLarge { span, .. } => *span, + | LexerError::IntegerLiteralTooLarge { span, .. } + | LexerError::UnterminatedStringLiteral { span } + | LexerError::InvalidEscape { span, .. } => *span, } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/mod.rs index 2db2c636a8f..3d8bd37dead 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/mod.rs @@ -10,8 +10,8 @@ use super::{ use acvm::{AcirField, FieldElement}; use ast::{ - Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedParameter, ParsedSsa, - ParsedValue, + AssertMessage, Identifier, ParsedBlock, ParsedFunction, ParsedInstruction, ParsedParameter, + ParsedSsa, ParsedValue, }; use lexer::{Lexer, LexerError}; use noirc_errors::Span; @@ -28,6 +28,11 @@ mod tests; 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 { let mut parser = Parser::new(src).map_err(|err| SsaErrorWithSource::parse_error(err, src))?; @@ -308,7 +313,20 @@ impl<'a> Parser<'a> { let lhs = self.parse_value_or_error()?; self.eat_or_error(Token::Equal)?; let rhs = self.parse_value_or_error()?; - Ok(Some(ParsedInstruction::Constrain { lhs, rhs })) + + let assert_message = if self.eat(Token::Comma)? { + if let Some(str) = self.eat_str()? { + Some(AssertMessage::Static(str)) + } else if self.eat_keyword(Keyword::Data)? { + Some(AssertMessage::Dynamic(self.parse_comma_separated_values()?)) + } else { + return self.expected_string_or_data(); + } + } else { + None + }; + + Ok(Some(ParsedInstruction::Constrain { lhs, rhs, assert_message })) } fn parse_decrement_rc(&mut self) -> ParseResult> { @@ -649,6 +667,10 @@ impl<'a> Parser<'a> { return Ok(Type::Reference(Arc::new(typ))); } + if self.eat_keyword(Keyword::Function)? { + return Ok(Type::Function); + } + self.expected_type() } @@ -762,6 +784,18 @@ impl<'a> Parser<'a> { } } + fn eat_str(&mut self) -> ParseResult> { + if matches!(self.token.token(), Token::Str(..)) { + let token = self.bump()?; + match token.into_token() { + Token::Str(string) => Ok(Some(string)), + _ => unreachable!(), + } + } else { + Ok(None) + } + } + fn eat(&mut self, token: Token) -> ParseResult { if self.token.token() == &token { self.bump()?; @@ -807,6 +841,13 @@ impl<'a> Parser<'a> { }) } + fn expected_string_or_data(&mut self) -> ParseResult { + Err(ParserError::ExpectedStringOrData { + found: self.token.token().clone(), + span: self.token.to_span(), + }) + } + fn expected_identifier(&mut self) -> ParseResult { Err(ParserError::ExpectedIdentifier { found: self.token.token().clone(), @@ -868,6 +909,8 @@ pub(crate) enum ParserError { ExpectedType { found: Token, span: Span }, #[error("Expected an instruction or terminator, found '{found}'")] ExpectedInstructionOrTerminator { found: Token, span: Span }, + #[error("Expected a string literal or 'data', found '{found}'")] + ExpectedStringOrData { found: Token, span: Span }, #[error("Expected a value, found '{found}'")] ExpectedValue { found: Token, span: Span }, #[error("Multiple return values only allowed for call")] @@ -884,6 +927,7 @@ impl ParserError { | ParserError::ExpectedInt { span, .. } | ParserError::ExpectedType { span, .. } | ParserError::ExpectedInstructionOrTerminator { span, .. } + | ParserError::ExpectedStringOrData { span, .. } | ParserError::ExpectedValue { span, .. } => *span, ParserError::MultipleReturnValuesOnlyAllowedForCall { second_target, .. } => { second_target.span diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 60d398bf9d5..593b66d0c98 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -214,6 +214,31 @@ fn test_constrain() { assert_ssa_roundtrip(src); } +#[test] +fn test_constrain_with_static_message() { + let src = r#" + acir(inline) fn main f0 { + b0(v0: Field): + constrain v0 == Field 1, "Oh no!" + return + } + "#; + assert_ssa_roundtrip(src); +} + +#[test] +fn test_constrain_with_dynamic_message() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v7 = make_array [u8 123, u8 120, u8 125, u8 32, u8 123, u8 121, u8 125] : [u8; 7] + constrain v0 == Field 1, data v7, u32 2, v0, v1 + return + } + "; + assert_ssa_roundtrip(src); +} + #[test] fn test_enable_side_effects() { let src = " @@ -441,3 +466,15 @@ fn test_negative() { "; assert_ssa_roundtrip(src); } + +#[test] +fn test_function_type() { + let src = " + acir(inline) fn main f0 { + b0(): + v0 = allocate -> &mut function + return + } + "; + assert_ssa_roundtrip(src); +} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs index f663879e899..d8dd4ec011e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/parser/token.rs @@ -29,6 +29,7 @@ impl SpannedToken { pub(crate) enum Token { Ident(String), Int(FieldElement), + Str(String), Keyword(Keyword), IntType(IntType), /// = @@ -77,6 +78,7 @@ impl Display for Token { match self { Token::Ident(ident) => write!(f, "{}", ident), Token::Int(int) => write!(f, "{}", int), + Token::Str(string) => write!(f, "{string:?}"), Token::Keyword(keyword) => write!(f, "{}", keyword), Token::IntType(int_type) => write!(f, "{}", int_type), Token::Assign => write!(f, "="), @@ -120,6 +122,7 @@ pub(crate) enum Keyword { Call, Cast, Constrain, + Data, DecRc, Div, Inline, @@ -130,6 +133,7 @@ pub(crate) enum Keyword { Field, Fold, Fn, + Function, IncRc, Index, Jmp, @@ -175,6 +179,7 @@ impl Keyword { "call" => Keyword::Call, "cast" => Keyword::Cast, "constrain" => Keyword::Constrain, + "data" => Keyword::Data, "dec_rc" => Keyword::DecRc, "div" => Keyword::Div, "else" => Keyword::Else, @@ -185,6 +190,7 @@ impl Keyword { "Field" => Keyword::Field, "fold" => Keyword::Fold, "fn" => Keyword::Fn, + "function" => Keyword::Function, "inc_rc" => Keyword::IncRc, "index" => Keyword::Index, "jmp" => Keyword::Jmp, @@ -234,6 +240,7 @@ impl Display for Keyword { Keyword::Call => write!(f, "call"), Keyword::Cast => write!(f, "cast"), Keyword::Constrain => write!(f, "constrain"), + Keyword::Data => write!(f, "data"), Keyword::DecRc => write!(f, "dec_rc"), Keyword::Div => write!(f, "div"), Keyword::Else => write!(f, "else"), @@ -242,6 +249,7 @@ impl Display for Keyword { Keyword::Field => write!(f, "Field"), Keyword::Fold => write!(f, "fold"), Keyword::Fn => write!(f, "fn"), + Keyword::Function => write!(f, "function"), Keyword::IncRc => write!(f, "inc_rc"), Keyword::Index => write!(f, "index"), Keyword::Inline => write!(f, "inline"), diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 0c6041029da..ddc3365b551 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -172,6 +172,7 @@ impl<'a> FunctionContext<'a> { /// Always returns a Value::Mutable wrapping the allocate instruction. pub(super) fn new_mutable_variable(&mut self, value_to_store: ValueId) -> Value { let element_type = self.builder.current_function.dfg.type_of_value(value_to_store); + self.builder.increment_array_reference_count(value_to_store); let alloc = self.builder.insert_allocate(element_type); self.builder.insert_store(alloc, value_to_store); let typ = self.builder.type_of_value(value_to_store); @@ -735,7 +736,6 @@ impl<'a> FunctionContext<'a> { // Reference counting in brillig relies on us incrementing reference // counts when arrays/slices are constructed or indexed. // Thus, if we dereference an lvalue which happens to be array/slice we should increment its reference counter. - self.builder.increment_array_reference_count(reference); self.builder.insert_load(reference, element_type).into() }) } @@ -916,7 +916,10 @@ impl<'a> FunctionContext<'a> { let parameters = self.builder.current_function.dfg.block_parameters(entry).to_vec(); for parameter in parameters { - self.builder.increment_array_reference_count(parameter); + // Avoid reference counts for immutable arrays that aren't behind references. + if self.builder.current_function.dfg.value_is_reference(parameter) { + self.builder.increment_array_reference_count(parameter); + } } entry @@ -933,7 +936,9 @@ impl<'a> FunctionContext<'a> { dropped_parameters.retain(|parameter| !terminator_args.contains(parameter)); for parameter in dropped_parameters { - self.builder.decrement_array_reference_count(parameter); + if self.builder.current_function.dfg.value_is_reference(parameter) { + self.builder.decrement_array_reference_count(parameter); + } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index c50f0a7f45c..d28236bd360 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -665,12 +665,11 @@ impl<'a> FunctionContext<'a> { values = values.map(|value| { let value = value.eval(self); - // Make sure to increment array reference counts on each let binding - self.builder.increment_array_reference_count(value); - Tree::Leaf(if let_expr.mutable { self.new_mutable_variable(value) } else { + // `new_mutable_variable` already increments rcs internally + self.builder.increment_array_reference_count(value); value::Value::Normal(value) }) }); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs index 3c6664dd569..35e57cd4528 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs @@ -200,6 +200,14 @@ impl GenericTypeArgs { pub fn is_empty(&self) -> bool { self.ordered_args.is_empty() && self.named_args.is_empty() } + + fn contains_unspecified(&self) -> bool { + let ordered_args_contains_unspecified = + self.ordered_args.iter().any(|ordered_arg| ordered_arg.contains_unspecified()); + let named_args_contains_unspecified = + self.named_args.iter().any(|(_name, named_arg)| named_arg.contains_unspecified()); + ordered_args_contains_unspecified || named_args_contains_unspecified + } } impl From> for GenericTypeArgs { @@ -375,6 +383,10 @@ impl UnresolvedType { let typ = UnresolvedTypeData::Named(path, generic_type_args, true); UnresolvedType { typ, span } } + + pub(crate) fn contains_unspecified(&self) -> bool { + self.typ.contains_unspecified() + } } impl UnresolvedTypeData { @@ -395,6 +407,47 @@ impl UnresolvedTypeData { pub fn with_span(&self, span: Span) -> UnresolvedType { UnresolvedType { typ: self.clone(), span } } + + fn contains_unspecified(&self) -> bool { + match self { + UnresolvedTypeData::Array(typ, length) => { + typ.contains_unspecified() || length.contains_unspecified() + } + UnresolvedTypeData::Slice(typ) => typ.contains_unspecified(), + UnresolvedTypeData::Expression(expr) => expr.contains_unspecified(), + UnresolvedTypeData::String(length) => length.contains_unspecified(), + UnresolvedTypeData::FormatString(typ, length) => { + typ.contains_unspecified() || length.contains_unspecified() + } + UnresolvedTypeData::Parenthesized(typ) => typ.contains_unspecified(), + UnresolvedTypeData::Named(path, args, _is_synthesized) => { + // '_' is unspecified + let path_is_wildcard = path.is_wildcard(); + let an_arg_is_unresolved = args.contains_unspecified(); + path_is_wildcard || an_arg_is_unresolved + } + UnresolvedTypeData::TraitAsType(_path, args) => args.contains_unspecified(), + UnresolvedTypeData::MutableReference(typ) => typ.contains_unspecified(), + UnresolvedTypeData::Tuple(args) => args.iter().any(|arg| arg.contains_unspecified()), + UnresolvedTypeData::Function(args, ret, env, _unconstrained) => { + let args_contains_unspecified = args.iter().any(|arg| arg.contains_unspecified()); + args_contains_unspecified + || ret.contains_unspecified() + || env.contains_unspecified() + } + UnresolvedTypeData::Unspecified => true, + + UnresolvedTypeData::FieldElement + | UnresolvedTypeData::Integer(_, _) + | UnresolvedTypeData::Bool + | UnresolvedTypeData::Unit + | UnresolvedTypeData::Quoted(_) + | UnresolvedTypeData::AsTraitPath(_) + | UnresolvedTypeData::Resolved(_) + | UnresolvedTypeData::Interned(_) + | UnresolvedTypeData::Error => false, + } + } } #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash, PartialOrd, Ord)] @@ -494,6 +547,19 @@ impl UnresolvedTypeExpression { | BinaryOpKind::Modulo ) } + + fn contains_unspecified(&self) -> bool { + match self { + // '_' is unspecified + UnresolvedTypeExpression::Variable(path) => path.is_wildcard(), + UnresolvedTypeExpression::BinaryOperation(lhs, _op, rhs, _span) => { + lhs.contains_unspecified() || rhs.contains_unspecified() + } + UnresolvedTypeExpression::Constant(_, _) | UnresolvedTypeExpression::AsTraitPath(_) => { + false + } + } + } } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 7244be371af..c77fe7513a1 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -27,6 +27,9 @@ use crate::token::{SecondaryAttribute, Token}; /// for an identifier that already failed to parse. pub const ERROR_IDENT: &str = "$error"; +/// This is used to represent an UnresolvedTypeData::Unspecified in a Path +pub const WILDCARD_TYPE: &str = "_"; + #[derive(Debug, PartialEq, Eq, Clone)] pub struct Statement { pub kind: StatementKind, @@ -483,6 +486,10 @@ impl Path { self.segments.first().cloned().map(|segment| segment.ident) } + pub(crate) fn is_wildcard(&self) -> bool { + self.to_ident().map(|ident| ident.0.contents) == Some(WILDCARD_TYPE.to_string()) + } + pub fn is_empty(&self) -> bool { self.segments.is_empty() && self.kind == PathKind::Plain } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index ff482dca4fb..f801c1817ef 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -907,7 +907,17 @@ impl<'context> Elaborator<'context> { let location = Location::new(span, self.file); match value.into_expression(self.interner, location) { - Ok(new_expr) => self.elaborate_expression(new_expr), + Ok(new_expr) => { + // At this point the Expression was already elaborated and we got a Value. + // We'll elaborate this value turned into Expression to inline it and get + // an ExprId and Type, but we don't want any visibility errors to happen + // here (they could if we have `Foo { inner: 5 }` and `inner` is not + // accessible from where this expression is being elaborated). + self.silence_field_visibility_errors += 1; + let value = self.elaborate_expression(new_expr); + self.silence_field_visibility_errors -= 1; + value + } Err(error) => make_error(self, error), } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index 084bcbe3f8d..20d27fbc9ac 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -164,6 +164,12 @@ pub struct Elaborator<'context> { unresolved_globals: BTreeMap, pub(crate) interpreter_call_stack: im::Vector, + + /// If greater than 0, field visibility errors won't be reported. + /// This is used when elaborating a comptime expression that is a struct constructor + /// like `Foo { inner: 5 }`: in that case we already elaborated the code that led to + /// that comptime value and any visibility errors were already reported. + silence_field_visibility_errors: usize, } #[derive(Default)] @@ -213,6 +219,7 @@ impl<'context> Elaborator<'context> { current_trait: None, interpreter_call_stack, in_comptime_context: false, + silence_field_visibility_errors: 0, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs index 757def16a93..6ed8fee753c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs @@ -76,8 +76,17 @@ impl<'context> Elaborator<'context> { ) -> (HirStatement, Type) { let expr_span = let_stmt.expression.span; let (expression, expr_type) = self.elaborate_expression(let_stmt.expression); + let type_contains_unspecified = let_stmt.r#type.contains_unspecified(); let annotated_type = self.resolve_inferred_type(let_stmt.r#type); + // Require the top-level of a global's type to be fully-specified + if type_contains_unspecified && global_id.is_some() { + let span = expr_span; + let expected_type = annotated_type.clone(); + let error = ResolverError::UnspecifiedGlobalType { span, expected_type }; + self.push_err(error); + } + let definition = match global_id { None => DefinitionKind::Local(Some(expression)), Some(id) => DefinitionKind::Global(id), @@ -509,6 +518,10 @@ impl<'context> Elaborator<'context> { visibility: ItemVisibility, span: Span, ) { + if self.silence_field_visibility_errors > 0 { + return; + } + if !struct_member_is_visible(struct_type.id, visibility, self.module_id(), self.def_maps) { self.push_err(ResolverError::PathResolutionError(PathResolutionError::Private( Ident::new(field_name.to_string(), span), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index ae2bb942f48..7e06964b563 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -9,7 +9,7 @@ use crate::{ ast::{ AsTraitPath, BinaryOpKind, GenericTypeArgs, Ident, IntegerBitSize, Path, PathKind, Signedness, UnaryOp, UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, - UnresolvedTypeData, UnresolvedTypeExpression, + UnresolvedTypeData, UnresolvedTypeExpression, WILDCARD_TYPE, }, hir::{ comptime::{Interpreter, Value}, @@ -40,7 +40,6 @@ use crate::{ use super::{lints, path_resolution::PathResolutionItem, Elaborator}; pub const SELF_TYPE_NAME: &str = "Self"; -pub const WILDCARD_TYPE: &str = "_"; pub(super) struct TraitPathResolution { pub(super) method: TraitMethod, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 80c1ee217c2..e8b34ab4323 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -60,6 +60,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "apply_range_constraint" => foreign::apply_range_constraint(arguments, location), "array_as_str_unchecked" => array_as_str_unchecked(interner, arguments, location), "array_len" => array_len(interner, arguments, location), + "array_refcount" => Ok(Value::U32(0)), "assert_constant" => Ok(Value::Bool(true)), "as_slice" => as_slice(interner, arguments, location), "ctstring_eq" => ctstring_eq(arguments, location), @@ -167,6 +168,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "slice_pop_front" => slice_pop_front(interner, arguments, location, call_stack), "slice_push_back" => slice_push_back(interner, arguments, location), "slice_push_front" => slice_push_front(interner, arguments, location), + "slice_refcount" => Ok(Value::U32(0)), "slice_remove" => slice_remove(interner, arguments, location, call_stack), "str_as_bytes" => str_as_bytes(interner, arguments, location), "str_as_ctstring" => str_as_ctstring(interner, arguments, location), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index b82eafa5b9d..80bd5247ee6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -101,6 +101,8 @@ pub enum ResolverError { JumpOutsideLoop { is_break: bool, span: Span }, #[error("Only `comptime` globals can be mutable")] MutableGlobal { span: Span }, + #[error("Globals must have a specified type")] + UnspecifiedGlobalType { span: Span, expected_type: Type }, #[error("Self-referential structs are not supported")] SelfReferentialStruct { span: Span }, #[error("#[no_predicates] attribute is only allowed on constrained functions")] @@ -431,6 +433,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::UnspecifiedGlobalType { span, expected_type } => { + Diagnostic::simple_error( + "Globals must have a specified type".to_string(), + format!("Inferred type is `{expected_type}`"), + *span, + ) + }, ResolverError::SelfReferentialStruct { span } => { Diagnostic::simple_error( "Self-referential structs are not supported".into(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 20a5bac49f6..cb45bf37c83 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -1300,11 +1300,17 @@ fn lambda$f1(mut env$l1: (Field)) -> Field { #[test] fn deny_cyclic_globals() { let src = r#" - global A = B; - global B = A; + global A: u32 = B; + global B: u32 = A; fn main() {} "#; - assert_eq!(get_program_errors(src).len(), 1); + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + assert!(matches!( + errors[0].0, + CompilationError::ResolverError(ResolverError::DependencyCycle { .. }) + )); } #[test] @@ -3210,10 +3216,10 @@ fn as_trait_path_syntax_no_impl() { } #[test] -fn infer_globals_to_u32_from_type_use() { +fn dont_infer_globals_to_u32_from_type_use() { let src = r#" global ARRAY_LEN = 3; - global STR_LEN = 2; + global STR_LEN: _ = 2; global FMT_STR_LEN = 2; fn main() { @@ -3223,6 +3229,59 @@ fn infer_globals_to_u32_from_type_use() { } "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 3); + assert!(matches!( + errors[0].0, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); + assert!(matches!( + errors[1].0, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); + assert!(matches!( + errors[2].0, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); +} + +#[test] +fn dont_infer_partial_global_types() { + let src = r#" + pub global ARRAY: [Field; _] = [0; 3]; + pub global NESTED_ARRAY: [[Field; _]; 3] = [[]; 3]; + pub global STR: str<_> = "hi"; + pub global NESTED_STR: [str<_>] = &["hi"]; + pub global FMT_STR: fmtstr<_, _> = f"hi {ARRAY}"; + pub global TUPLE_WITH_MULTIPLE: ([str<_>], [[Field; _]; 3]) = (&["hi"], [[]; 3]); + + fn main() { } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 6); + for (error, _file_id) in errors { + assert!(matches!( + error, + CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. }) + )); + } +} + +#[test] +fn u32_globals_as_sizes_in_types() { + let src = r#" + global ARRAY_LEN: u32 = 3; + global STR_LEN: u32 = 2; + global FMT_STR_LEN: u32 = 2; + + fn main() { + let _a: [u32; ARRAY_LEN] = [1, 2, 3]; + let _b: str = "hi"; + let _c: fmtstr = f"hi"; + } + "#; + let errors = get_program_errors(src); assert_eq!(errors.len(), 0); } @@ -3686,7 +3745,7 @@ fn allows_struct_with_generic_infix_type_as_main_input_3() { x: [u64; N * 2], } - global N = 9; + global N: u32 = 9; fn main(_x: Foo) {} "#; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs index 5f9fc887b27..9304209e501 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/unused_items.rs @@ -191,8 +191,8 @@ fn errors_on_unused_type_alias() { #[test] fn warns_on_unused_global() { let src = r#" - global foo = 1; - global bar = 1; + global foo: u32 = 1; + global bar: Field = 1; fn main() { let _ = bar; @@ -216,7 +216,7 @@ fn does_not_warn_on_unused_global_if_it_has_an_abi_attribute() { let src = r#" contract foo { #[abi(notes)] - global bar = 1; + global bar: u64 = 1; } fn main() {} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs index 7cfec32062d..824a1de4c37 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs @@ -493,3 +493,108 @@ fn does_not_error_if_referring_to_top_level_private_module_via_crate() { "#; assert_no_errors(src); } + +#[test] +fn visibility_bug_inside_comptime() { + let src = r#" + mod foo { + pub struct Foo { + inner: Field, + } + + impl Foo { + pub fn new(inner: Field) -> Self { + Self { inner } + } + } + } + + use foo::Foo; + + fn main() { + let _ = Foo::new(5); + let _ = comptime { Foo::new(5) }; + } + "#; + assert_no_errors(src); +} + +#[test] +fn errors_if_accessing_private_struct_member_inside_comptime_context() { + let src = r#" + mod foo { + pub struct Foo { + inner: Field, + } + + impl Foo { + pub fn new(inner: Field) -> Self { + Self { inner } + } + } + } + + use foo::Foo; + + fn main() { + comptime { + let foo = Foo::new(5); + let _ = foo.inner; + }; + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::Private(ident), + )) = &errors[0].0 + else { + panic!("Expected a private error"); + }; + + assert_eq!(ident.to_string(), "inner"); +} + +#[test] +fn errors_if_accessing_private_struct_member_inside_function_generated_at_comptime() { + let src = r#" + mod foo { + pub struct Foo { + foo_inner: Field, + } + } + + use foo::Foo; + + #[generate_inner_accessor] + struct Bar { + bar_inner: Foo, + } + + comptime fn generate_inner_accessor(_s: StructDefinition) -> Quoted { + quote { + fn bar_get_foo_inner(x: Bar) -> Field { + x.bar_inner.foo_inner + } + } + } + + fn main(x: Bar) { + let _ = bar_get_foo_inner(x); + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::Private(ident), + )) = &errors[0].0 + else { + panic!("Expected a private error"); + }; + + assert_eq!(ident.to_string(), "foo_inner"); +} diff --git a/noir/noir-repo/cspell.json b/noir/noir-repo/cspell.json index a386ed80ee9..36bba737cd7 100644 --- a/noir/noir-repo/cspell.json +++ b/noir/noir-repo/cspell.json @@ -171,6 +171,7 @@ "PLONKish", "pprof", "precomputes", + "preheader", "preimage", "preprocess", "prettytable", @@ -182,6 +183,7 @@ "quantile", "quasiquote", "rangemap", + "refcount", "repr", "reqwest", "rfind", diff --git a/noir/noir-repo/docs/docs/how_to/how-to-oracles.md b/noir/noir-repo/docs/docs/how_to/how-to-oracles.md index 4763b7788d6..0bb8743e361 100644 --- a/noir/noir-repo/docs/docs/how_to/how-to-oracles.md +++ b/noir/noir-repo/docs/docs/how_to/how-to-oracles.md @@ -30,7 +30,7 @@ This guide has 3 major steps: An oracle is defined in a Noir program by defining two methods: -- An unconstrained method - This tells the compiler that it is executing an [unconstrained functions](../noir/concepts//unconstrained.md). +- An unconstrained method - This tells the compiler that it is executing an [unconstrained function](../noir/concepts//unconstrained.md). - A decorated oracle method - This tells the compiler that this method is an RPC call. An example of an oracle that returns a `Field` would be: diff --git a/noir/noir-repo/docs/docs/noir/concepts/globals.md b/noir/noir-repo/docs/docs/noir/concepts/globals.md index 6b8314399a2..c64b6c53746 100644 --- a/noir/noir-repo/docs/docs/noir/concepts/globals.md +++ b/noir/noir-repo/docs/docs/noir/concepts/globals.md @@ -10,12 +10,12 @@ sidebar_position: 8 ## Globals -Noir supports global variables. The global's type can be inferred by the compiler entirely: +Noir supports global variables. The global's type must be specified by the user: ```rust -global N = 5; // Same as `global N: Field = 5` +global N: Field = 5; -global TUPLE = (3, 2); +global TUPLE: (Field, Field) = (3, 2); fn main() { assert(N == 5); @@ -28,7 +28,7 @@ fn main() { Globals can be defined as any expression, so long as they don't depend on themselves - otherwise there would be a dependency cycle! For example: ```rust -global T = foo(T); // dependency error +global T: u32 = foo(T); // dependency error ``` ::: @@ -47,7 +47,7 @@ fn main(y : [Field; N]) { A global from another module can be imported or referenced externally like any other name: ```rust -global N = 20; +global N: Field = 20; fn main() { assert(my_submodule::N != N); @@ -62,7 +62,7 @@ When a global is used, Noir replaces the name with its definition on each occurr This means globals defined using function calls will repeat the call each time they're used: ```rust -global RESULT = foo(); +global RESULT: [Field; 100] = foo(); fn foo() -> [Field; 100] { ... } ``` @@ -78,5 +78,5 @@ to make the global public or `pub(crate)` to make it public to just its crate: ```rust // This global is now public -pub global N = 5; -``` \ No newline at end of file +pub global N: u32 = 5; +``` diff --git a/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/ec_primitives.md b/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/ec_primitives.md index f262d8160d6..00b8071487e 100644 --- a/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/ec_primitives.md +++ b/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/ec_primitives.md @@ -97,6 +97,5 @@ fn bjj_pub_key(priv_key: Field) -> Point This would come in handy in a Merkle proof. - EdDSA signature verification: This is a matter of combining these primitives with a suitable hash - function. See - [feat(stdlib): EdDSA sig verification noir#1136](https://github.com/noir-lang/noir/pull/1136) for - the case of Baby Jubjub and the Poseidon hash function. + function. See the [eddsa](https://github.com/noir-lang/eddsa) library an example of eddsa signature verification + over the Baby Jubjub curve. diff --git a/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx b/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx deleted file mode 100644 index b283de693c8..00000000000 --- a/noir/noir-repo/docs/docs/noir/standard_library/cryptographic_primitives/eddsa.mdx +++ /dev/null @@ -1,37 +0,0 @@ ---- -title: EdDSA Verification -description: Learn about the cryptographic primitives regarding EdDSA -keywords: [cryptographic primitives, Noir project, eddsa, signatures] -sidebar_position: 5 ---- - -import BlackBoxInfo from '@site/src/components/Notes/_blackbox'; - -## eddsa::eddsa_poseidon_verify - -Verifier for EdDSA signatures - -```rust -fn eddsa_poseidon_verify(public_key_x : Field, public_key_y : Field, signature_s: Field, signature_r8_x: Field, signature_r8_y: Field, message: Field) -> bool -``` - -It is also possible to specify the hash algorithm used for the signature by using the `eddsa_verify` function by passing a type implementing the Hasher trait with the turbofish operator. -For instance, if you want to use Poseidon2 instead, you can do the following: -```rust -use std::hash::poseidon2::Poseidon2Hasher; - -eddsa_verify::(pub_key_a.x, pub_key_a.y, s_a, r8_a.x, r8_a.y, msg); -``` - - - -## eddsa::eddsa_to_pub - -Private to public key conversion. - -Returns `(pub_key_x, pub_key_y)` - -```rust -fn eddsa_to_pub(secret : Field) -> (Field, Field) -``` - diff --git a/noir/noir-repo/docs/docs/noir/standard_library/mem.md b/noir/noir-repo/docs/docs/noir/standard_library/mem.md index 95d36ac2a72..3619550273e 100644 --- a/noir/noir-repo/docs/docs/noir/standard_library/mem.md +++ b/noir/noir-repo/docs/docs/noir/standard_library/mem.md @@ -50,3 +50,33 @@ by checking this equality once `N`, `A`, `B` are fully resolved. Note that since this safety check is performed after type checking rather than during, no error is issued if the function containing `checked_transmute` is never called. + +# `std::mem::array_refcount` + +```rust +fn array_refcount(array: [T; N]) -> u32 {} +``` + +Returns the internal reference count of an array value in unconstrained code. + +Arrays only have reference count in unconstrained code - using this anywhere +else will return zero. + +This function is mostly intended for debugging compiler optimizations but can also be used +to find where array copies may be happening in unconstrained code by placing it before array +mutations. + +# `std::mem::slice_refcount` + +```rust +fn slice_refcount(slice: [T]) -> u32 {} +``` + +Returns the internal reference count of a slice value in unconstrained code. + +Slices only have reference count in unconstrained code - using this anywhere +else will return zero. + +This function is mostly intended for debugging compiler optimizations but can also be used +to find where slice copies may be happening in unconstrained code by placing it before slice +mutations. diff --git a/noir/noir-repo/noir_stdlib/src/bigint.nr b/noir/noir-repo/noir_stdlib/src/bigint.nr index be072257be3..c94a7a75f25 100644 --- a/noir/noir-repo/noir_stdlib/src/bigint.nr +++ b/noir/noir-repo/noir_stdlib/src/bigint.nr @@ -1,27 +1,27 @@ use crate::cmp::Eq; use crate::ops::{Add, Div, Mul, Sub}; -global bn254_fq = &[ +global bn254_fq: [u8] = &[ 0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97, 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, 0x64, 0x30, ]; -global bn254_fr = &[ +global bn254_fr: [u8] = &[ 1, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48, ]; -global secpk1_fr = &[ +global secpk1_fr: [u8] = &[ 0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, 0xBA, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, ]; -global secpk1_fq = &[ +global secpk1_fq: [u8] = &[ 0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, ]; -global secpr1_fq = &[ +global secpr1_fq: [u8] = &[ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, ]; -global secpr1_fr = &[ +global secpr1_fr: [u8] = &[ 81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255, ]; diff --git a/noir/noir-repo/noir_stdlib/src/collections/map.nr b/noir/noir-repo/noir_stdlib/src/collections/map.nr index b46bfa837fb..bcce08faab4 100644 --- a/noir/noir-repo/noir_stdlib/src/collections/map.nr +++ b/noir/noir-repo/noir_stdlib/src/collections/map.nr @@ -7,8 +7,8 @@ use crate::option::Option; // We use load factor alpha_max = 0.75. // Upon exceeding it, assert will fail in order to inform the user // about performance degradation, so that he can adjust the capacity. -global MAX_LOAD_FACTOR_NUMERATOR = 3; -global MAX_LOAD_FACTOR_DEN0MINATOR = 4; +global MAX_LOAD_FACTOR_NUMERATOR: u32 = 3; +global MAX_LOAD_FACTOR_DEN0MINATOR: u32 = 4; /// `HashMap` is used to efficiently store and look up key-value pairs. /// diff --git a/noir/noir-repo/noir_stdlib/src/ec/mod.nr b/noir/noir-repo/noir_stdlib/src/ec/mod.nr index b62bc99d9c8..e2b2df1b794 100644 --- a/noir/noir-repo/noir_stdlib/src/ec/mod.nr +++ b/noir/noir-repo/noir_stdlib/src/ec/mod.nr @@ -122,12 +122,12 @@ pub mod consts; // Commonly used curve presets // Field-dependent constant ZETA = a non-square element of Field // Required for Elligator 2 map // TODO: Replace with built-in constant. -global ZETA = 5; +global ZETA: Field = 5; // Field-dependent constants for Tonelli-Shanks algorithm (see sqrt function below) // TODO: Possibly make this built-in. -global C1 = 28; -global C3 = 40770029410420498293352137776570907027550720424234931066070132305055; -global C5 = 19103219067921713944291392827692070036145651957329286315305642004821462161904; +global C1: u32 = 28; +global C3: Field = 40770029410420498293352137776570907027550720424234931066070132305055; +global C5: Field = 19103219067921713944291392827692070036145651957329286315305642004821462161904; // Higher-order version of scalar multiplication // TODO: Make this work so that the submodules' bit_mul may be defined in terms of it. //fn bit_mul(add: fn(T,T) -> T, e: T, bits: [u1; N], p: T) -> T { diff --git a/noir/noir-repo/noir_stdlib/src/eddsa.nr b/noir/noir-repo/noir_stdlib/src/eddsa.nr deleted file mode 100644 index c049b7abbb5..00000000000 --- a/noir/noir-repo/noir_stdlib/src/eddsa.nr +++ /dev/null @@ -1,76 +0,0 @@ -use crate::default::Default; -use crate::ec::consts::te::baby_jubjub; -use crate::ec::tecurve::affine::Point as TEPoint; -use crate::hash::Hasher; -use crate::hash::poseidon::PoseidonHasher; - -// Returns true if signature is valid -pub fn eddsa_poseidon_verify( - pub_key_x: Field, - pub_key_y: Field, - signature_s: Field, - signature_r8_x: Field, - signature_r8_y: Field, - message: Field, -) -> bool { - eddsa_verify::( - pub_key_x, - pub_key_y, - signature_s, - signature_r8_x, - signature_r8_y, - message, - ) -} - -pub fn eddsa_verify( - pub_key_x: Field, - pub_key_y: Field, - signature_s: Field, - signature_r8_x: Field, - signature_r8_y: Field, - message: Field, -) -> bool -where - H: Hasher + Default, -{ - // Verifies by testing: - // S * B8 = R8 + H(R8, A, m) * A8 - let bjj = baby_jubjub(); - - let pub_key = TEPoint::new(pub_key_x, pub_key_y); - assert(bjj.curve.contains(pub_key)); - - let signature_r8 = TEPoint::new(signature_r8_x, signature_r8_y); - assert(bjj.curve.contains(signature_r8)); - // Ensure S < Subgroup Order - assert(signature_s.lt(bjj.suborder)); - // Calculate the h = H(R, A, msg) - let mut hasher = H::default(); - hasher.write(signature_r8_x); - hasher.write(signature_r8_y); - hasher.write(pub_key_x); - hasher.write(pub_key_y); - hasher.write(message); - let hash: Field = hasher.finish(); - // Calculate second part of the right side: right2 = h*8*A - // Multiply by 8 by doubling 3 times. This also ensures that the result is in the subgroup. - let pub_key_mul_2 = bjj.curve.add(pub_key, pub_key); - let pub_key_mul_4 = bjj.curve.add(pub_key_mul_2, pub_key_mul_2); - let pub_key_mul_8 = bjj.curve.add(pub_key_mul_4, pub_key_mul_4); - // We check that A8 is not zero. - assert(!pub_key_mul_8.is_zero()); - // Compute the right side: R8 + h * A8 - let right = bjj.curve.add(signature_r8, bjj.curve.mul(hash, pub_key_mul_8)); - // Calculate left side of equation left = S * B8 - let left = bjj.curve.mul(signature_s, bjj.base8); - - left.eq(right) -} - -// Returns the public key of the given secret key as (pub_key_x, pub_key_y) -pub fn eddsa_to_pub(secret: Field) -> (Field, Field) { - let bjj = baby_jubjub(); - let pub_key = bjj.curve.mul(secret, bjj.curve.gen); - (pub_key.x, pub_key.y) -} diff --git a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr index d55044907ac..b9a2b02c9d9 100644 --- a/noir/noir-repo/noir_stdlib/src/hash/sha256.nr +++ b/noir/noir-repo/noir_stdlib/src/hash/sha256.nr @@ -4,27 +4,27 @@ use crate::runtime::is_unconstrained; // 32 bytes. // A message block is up to 64 bytes taken from the input. -global BLOCK_SIZE = 64; +global BLOCK_SIZE: u32 = 64; // The first index in the block where the 8 byte message size will be written. -global MSG_SIZE_PTR = 56; +global MSG_SIZE_PTR: u32 = 56; // Size of the message block when packed as 4-byte integer array. -global INT_BLOCK_SIZE = 16; +global INT_BLOCK_SIZE: u32 = 16; // A `u32` integer consists of 4 bytes. -global INT_SIZE = 4; +global INT_SIZE: u32 = 4; // Index of the integer in the `INT_BLOCK` where the length is written. -global INT_SIZE_PTR = MSG_SIZE_PTR / INT_SIZE; +global INT_SIZE_PTR: u32 = MSG_SIZE_PTR / INT_SIZE; // Magic numbers for bit shifting. // Works with actual bit shifting as well as the compiler turns them into * and / // but circuit execution appears to be 10% faster this way. -global TWO_POW_8 = 256; -global TWO_POW_16 = TWO_POW_8 * 256; -global TWO_POW_24 = TWO_POW_16 * 256; -global TWO_POW_32 = TWO_POW_24 as u64 * 256; +global TWO_POW_8: u32 = 256; +global TWO_POW_16: u32 = TWO_POW_8 * 256; +global TWO_POW_24: u32 = TWO_POW_16 * 256; +global TWO_POW_32: u64 = TWO_POW_24 as u64 * 256; // Index of a byte in a 64 byte block; ie. 0..=63 type BLOCK_BYTE_PTR = u32; diff --git a/noir/noir-repo/noir_stdlib/src/lib.nr b/noir/noir-repo/noir_stdlib/src/lib.nr index 91a1980fe70..b1aabc48d3b 100644 --- a/noir/noir-repo/noir_stdlib/src/lib.nr +++ b/noir/noir-repo/noir_stdlib/src/lib.nr @@ -6,7 +6,6 @@ pub mod merkle; pub mod schnorr; pub mod ecdsa_secp256k1; pub mod ecdsa_secp256r1; -pub mod eddsa; pub mod embedded_curve_ops; pub mod sha256; pub mod sha512; diff --git a/noir/noir-repo/noir_stdlib/src/mem.nr b/noir/noir-repo/noir_stdlib/src/mem.nr index 0d47a21b50d..23125867eab 100644 --- a/noir/noir-repo/noir_stdlib/src/mem.nr +++ b/noir/noir-repo/noir_stdlib/src/mem.nr @@ -15,3 +15,17 @@ pub fn zeroed() -> T {} /// that it is equal to the previous. #[builtin(checked_transmute)] pub fn checked_transmute(value: T) -> U {} + +/// Returns the internal reference count of an array value in unconstrained code. +/// +/// Arrays only have reference count in unconstrained code - using this anywhere +/// else will return zero. +#[builtin(array_refcount)] +pub fn array_refcount(array: [T; N]) -> u32 {} + +/// Returns the internal reference count of a slice value in unconstrained code. +/// +/// Slices only have reference count in unconstrained code - using this anywhere +/// else will return zero. +#[builtin(slice_refcount)] +pub fn slice_refcount(slice: [T]) -> u32 {} diff --git a/noir/noir-repo/test_programs/benchmarks/bench_eddsa_poseidon/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_eddsa_poseidon/src/main.nr index cb853e48c30..bcb0930cf24 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_eddsa_poseidon/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_eddsa_poseidon/src/main.nr @@ -1,4 +1,8 @@ -use std::eddsa::eddsa_poseidon_verify; +use std::default::Default; +use std::ec::consts::te::baby_jubjub; +use std::ec::tecurve::affine::Point as TEPoint; +use std::hash::Hasher; +use std::hash::poseidon::PoseidonHasher; fn main( msg: pub Field, @@ -6,7 +10,52 @@ fn main( pub_key_y: Field, r8_x: Field, r8_y: Field, - s: Field + s: Field, ) -> pub bool { - eddsa_poseidon_verify(pub_key_x, pub_key_y, s, r8_x, r8_y, msg) + eddsa_verify::(pub_key_x, pub_key_y, s, r8_x, r8_y, msg) +} + +pub fn eddsa_verify( + pub_key_x: Field, + pub_key_y: Field, + signature_s: Field, + signature_r8_x: Field, + signature_r8_y: Field, + message: Field, +) -> bool +where + H: Hasher + Default, +{ + // Verifies by testing: + // S * B8 = R8 + H(R8, A, m) * A8 + let bjj = baby_jubjub(); + + let pub_key = TEPoint::new(pub_key_x, pub_key_y); + assert(bjj.curve.contains(pub_key)); + + let signature_r8 = TEPoint::new(signature_r8_x, signature_r8_y); + assert(bjj.curve.contains(signature_r8)); + // Ensure S < Subgroup Order + assert(signature_s.lt(bjj.suborder)); + // Calculate the h = H(R, A, msg) + let mut hasher = H::default(); + hasher.write(signature_r8_x); + hasher.write(signature_r8_y); + hasher.write(pub_key_x); + hasher.write(pub_key_y); + hasher.write(message); + let hash: Field = hasher.finish(); + // Calculate second part of the right side: right2 = h*8*A + // Multiply by 8 by doubling 3 times. This also ensures that the result is in the subgroup. + let pub_key_mul_2 = bjj.curve.add(pub_key, pub_key); + let pub_key_mul_4 = bjj.curve.add(pub_key_mul_2, pub_key_mul_2); + let pub_key_mul_8 = bjj.curve.add(pub_key_mul_4, pub_key_mul_4); + // We check that A8 is not zero. + assert(!pub_key_mul_8.is_zero()); + // Compute the right side: R8 + h * A8 + let right = bjj.curve.add(signature_r8, bjj.curve.mul(hash, pub_key_mul_8)); + // Calculate left side of equation left = S * B8 + let left = bjj.curve.mul(signature_s, bjj.base8); + + left.eq(right) } diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_100/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_100/src/main.nr index 39c714e524f..66a785f446a 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_100/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_100/src/main.nr @@ -1,6 +1,6 @@ use std::hash::poseidon2; -global SIZE = 100; +global SIZE: u32 = 100; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_30/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_30/src/main.nr index d1251a4c853..2e72ebc3519 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_30/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon2_hash_30/src/main.nr @@ -1,6 +1,6 @@ use std::hash::poseidon2; -global SIZE = 30; +global SIZE: u32 = 30; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_100/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_100/src/main.nr index 1c9bbfe61bf..75d853941e5 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_100/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_100/src/main.nr @@ -1,6 +1,6 @@ use std::hash; -global SIZE = 100; +global SIZE: u32 = 100; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_30/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_30/src/main.nr index 3edb47e9f72..d4f357e11f9 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_30/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash/bench_poseidon_hash_30/src/main.nr @@ -1,6 +1,6 @@ use std::hash; -global SIZE = 30; +global SIZE: u32 = 30; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_100/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_100/src/main.nr index 1c9bbfe61bf..75d853941e5 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_100/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_100/src/main.nr @@ -1,6 +1,6 @@ use std::hash; -global SIZE = 100; +global SIZE: u32 = 100; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_30/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_30/src/main.nr index 3edb47e9f72..d4f357e11f9 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_30/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_poseidon_hash_30/src/main.nr @@ -1,6 +1,6 @@ use std::hash; -global SIZE = 30; +global SIZE: u32 = 30; fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] { let mut results: [Field; SIZE] = [0; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_sha256_100/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_sha256_100/src/main.nr index 6df856a83fc..6e4bfc27c8f 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_sha256_100/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_sha256_100/src/main.nr @@ -1,4 +1,4 @@ -global SIZE = 100; +global SIZE: u32 = 100; fn main(input: [[u8; 2]; SIZE]) -> pub [[u8; 32]; SIZE] { let mut results: [[u8; 32]; SIZE] = [[0; 32]; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_sha256_30/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_sha256_30/src/main.nr index 220c1cfbbed..0a4288114e3 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_sha256_30/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_sha256_30/src/main.nr @@ -1,4 +1,4 @@ -global SIZE = 30; +global SIZE: u32 = 30; fn main(input: [[u8; 2]; SIZE]) -> pub [[u8; 32]; SIZE] { let mut results: [[u8; 32]; SIZE] = [[0; 32]; SIZE]; diff --git a/noir/noir-repo/test_programs/benchmarks/bench_sha256_long/src/main.nr b/noir/noir-repo/test_programs/benchmarks/bench_sha256_long/src/main.nr index 17129275371..c47bdc2a561 100644 --- a/noir/noir-repo/test_programs/benchmarks/bench_sha256_long/src/main.nr +++ b/noir/noir-repo/test_programs/benchmarks/bench_sha256_long/src/main.nr @@ -1,6 +1,6 @@ // Input size long enough that we have to compress a few times // and then pad the last block out. -global INPUT_SIZE = 2 * 64 + 60; +global INPUT_SIZE: u32 = 2 * 64 + 60; fn main(input: [u8; INPUT_SIZE]) -> pub [u8; 32] { std::hash::sha256(input) diff --git a/noir/noir-repo/test_programs/compile_success_empty/assert_constant/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/assert_constant/src/main.nr index 978f668f611..42d66f88137 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/assert_constant/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/assert_constant/src/main.nr @@ -1,13 +1,13 @@ use std::static_assert; -global GLOBAL_ONE = 1; -global GLOBAL_TWO = 2; -global GLOBAL_THREE = GLOBAL_ONE + GLOBAL_TWO; +global GLOBAL_ONE: Field = 1; +global GLOBAL_TWO: Field = 2; +global GLOBAL_THREE: Field = GLOBAL_ONE + GLOBAL_TWO; // contents known at compile time // length known at compile time -global GLOBAL_ARRAY_PAIR = [GLOBAL_ONE, GLOBAL_TWO]; -global GLOBAL_SLICE_PAIR = &[GLOBAL_ONE, GLOBAL_TWO]; +global GLOBAL_ARRAY_PAIR: [Field; 2] = [GLOBAL_ONE, GLOBAL_TWO]; +global GLOBAL_SLICE_PAIR: [Field] = &[GLOBAL_ONE, GLOBAL_TWO]; struct Foo { field: Field, diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr index 86b85fbc00a..45afef6d831 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_globals_regression/src/main.nr @@ -1,4 +1,4 @@ -comptime mut global COUNTER = 0; +comptime mut global COUNTER: Field = 0; fn main() { comptime { increment() }; diff --git a/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr index 8114fa34555..20fd8053fbe 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/comptime_module/src/main.nr @@ -24,7 +24,7 @@ mod yet_another_module { #[outer_attribute_separate_module] mod separate_module; -comptime mut global counter = 0; +comptime mut global counter: u32 = 0; comptime fn increment_counter() { counter += 1; diff --git a/noir/noir-repo/test_programs/compile_success_empty/numeric_generics_explicit/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/numeric_generics_explicit/src/main.nr index c2eeeb37395..978a7fdf66b 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/numeric_generics_explicit/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/numeric_generics_explicit/src/main.nr @@ -1,5 +1,5 @@ // Regression that a global of the same name does not trigger a duplicate definition error -global N = 1000; +global N: u32 = 1000; fn main() { let a = id([1, 2]); diff --git a/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr index b596d331e7f..d4479ec933b 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/parenthesized_expression_in_array_length/src/main.nr @@ -1,5 +1,5 @@ -global N = 100; -global BLOCK_SIZE = 10; +global N: u32 = 100; +global BLOCK_SIZE: u32 = 10; fn main() { let _: [Field; 110] = [0; ((N + BLOCK_SIZE) * BLOCK_SIZE) / BLOCK_SIZE]; diff --git a/noir/noir-repo/test_programs/compile_success_empty/raw_string/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/raw_string/src/main.nr index ad8dfe82ae5..6bed1cfecc9 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/raw_string/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/raw_string/src/main.nr @@ -1,4 +1,4 @@ -global D = r#####"Hello "world""#####; +global D: str<13> = r#####"Hello "world""#####; fn main() { let a = "Hello \"world\""; diff --git a/noir/noir-repo/test_programs/compile_success_empty/static_assert/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/static_assert/src/main.nr index 873efe734e1..fda310ba7eb 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/static_assert/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/static_assert/src/main.nr @@ -1,13 +1,13 @@ use std::static_assert; -global GLOBAL_ONE = 1; -global GLOBAL_TWO = 2; -global GLOBAL_THREE = GLOBAL_ONE + GLOBAL_TWO; +global GLOBAL_ONE: Field = 1; +global GLOBAL_TWO: Field = 2; +global GLOBAL_THREE: Field = GLOBAL_ONE + GLOBAL_TWO; // contents known at compile time // length known at compile time -global GLOBAL_ARRAY_PAIR = [GLOBAL_ONE, GLOBAL_TWO]; -global GLOBAL_SLICE_PAIR = &[GLOBAL_ONE, GLOBAL_TWO]; +global GLOBAL_ARRAY_PAIR: [Field; 2] = [GLOBAL_ONE, GLOBAL_TWO]; +global GLOBAL_SLICE_PAIR: [Field] = &[GLOBAL_ONE, GLOBAL_TWO]; pub struct Foo { field: Field, diff --git a/noir/noir-repo/test_programs/compile_success_empty/unquote_multiple_items_from_annotation/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/unquote_multiple_items_from_annotation/src/main.nr index 11d50fc2ab5..591c03de905 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/unquote_multiple_items_from_annotation/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/unquote_multiple_items_from_annotation/src/main.nr @@ -8,7 +8,7 @@ fn main() { comptime fn foo(_: StructDefinition) -> Quoted { quote { - global ONE = 1; - global TWO = 2; + global ONE: Field = 1; + global TWO: u32 = 2; } } diff --git a/noir/noir-repo/test_programs/compile_success_no_bug/databus_mapping_regression/src/main.nr b/noir/noir-repo/test_programs/compile_success_no_bug/databus_mapping_regression/src/main.nr index ff74c82f2ee..9b6ad264a9e 100644 --- a/noir/noir-repo/test_programs/compile_success_no_bug/databus_mapping_regression/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_no_bug/databus_mapping_regression/src/main.nr @@ -23,8 +23,8 @@ pub fn array_to_bounded_vec(array: [T; N]) -> BoundedVec wh BoundedVec { storage: array, len } } -global TX_SIZE = 5; -global APP_CALL_SIZE = 2; +global TX_SIZE: u32 = 5; +global APP_CALL_SIZE: u32 = 2; fn main( a: call_data(0) [Field; TX_SIZE], diff --git a/noir/noir-repo/test_programs/execution_success/bench_2_to_17/src/main.nr b/noir/noir-repo/test_programs/execution_success/bench_2_to_17/src/main.nr index ae80dfcf0b4..204fbc38a16 100644 --- a/noir/noir-repo/test_programs/execution_success/bench_2_to_17/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/bench_2_to_17/src/main.nr @@ -1,6 +1,6 @@ use std::hash::poseidon2; -global len = 2450 * 2; +global len: u32 = 2450 * 2; fn main(x: Field) { let ped_input = [x; len]; let mut val = poseidon2::Poseidon2::hash(ped_input, len); diff --git a/noir/noir-repo/test_programs/execution_success/brillig_cow/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_cow/src/main.nr index 1d4c7f3172e..2dd0d4b3411 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_cow/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_cow/src/main.nr @@ -1,5 +1,5 @@ // Tests the copy on write optimization for arrays. We look for cases where we are modifying an array in place when we shouldn't. -global ARRAY_SIZE = 5; +global ARRAY_SIZE: u32 = 5; struct ExecutionResult { original: [Field; ARRAY_SIZE], diff --git a/noir/noir-repo/test_programs/execution_success/brillig_cow_assign/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_cow_assign/src/main.nr index 73b91e24bea..cfa228b3a96 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_cow_assign/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_cow_assign/src/main.nr @@ -1,4 +1,4 @@ -global N = 10; +global N: u32 = 10; unconstrained fn main() { let mut arr = [0; N]; diff --git a/noir/noir-repo/test_programs/execution_success/brillig_cow_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_cow_regression/src/main.nr index ad2a291f87d..69273bc3dca 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_cow_regression/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/brillig_cow_regression/src/main.nr @@ -7,7 +7,7 @@ global MAX_NEW_CONTRACTS_PER_TX: u32 = 1; global NUM_ENCRYPTED_LOGS_HASHES_PER_TX: u32 = 1; global NUM_UNENCRYPTED_LOGS_HASHES_PER_TX: u32 = 1; global NUM_FIELDS_PER_SHA256: u32 = 2; -global TX_EFFECT_HASH_INPUT_SIZE = 169; +global TX_EFFECT_HASH_INPUT_SIZE: u32 = 169; global TX_EFFECT_HASH_LOG_FIELDS: u32 = 4; global TX_EFFECT_HASH_FULL_FIELDS: u32 = 165; diff --git a/noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml similarity index 58% rename from noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml rename to noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml index f23ecc787d0..68bcf9929cc 100644 --- a/noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/Nargo.toml +++ b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "brillig_unitialised_arrays" +name = "brillig_uninitialized_arrays" type = "bin" authors = [""] diff --git a/noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/Prover.toml b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/Prover.toml similarity index 100% rename from noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/Prover.toml rename to noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/Prover.toml diff --git a/noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr b/noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr similarity index 100% rename from noir/noir-repo/test_programs/execution_success/brillig_unitialised_arrays/src/main.nr rename to noir/noir-repo/test_programs/execution_success/brillig_uninitialized_arrays/src/main.nr diff --git a/noir/noir-repo/test_programs/execution_success/eddsa/src/main.nr b/noir/noir-repo/test_programs/execution_success/eddsa/src/main.nr index d4c3664f0c9..83e6c6565a9 100644 --- a/noir/noir-repo/test_programs/execution_success/eddsa/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/eddsa/src/main.nr @@ -1,7 +1,8 @@ use std::compat; use std::ec::consts::te::baby_jubjub; use std::ec::tecurve::affine::Point as TEPoint; -use std::eddsa::{eddsa_poseidon_verify, eddsa_to_pub, eddsa_verify}; +use std::hash::Hasher; +use std::hash::poseidon::PoseidonHasher; use std::hash::poseidon2::Poseidon2Hasher; fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { @@ -54,3 +55,74 @@ fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { ); } } + +// Returns true if signature is valid +pub fn eddsa_poseidon_verify( + pub_key_x: Field, + pub_key_y: Field, + signature_s: Field, + signature_r8_x: Field, + signature_r8_y: Field, + message: Field, +) -> bool { + eddsa_verify::( + pub_key_x, + pub_key_y, + signature_s, + signature_r8_x, + signature_r8_y, + message, + ) +} + +pub fn eddsa_verify( + pub_key_x: Field, + pub_key_y: Field, + signature_s: Field, + signature_r8_x: Field, + signature_r8_y: Field, + message: Field, +) -> bool +where + H: Hasher + Default, +{ + // Verifies by testing: + // S * B8 = R8 + H(R8, A, m) * A8 + let bjj = baby_jubjub(); + + let pub_key = TEPoint::new(pub_key_x, pub_key_y); + assert(bjj.curve.contains(pub_key)); + + let signature_r8 = TEPoint::new(signature_r8_x, signature_r8_y); + assert(bjj.curve.contains(signature_r8)); + // Ensure S < Subgroup Order + assert(signature_s.lt(bjj.suborder)); + // Calculate the h = H(R, A, msg) + let mut hasher = H::default(); + hasher.write(signature_r8_x); + hasher.write(signature_r8_y); + hasher.write(pub_key_x); + hasher.write(pub_key_y); + hasher.write(message); + let hash: Field = hasher.finish(); + // Calculate second part of the right side: right2 = h*8*A + // Multiply by 8 by doubling 3 times. This also ensures that the result is in the subgroup. + let pub_key_mul_2 = bjj.curve.add(pub_key, pub_key); + let pub_key_mul_4 = bjj.curve.add(pub_key_mul_2, pub_key_mul_2); + let pub_key_mul_8 = bjj.curve.add(pub_key_mul_4, pub_key_mul_4); + // We check that A8 is not zero. + assert(!pub_key_mul_8.is_zero()); + // Compute the right side: R8 + h * A8 + let right = bjj.curve.add(signature_r8, bjj.curve.mul(hash, pub_key_mul_8)); + // Calculate left side of equation left = S * B8 + let left = bjj.curve.mul(signature_s, bjj.base8); + + left.eq(right) +} + +// Returns the public key of the given secret key as (pub_key_x, pub_key_y) +pub fn eddsa_to_pub(secret: Field) -> (Field, Field) { + let bjj = baby_jubjub(); + let pub_key = bjj.curve.mul(secret, bjj.curve.gen); + (pub_key.x, pub_key.y) +} diff --git a/noir/noir-repo/test_programs/execution_success/fmtstr_with_global/src/main.nr b/noir/noir-repo/test_programs/execution_success/fmtstr_with_global/src/main.nr index 8b9c9635015..4ca118f856f 100644 --- a/noir/noir-repo/test_programs/execution_success/fmtstr_with_global/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/fmtstr_with_global/src/main.nr @@ -1,4 +1,4 @@ -global FOO = 1; +global FOO: Field = 1; fn main() { println(f"foo = {FOO}"); diff --git a/noir/noir-repo/test_programs/execution_success/fold_2_to_17/src/main.nr b/noir/noir-repo/test_programs/execution_success/fold_2_to_17/src/main.nr index a3a747e4aee..d54dff4617a 100644 --- a/noir/noir-repo/test_programs/execution_success/fold_2_to_17/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/fold_2_to_17/src/main.nr @@ -1,6 +1,6 @@ use std::hash::poseidon2; -global len = 2450 * 2 - 240; // for just under 2^17 gates +global len: u32 = 2450 * 2 - 240; // for just under 2^17 gates fn main(x: Field) { let ped_input = [x; len]; let mut val = poseidon2::Poseidon2::hash(ped_input, len); diff --git a/noir/noir-repo/test_programs/execution_success/fold_call_witness_condition/src/main.nr b/noir/noir-repo/test_programs/execution_success/fold_call_witness_condition/src/main.nr index 5dc75e4a99f..5b9a5db62c5 100644 --- a/noir/noir-repo/test_programs/execution_success/fold_call_witness_condition/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/fold_call_witness_condition/src/main.nr @@ -1,4 +1,4 @@ -global NUM_RESULTS = 2; +global NUM_RESULTS: u32 = 2; fn main(x: Field, y: pub Field, enable: bool) -> pub [Field; NUM_RESULTS] { let mut result = [0; NUM_RESULTS]; for i in 0..NUM_RESULTS { diff --git a/noir/noir-repo/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr b/noir/noir-repo/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr index c5993cf6523..15b9dd26195 100644 --- a/noir/noir-repo/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/fold_numeric_generic_poseidon/src/main.nr @@ -1,7 +1,7 @@ -use std::hash::{pedersen_hash_with_separator, poseidon2::Poseidon2}; +use std::hash::poseidon2::Poseidon2; global NUM_HASHES: u32 = 2; -global HASH_LENGTH = 10; +global HASH_LENGTH: u32 = 10; #[fold] pub fn poseidon_hash(inputs: [Field; N]) -> Field { diff --git a/noir/noir-repo/test_programs/execution_success/global_consts/src/foo.nr b/noir/noir-repo/test_programs/execution_success/global_consts/src/foo.nr index 50e331493dc..2c39b534259 100644 --- a/noir/noir-repo/test_programs/execution_success/global_consts/src/foo.nr +++ b/noir/noir-repo/test_programs/execution_success/global_consts/src/foo.nr @@ -2,7 +2,7 @@ mod bar; global N: u32 = 5; global MAGIC_NUMBER: u32 = 3; -global TYPE_INFERRED = 42; +global TYPE_INFERRED: u32 = 42; pub fn from_foo(x: [Field; bar::N]) { for i in 0..bar::N { diff --git a/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr b/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr index 30c5f7167f3..2eaab810d6a 100644 --- a/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/global_consts/src/main.nr @@ -18,7 +18,7 @@ struct Test { v: Field, } global VALS: [Test; 1] = [Test { v: 100 }]; -global NESTED = [VALS, VALS]; +global NESTED: [[Test; 1]; 2] = [VALS, VALS]; unconstrained fn calculate_global_value() -> Field { 42 @@ -121,4 +121,4 @@ impl Bar { } // Regression for #1440 -global foo = Foo { a: Bar::get_a() }; +global foo: Foo = Foo { a: Bar::get_a() }; diff --git a/noir/noir-repo/test_programs/execution_success/hashmap/src/main.nr b/noir/noir-repo/test_programs/execution_success/hashmap/src/main.nr index 964b900dce5..cfd4e4a9136 100644 --- a/noir/noir-repo/test_programs/execution_success/hashmap/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/hashmap/src/main.nr @@ -16,15 +16,15 @@ struct Entry { } global HASHMAP_CAP: u32 = 8; -global HASHMAP_LEN = 6; +global HASHMAP_LEN: u32 = 6; -global FIELD_CMP = |a: Field, b: Field| a.lt(b); +global FIELD_CMP: fn(Field, Field) -> bool = |a: Field, b: Field| a.lt(b); -global K_CMP = FIELD_CMP; -global V_CMP = FIELD_CMP; -global KV_CMP = |a: (K, V), b: (K, V)| a.0.lt(b.0); +global K_CMP: fn(Field, Field) -> bool = FIELD_CMP; +global V_CMP: fn(Field, Field) -> bool = FIELD_CMP; +global KV_CMP: fn((K, V), (K, V)) -> bool = |a: (K, V), b: (K, V)| a.0.lt(b.0); -global ALLOCATE_HASHMAP = +global ALLOCATE_HASHMAP: fn() -> HashMap> = || -> HashMap> HashMap::default(); fn main(input: [Entry; HASHMAP_LEN]) { diff --git a/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Nargo.toml b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Nargo.toml new file mode 100644 index 00000000000..9590789f52e --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "loop_invariant_regression" +type = "bin" +authors = [""] +compiler_version = ">=0.38.0" + +[dependencies] \ No newline at end of file diff --git a/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Prover.toml b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Prover.toml new file mode 100644 index 00000000000..18680c805a7 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/Prover.toml @@ -0,0 +1,2 @@ +x = "2" +y = "3" diff --git a/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/src/main.nr new file mode 100644 index 00000000000..25f6e92f868 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/loop_invariant_regression/src/main.nr @@ -0,0 +1,13 @@ +// Tests a simple loop where we expect loop invariant instructions +// to be hoisted to the loop's pre-header block. +fn main(x: u32, y: u32) { + loop(4, x, y); +} + +fn loop(upper_bound: u32, x: u32, y: u32) { + for _ in 0..upper_bound { + let mut z = x * y; + z = z * x; + assert_eq(z, 12); + } +} diff --git a/noir/noir-repo/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr b/noir/noir-repo/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr index aa1106132ff..82a868f3ffb 100644 --- a/noir/noir-repo/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/no_predicates_numeric_generic_poseidon/src/main.nr @@ -1,7 +1,7 @@ use std::hash::poseidon2::Poseidon2; global NUM_HASHES: u32 = 2; -global HASH_LENGTH = 10; +global HASH_LENGTH: u32 = 10; #[no_predicates] pub fn poseidon_hash(inputs: [Field; N]) -> Field { diff --git a/noir/noir-repo/test_programs/execution_success/ram_blowup_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/ram_blowup_regression/src/main.nr index 59843c368ec..6deb54dd21d 100644 --- a/noir/noir-repo/test_programs/execution_success/ram_blowup_regression/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/ram_blowup_regression/src/main.nr @@ -1,4 +1,4 @@ -global TX_EFFECTS_HASH_INPUT_FIELDS = 256; +global TX_EFFECTS_HASH_INPUT_FIELDS: u32 = 256; // Convert a 32 byte array to a field element by truncating the final byte pub fn field_from_bytes_32_trunc(bytes32: [u8; 32]) -> Field { diff --git a/noir/noir-repo/test_programs/execution_success/reference_counts/Nargo.toml b/noir/noir-repo/test_programs/execution_success/reference_counts/Nargo.toml new file mode 100644 index 00000000000..ae787e0ccb9 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/reference_counts/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "reference_counts" +type = "bin" +authors = [""] +compiler_version = ">=0.35.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/execution_success/reference_counts/Prover.toml b/noir/noir-repo/test_programs/execution_success/reference_counts/Prover.toml new file mode 100644 index 00000000000..c01dd9462d8 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/reference_counts/Prover.toml @@ -0,0 +1,2 @@ +x = 5 +b = true diff --git a/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr new file mode 100644 index 00000000000..7ab7de893fa --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr @@ -0,0 +1,40 @@ +fn main() { + let mut array = [0, 1, 2]; + assert_refcount(array, 1); + + borrow(array, std::mem::array_refcount(array)); + borrow_mut(&mut array, std::mem::array_refcount(array)); + copy_mut(array, std::mem::array_refcount(array)); +} + +fn borrow(array: [Field; 3], rc_before_call: u32) { + assert_refcount(array, rc_before_call); + println(array[0]); +} + +fn borrow_mut(array: &mut [Field; 3], rc_before_call: u32) { + assert_refcount(*array, rc_before_call + 0); // Issue! This should be rc_before_call + 1 + array[0] = 5; + println(array[0]); +} + +fn copy_mut(mut array: [Field; 3], rc_before_call: u32) { + assert_refcount(array, rc_before_call + 0); // Issue! This should be rc_before_call + 1 + array[0] = 6; + println(array[0]); +} + +fn assert_refcount(array: [Field; 3], expected: u32) { + let count = std::mem::array_refcount(array); + + // All refcounts are zero when running this as a constrained program + if std::runtime::is_unconstrained() { + if count != expected { + // Brillig doesn't print the actual & expected arguments on assertion failure + println(f"actual = {count}, expected = {expected}"); + } + assert_eq(count, expected); + } else { + assert_eq(count, 0); + } +} diff --git a/noir/noir-repo/test_programs/execution_success/regression_2660/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_2660/src/main.nr index f32bc924e3a..92aa15abb43 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_2660/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_2660/src/main.nr @@ -1,4 +1,4 @@ -global foo = -1; +global foo: i32 = -1; fn main(x: i32) { let y = x + foo; diff --git a/noir/noir-repo/test_programs/execution_success/regression_5252/src/main.nr b/noir/noir-repo/test_programs/execution_success/regression_5252/src/main.nr index 6ab4157e7a5..5f56b7f7f35 100644 --- a/noir/noir-repo/test_programs/execution_success/regression_5252/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/regression_5252/src/main.nr @@ -1,7 +1,7 @@ use std::hash::{poseidon, poseidon2::Poseidon2}; -global NUM_HASHES = 3; -global HASH_LENGTH = 20; +global NUM_HASHES: u32 = 3; +global HASH_LENGTH: u32 = 20; pub fn poseidon_hash(inputs: [Field; N]) -> Field { Poseidon2::hash(inputs, inputs.len()) diff --git a/noir/noir-repo/test_programs/execution_success/sha256_var_size_regression/src/main.nr b/noir/noir-repo/test_programs/execution_success/sha256_var_size_regression/src/main.nr index de1c2b23c5f..4278cdda8a3 100644 --- a/noir/noir-repo/test_programs/execution_success/sha256_var_size_regression/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/sha256_var_size_regression/src/main.nr @@ -1,4 +1,4 @@ -global NUM_HASHES = 2; +global NUM_HASHES: u32 = 2; fn main(foo: [u8; 95], toggle: bool, enable: [bool; NUM_HASHES]) { let mut result = [[0; 32]; NUM_HASHES]; diff --git a/noir/noir-repo/test_programs/execution_success/strings/src/main.nr b/noir/noir-repo/test_programs/execution_success/strings/src/main.nr index d28a9f483ac..c4fa0539745 100644 --- a/noir/noir-repo/test_programs/execution_success/strings/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/strings/src/main.nr @@ -1,5 +1,5 @@ // Test global string literals -global HELLO_WORLD = "hello world"; +global HELLO_WORLD: str<11> = "hello world"; fn main(message: pub str<11>, y: Field, hex_as_string: str<4>, hex_as_field: Field) { let mut bad_message = "hello world"; diff --git a/noir/noir-repo/test_programs/execution_success/struct_inputs/src/foo/bar.nr b/noir/noir-repo/test_programs/execution_success/struct_inputs/src/foo/bar.nr index 6d879326677..7a79528f8ab 100644 --- a/noir/noir-repo/test_programs/execution_success/struct_inputs/src/foo/bar.nr +++ b/noir/noir-repo/test_programs/execution_success/struct_inputs/src/foo/bar.nr @@ -1,4 +1,4 @@ -global N = 2; +global N: Field = 2; struct barStruct { val: Field, diff --git a/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr b/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr index e917a83c5fd..b56a4fe1747 100644 --- a/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/uhashmap/src/main.nr @@ -11,15 +11,15 @@ struct Entry { value: Field, } -global HASHMAP_LEN = 6; +global HASHMAP_LEN: u32 = 6; -global FIELD_CMP = |a: Field, b: Field| a.lt(b); +global FIELD_CMP: fn(Field, Field) -> bool = |a: Field, b: Field| a.lt(b); -global K_CMP = FIELD_CMP; -global V_CMP = FIELD_CMP; -global KV_CMP = |a: (K, V), b: (K, V)| a.0.lt(b.0); +global K_CMP: fn(Field, Field) -> bool = FIELD_CMP; +global V_CMP: fn(Field, Field) -> bool = FIELD_CMP; +global KV_CMP: fn((K, V), (K, V)) -> bool = |a: (K, V), b: (K, V)| a.0.lt(b.0); -global ALLOCATE_HASHMAP = +global ALLOCATE_HASHMAP: fn() -> UHashMap> = || -> UHashMap> UHashMap::default(); unconstrained fn main(input: [Entry; HASHMAP_LEN]) { diff --git a/noir/noir-repo/test_programs/test_libraries/diamond_deps_2/src/lib.nr b/noir/noir-repo/test_programs/test_libraries/diamond_deps_2/src/lib.nr index 46dce3d5600..23de4d4c0f3 100644 --- a/noir/noir-repo/test_programs/test_libraries/diamond_deps_2/src/lib.nr +++ b/noir/noir-repo/test_programs/test_libraries/diamond_deps_2/src/lib.nr @@ -1,4 +1,4 @@ -global RESOLVE_THIS = 3; +global RESOLVE_THIS: Field = 3; pub fn call_dep2(x: Field, y: Field) -> Field { x + y diff --git a/noir/noir-repo/tooling/debugger/ignored-tests.txt b/noir/noir-repo/tooling/debugger/ignored-tests.txt index 0037b8e5d5f..e0548fe1e1a 100644 --- a/noir/noir-repo/tooling/debugger/ignored-tests.txt +++ b/noir/noir-repo/tooling/debugger/ignored-tests.txt @@ -2,7 +2,8 @@ brillig_references debug_logs is_unconstrained macros +reference_counts references regression_4709 reference_only_used_as_alias -brillig_rc_regression_6123 \ No newline at end of file +brillig_rc_regression_6123 diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index ad1f82f4e45..04eb0ea0f52 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -60,9 +60,13 @@ const IGNORED_BRILLIG_TESTS: [&str; 11] = [ ]; /// Tests which aren't expected to work with the default inliner cases. -const INLINER_MIN_OVERRIDES: [(&str, i64); 1] = [ +const INLINER_MIN_OVERRIDES: [(&str, i64); 2] = [ // 0 works if PoseidonHasher::write is tagged as `inline_always`, otherwise 22. ("eddsa", 0), + // (#6583): The RcTracker in the DIE SSA pass is removing inc_rcs that are still needed. + // This triggers differently depending on the optimization level (although all are wrong), + // so we arbitrarily only run with the inlined versions. + ("reference_counts", 0), ]; /// Some tests are expected to have warnings diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs index c69775d3323..ffeb5d9ba74 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs @@ -5,7 +5,6 @@ use super::NargoConfig; use clap::Args; use nargo::constants::{PKG_FILE, SRC_DIR}; use nargo::package::{CrateName, PackageType}; -use noirc_driver::NOIRC_VERSION; use std::path::PathBuf; /// Create a Noir project in the current directory. @@ -66,7 +65,6 @@ pub(crate) fn initialize_project( name = "{package_name}" type = "{package_type}" authors = [""] -compiler_version = ">={NOIRC_VERSION}" [dependencies]"# ); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs index 0ac4c98bb95..0730d06ad72 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/expression.rs @@ -1165,7 +1165,7 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { // Finally format the comment, if any group.text(self.chunk(|formatter| { - formatter.skip_comments_and_whitespace(); + formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found(); })); group.decrease_indentation(); diff --git a/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs b/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs index fd6977df613..8207db5e486 100644 --- a/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs +++ b/noir/noir-repo/tooling/nargo_fmt/src/formatter/function.rs @@ -571,6 +571,36 @@ fn baz() { let z = 3 ; let y = 2; } +"; + let expected = src; + assert_format(src, expected); + } + + #[test] + fn keeps_newlines_between_comments_no_statements() { + let src = "fn foo() { + // foo + + // bar + + // baz +} +"; + let expected = src; + assert_format(src, expected); + } + + #[test] + fn keeps_newlines_between_comments_one_statement() { + let src = "fn foo() { + let x = 1; + + // foo + + // bar + + // baz +} "; let expected = src; assert_format(src, expected); diff --git a/noir/noir-repo/tooling/nargo_toml/src/errors.rs b/noir/noir-repo/tooling/nargo_toml/src/errors.rs index 1ee8e90c8e5..7e1003d04f7 100644 --- a/noir/noir-repo/tooling/nargo_toml/src/errors.rs +++ b/noir/noir-repo/tooling/nargo_toml/src/errors.rs @@ -80,6 +80,8 @@ pub enum ManifestError { #[allow(clippy::enum_variant_names)] #[derive(Error, Debug, PartialEq, Eq, Clone)] pub enum SemverError { + #[error("Invalid value for `compiler_version` in package {package_name}. Requirements may only refer to full releases")] + InvalidCompilerVersionRequirement { package_name: CrateName, required_compiler_version: String }, #[error("Incompatible compiler version in package {package_name}. Required compiler version is {required_compiler_version} but the compiler version is {compiler_version_found}.\n Update the compiler_version field in Nargo.toml to >={required_compiler_version} or compile this project with version {required_compiler_version}")] IncompatibleVersion { package_name: CrateName, diff --git a/noir/noir-repo/tooling/nargo_toml/src/semver.rs b/noir/noir-repo/tooling/nargo_toml/src/semver.rs index 253ac82aa34..ececa1b30dd 100644 --- a/noir/noir-repo/tooling/nargo_toml/src/semver.rs +++ b/noir/noir-repo/tooling/nargo_toml/src/semver.rs @@ -3,11 +3,14 @@ use nargo::{ package::{Dependency, Package}, workspace::Workspace, }; -use semver::{Error, Version, VersionReq}; +use noirc_driver::CrateName; +use semver::{Error, Prerelease, Version, VersionReq}; // Parse a semver compatible version string pub(crate) fn parse_semver_compatible_version(version: &str) -> Result { - Version::parse(version) + let mut version = Version::parse(version)?; + version.pre = Prerelease::EMPTY; + Ok(version) } // Check that all of the packages in the workspace are compatible with the current compiler version @@ -25,10 +28,7 @@ pub(crate) fn semver_check_workspace( } // Check that a package and all of its dependencies are compatible with the current compiler version -pub(crate) fn semver_check_package( - package: &Package, - compiler_version: &Version, -) -> Result<(), SemverError> { +fn semver_check_package(package: &Package, compiler_version: &Version) -> Result<(), SemverError> { // Check that this package's compiler version requirements are satisfied if let Some(version) = &package.compiler_required_version { let version_req = match VersionReq::parse(version) { @@ -40,6 +40,9 @@ pub(crate) fn semver_check_package( }) } }; + + validate_compiler_version_requirement(&package.name, &version_req)?; + if !version_req.matches(compiler_version) { return Err(SemverError::IncompatibleVersion { package_name: package.name.clone(), @@ -61,6 +64,20 @@ pub(crate) fn semver_check_package( Ok(()) } +fn validate_compiler_version_requirement( + package_name: &CrateName, + required_compiler_version: &VersionReq, +) -> Result<(), SemverError> { + if required_compiler_version.comparators.iter().any(|comparator| !comparator.pre.is_empty()) { + return Err(SemverError::InvalidCompilerVersionRequirement { + package_name: package_name.clone(), + required_compiler_version: required_compiler_version.to_string(), + }); + } + + Ok(()) +} + // Strip the build meta data from the version string since it is ignored by semver. fn strip_build_meta_data(version: &Version) -> String { let version_string = version.to_string(); @@ -191,6 +208,26 @@ mod tests { }; } + #[test] + fn test_semver_prerelease() { + let compiler_version = parse_semver_compatible_version("1.0.0-beta.0").unwrap(); + + let package = Package { + compiler_required_version: Some(">=0.1.0".to_string()), + root_dir: PathBuf::new(), + package_type: PackageType::Library, + entry_path: PathBuf::new(), + name: CrateName::from_str("test").unwrap(), + dependencies: BTreeMap::new(), + version: Some("1.0".to_string()), + expression_width: None, + }; + + if let Err(err) = semver_check_package(&package, &compiler_version) { + panic!("{err}"); + }; + } + #[test] fn test_semver_build_data() { let compiler_version = Version::parse("0.1.0+this-is-ignored-by-semver").unwrap(); diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/build.sh b/noir/noir-repo/tooling/noirc_abi_wasm/build.sh index c07d2d8a4c1..16fb26e55db 100755 --- a/noir/noir-repo/tooling/noirc_abi_wasm/build.sh +++ b/noir/noir-repo/tooling/noirc_abi_wasm/build.sh @@ -25,7 +25,7 @@ function run_if_available { require_command jq require_command cargo require_command wasm-bindgen -#require_command wasm-opt +require_command wasm-opt self_path=$(dirname "$(readlink -f "$0")") pname=$(cargo read-manifest | jq -r '.name') diff --git a/noir/noir-repo/yarn.lock b/noir/noir-repo/yarn.lock index 03cea21026e..f7b7b3df372 100644 --- a/noir/noir-repo/yarn.lock +++ b/noir/noir-repo/yarn.lock @@ -221,9 +221,9 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@portal:../../../../barretenberg/ts::locator=integration-tests%40workspace%3Acompiler%2Fintegration-tests": - version: 0.0.0-use.local - resolution: "@aztec/bb.js@portal:../../../../barretenberg/ts::locator=integration-tests%40workspace%3Acompiler%2Fintegration-tests" +"@aztec/bb.js@npm:0.63.1": + version: 0.63.1 + resolution: "@aztec/bb.js@npm:0.63.1" dependencies: comlink: ^4.4.1 commander: ^10.0.1 @@ -231,9 +231,10 @@ __metadata: fflate: ^0.8.0 tslib: ^2.4.0 bin: - bb.js: ./dest/node/main.js + bb.js: dest/node/main.js + checksum: b80730f1cb87e4d2ca21d991a42950bc069367896db309ab3f909c5f53efa9291538d51e35bc3c6d2eea042ca33c279ae59eb3f5d844a24336c7bb9664c2404b languageName: node - linkType: soft + linkType: hard "@babel/code-frame@npm:^7.0.0, @babel/code-frame@npm:^7.10.4, @babel/code-frame@npm:^7.12.11, @babel/code-frame@npm:^7.16.0, @babel/code-frame@npm:^7.22.13, @babel/code-frame@npm:^7.23.5, @babel/code-frame@npm:^7.8.3": version: 7.23.5 @@ -14122,7 +14123,7 @@ __metadata: version: 0.0.0-use.local resolution: "integration-tests@workspace:compiler/integration-tests" dependencies: - "@aztec/bb.js": "portal:../../../../barretenberg/ts" + "@aztec/bb.js": 0.63.1 "@noir-lang/noir_js": "workspace:*" "@noir-lang/noir_wasm": "workspace:*" "@nomicfoundation/hardhat-chai-matchers": ^2.0.0 From 08c0e67637fad86c559d5efd68deb370d24f6962 Mon Sep 17 00:00:00 2001 From: AztecBot Date: Sun, 24 Nov 2024 08:04:01 +0000 Subject: [PATCH 2/2] chore: apply sync fixes --- noir/noir-repo/acvm-repo/acvm_js/build.sh | 2 +- .../compiler/integration-tests/package.json | 2 +- noir/noir-repo/tooling/noirc_abi_wasm/build.sh | 2 +- noir/noir-repo/yarn.lock | 13 ++++++------- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/noir/noir-repo/acvm-repo/acvm_js/build.sh b/noir/noir-repo/acvm-repo/acvm_js/build.sh index 16fb26e55db..c07d2d8a4c1 100755 --- a/noir/noir-repo/acvm-repo/acvm_js/build.sh +++ b/noir/noir-repo/acvm-repo/acvm_js/build.sh @@ -25,7 +25,7 @@ function run_if_available { require_command jq require_command cargo require_command wasm-bindgen -require_command wasm-opt +#require_command wasm-opt self_path=$(dirname "$(readlink -f "$0")") pname=$(cargo read-manifest | jq -r '.name') diff --git a/noir/noir-repo/compiler/integration-tests/package.json b/noir/noir-repo/compiler/integration-tests/package.json index a9d437da792..e33179f31e7 100644 --- a/noir/noir-repo/compiler/integration-tests/package.json +++ b/noir/noir-repo/compiler/integration-tests/package.json @@ -13,7 +13,7 @@ "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore --max-warnings 0" }, "dependencies": { - "@aztec/bb.js": "0.63.1", + "@aztec/bb.js": "portal:../../../../barretenberg/ts", "@noir-lang/noir_js": "workspace:*", "@noir-lang/noir_wasm": "workspace:*", "@nomicfoundation/hardhat-chai-matchers": "^2.0.0", diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/build.sh b/noir/noir-repo/tooling/noirc_abi_wasm/build.sh index 16fb26e55db..c07d2d8a4c1 100755 --- a/noir/noir-repo/tooling/noirc_abi_wasm/build.sh +++ b/noir/noir-repo/tooling/noirc_abi_wasm/build.sh @@ -25,7 +25,7 @@ function run_if_available { require_command jq require_command cargo require_command wasm-bindgen -require_command wasm-opt +#require_command wasm-opt self_path=$(dirname "$(readlink -f "$0")") pname=$(cargo read-manifest | jq -r '.name') diff --git a/noir/noir-repo/yarn.lock b/noir/noir-repo/yarn.lock index f7b7b3df372..03cea21026e 100644 --- a/noir/noir-repo/yarn.lock +++ b/noir/noir-repo/yarn.lock @@ -221,9 +221,9 @@ __metadata: languageName: node linkType: hard -"@aztec/bb.js@npm:0.63.1": - version: 0.63.1 - resolution: "@aztec/bb.js@npm:0.63.1" +"@aztec/bb.js@portal:../../../../barretenberg/ts::locator=integration-tests%40workspace%3Acompiler%2Fintegration-tests": + version: 0.0.0-use.local + resolution: "@aztec/bb.js@portal:../../../../barretenberg/ts::locator=integration-tests%40workspace%3Acompiler%2Fintegration-tests" dependencies: comlink: ^4.4.1 commander: ^10.0.1 @@ -231,10 +231,9 @@ __metadata: fflate: ^0.8.0 tslib: ^2.4.0 bin: - bb.js: dest/node/main.js - checksum: b80730f1cb87e4d2ca21d991a42950bc069367896db309ab3f909c5f53efa9291538d51e35bc3c6d2eea042ca33c279ae59eb3f5d844a24336c7bb9664c2404b + bb.js: ./dest/node/main.js languageName: node - linkType: hard + linkType: soft "@babel/code-frame@npm:^7.0.0, @babel/code-frame@npm:^7.10.4, @babel/code-frame@npm:^7.12.11, @babel/code-frame@npm:^7.16.0, @babel/code-frame@npm:^7.22.13, @babel/code-frame@npm:^7.23.5, @babel/code-frame@npm:^7.8.3": version: 7.23.5 @@ -14123,7 +14122,7 @@ __metadata: version: 0.0.0-use.local resolution: "integration-tests@workspace:compiler/integration-tests" dependencies: - "@aztec/bb.js": 0.63.1 + "@aztec/bb.js": "portal:../../../../barretenberg/ts" "@noir-lang/noir_js": "workspace:*" "@noir-lang/noir_wasm": "workspace:*" "@nomicfoundation/hardhat-chai-matchers": ^2.0.0