From 54d81ca7236b2d418cd87d8c26e87f4790ab3d78 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 13:08:05 -0300 Subject: [PATCH 1/3] feat: lock on Nargo.toml on several nargo commands (#6941) --- Cargo.lock | 22 +++++++-------- Cargo.toml | 2 -- tooling/nargo_cli/Cargo.toml | 4 +-- tooling/nargo_cli/build.rs | 23 --------------- tooling/nargo_cli/src/cli/mod.rs | 48 ++++++++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6235fbf4f8f..050c70a0f0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1697,16 +1697,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "file-lock" -version = "2.1.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "040b48f80a749da50292d0f47a1e2d5bf1d772f52836c07f64bfccc62ba6e664" -dependencies = [ - "cc", - "libc", -] - [[package]] name = "filetime" version = "0.2.25" @@ -1780,6 +1770,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fsevent-sys" version = "4.1.0" @@ -2999,8 +2999,8 @@ dependencies = [ "criterion", "dap", "dirs", - "file-lock", "fm", + "fs2", "fxhash", "iai", "iter-extended", diff --git a/Cargo.toml b/Cargo.toml index 8d48a846457..567f1db085b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -83,8 +83,6 @@ noir_lsp = { path = "tooling/lsp" } noir_debugger = { path = "tooling/debugger" } noirc_abi = { path = "tooling/noirc_abi" } noirc_artifacts = { path = "tooling/noirc_artifacts" } -bb_abstraction_leaks = { path = "tooling/bb_abstraction_leaks" } -acvm_cli = { path = "tooling/acvm_cli" } # Arkworks ark-bn254 = { version = "^0.5.0", default-features = false, features = [ diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 742c397a0d3..001306bb162 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -75,6 +75,7 @@ tracing.workspace = true tracing-subscriber.workspace = true tracing-appender = "0.2.3" clap_complete = "4.5.36" +fs2 = "0.4.3" [target.'cfg(not(unix))'.dependencies] tokio-util = { version = "0.7.8", features = ["compat"] } @@ -86,7 +87,6 @@ dirs.workspace = true assert_cmd = "2.0.8" assert_fs = "1.0.10" predicates = "2.1.5" -file-lock = "2.1.11" fm.workspace = true criterion.workspace = true pprof.workspace = true @@ -102,7 +102,7 @@ light-poseidon = "0.2.0" ark-bn254-v04 = { package = "ark-bn254", version = "^0.4.0", default-features = false, features = [ "curve", ] } -ark-ff-v04 = { package = "ark-ff", version = "^0.4.0", default-features = false } +ark-ff-v04 = { package = "ark-ff", version = "^0.4.0", default-features = false } [[bench]] name = "criterion" diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index d041cf3e53f..04cc463f6b3 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -177,33 +177,13 @@ fn generate_test_cases( } let test_cases = test_cases.join("\n"); - // We need to isolate test cases in the same group, otherwise they overwrite each other's artifacts. - // On CI we use `cargo nextest`, which runs tests in different processes; for this we use a file lock. - // Locally we might be using `cargo test`, which run tests in the same process; in this case the file lock - // wouldn't work, becuase the process itself has the lock, and it looks like it can have N instances without - // any problems; for this reason we also use a `Mutex`. - let mutex_name = format! {"TEST_MUTEX_{}", test_name.to_uppercase()}; write!( test_file, r#" -lazy_static::lazy_static! {{ - /// Prevent concurrent tests in the matrix from overwriting the compilation artifacts in {test_dir} - static ref {mutex_name}: std::sync::Mutex<()> = std::sync::Mutex::new(()); -}} - {test_cases} fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner) {{ let test_program_dir = PathBuf::from("{test_dir}"); - // Ignore poisoning errors if some of the matrix cases failed. - let mutex_guard = {mutex_name}.lock().unwrap_or_else(|e| e.into_inner()); - - let file_guard = file_lock::FileLock::lock( - test_program_dir.join("Nargo.toml"), - true, - file_lock::FileOptions::new().read(true).write(true).append(true) - ).expect("failed to lock Nargo.toml"); - let mut nargo = Command::cargo_bin("nargo").unwrap(); nargo.arg("--program-dir").arg(test_program_dir); nargo.arg("{test_command}").arg("--force"); @@ -221,9 +201,6 @@ fn test_{test_name}(force_brillig: ForceBrillig, inliner_aggressiveness: Inliner }} {test_content} - - drop(file_guard); - drop(mutex_guard); }} "# ) diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 43e3de9c6d0..0b725afcf4e 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -118,6 +118,11 @@ enum NargoCommand { #[cfg(not(feature = "codegen-docs"))] #[tracing::instrument(level = "trace")] pub(crate) fn start_cli() -> eyre::Result<()> { + use std::fs::File; + + use fs2::FileExt as _; + use nargo_toml::get_package_manifest; + let NargoCli { command, mut config } = NargoCli::parse(); // If the provided `program_dir` is relative, make it absolute by joining it to the current directory. @@ -130,6 +135,28 @@ pub(crate) fn start_cli() -> eyre::Result<()> { config.program_dir = program_dir; } + let lock_file = match needs_lock(&command) { + Some(exclusive) => { + let toml_path = get_package_manifest(&config.program_dir)?; + let file = File::open(toml_path).expect("Expected Nargo.toml to exist"); + if exclusive { + if file.try_lock_exclusive().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); + } + + file.lock_exclusive().expect("Failed to lock Nargo.toml"); + } else { + if file.try_lock_shared().is_err() { + eprintln!("Waiting for lock on Nargo.toml..."); + } + + file.lock_shared().expect("Failed to lock Nargo.toml"); + } + Some(file) + } + None => None, + }; + match command { NargoCommand::New(args) => new_cmd::run(args, config), NargoCommand::Init(args) => init_cmd::run(args, config), @@ -146,6 +173,10 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; + if let Some(lock_file) = lock_file { + lock_file.unlock().expect("Failed to unlock Nargo.toml"); + } + Ok(()) } @@ -193,6 +224,23 @@ fn command_dir(cmd: &NargoCommand, program_dir: &Path) -> Result Ok(Some(nargo_toml::find_root(program_dir, workspace)?)) } +fn needs_lock(cmd: &NargoCommand) -> Option { + match cmd { + NargoCommand::Check(..) + | NargoCommand::Compile(..) + | NargoCommand::Execute(..) + | NargoCommand::Export(..) + | NargoCommand::Info(..) => Some(true), + NargoCommand::Debug(..) | NargoCommand::Test(..) => Some(false), + NargoCommand::Fmt(..) + | NargoCommand::New(..) + | NargoCommand::Init(..) + | NargoCommand::Lsp(..) + | NargoCommand::Dap(..) + | NargoCommand::GenerateCompletionScript(..) => None, + } +} + #[cfg(test)] mod tests { use clap::Parser; From da18a12e32e60fb2301e747fd24505fb46d679d7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 6 Jan 2025 14:24:32 -0300 Subject: [PATCH 2/3] feat!: turn CannotReexportItemWithLessVisibility into an error (#6952) --- compiler/noirc_frontend/src/hir/def_collector/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index cafbc670e32..1582e297144 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -183,7 +183,7 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { } DefCollectorErrorKind::PathResolutionError(error) => error.into(), DefCollectorErrorKind::CannotReexportItemWithLessVisibility{item_name, desired_visibility} => { - Diagnostic::simple_warning( + Diagnostic::simple_error( format!("cannot re-export {item_name} because it has less visibility than this use statement"), format!("consider marking {item_name} as {desired_visibility}"), item_name.span()) From 2e5d91f3d69ce629d07eb6041736b5a8e8ed2013 Mon Sep 17 00:00:00 2001 From: jfecher Date: Mon, 6 Jan 2025 12:55:26 -0600 Subject: [PATCH 3/3] chore: Separate unconstrained functions during monomorphization (#6894) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- Cargo.lock | 1 + compiler/noirc_driver/src/lib.rs | 12 +- compiler/noirc_evaluator/src/ssa.rs | 29 +- .../src/ssa/function_builder/mod.rs | 44 ++- .../noirc_evaluator/src/ssa/ir/printer.rs | 5 +- .../src/ssa/opt/constant_folding.rs | 4 +- compiler/noirc_evaluator/src/ssa/opt/die.rs | 17 +- compiler/noirc_evaluator/src/ssa/opt/hint.rs | 1 - compiler/noirc_evaluator/src/ssa/opt/mod.rs | 2 +- .../src/ssa/opt/normalize_value_ids.rs | 4 +- .../src/ssa/opt/remove_unreachable.rs | 80 ++++ .../src/ssa/opt/runtime_separation.rs | 351 ------------------ .../noirc_evaluator/src/ssa/parser/tests.rs | 4 +- .../src/ssa/ssa_gen/context.rs | 11 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 24 +- .../src/ssa/ssa_gen/program.rs | 31 +- compiler/noirc_frontend/Cargo.toml | 1 + .../src/monomorphization/mod.rs | 110 ++++-- compiler/noirc_frontend/src/tests.rs | 2 +- 19 files changed, 240 insertions(+), 493 deletions(-) create mode 100644 compiler/noirc_evaluator/src/ssa/opt/remove_unreachable.rs delete mode 100644 compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs diff --git a/Cargo.lock b/Cargo.lock index 050c70a0f0b..889c5c1ec6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3359,6 +3359,7 @@ dependencies = [ "bn254_blackbox_solver", "cfg-if", "fm", + "fxhash", "im", "iter-extended", "noirc_arena", diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 1b311504b5c..6b852354824 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -590,10 +590,17 @@ pub fn compile_no_check( cached_program: Option, force_compile: bool, ) -> Result { + let force_unconstrained = options.force_brillig; + let program = if options.instrument_debug { - monomorphize_debug(main_function, &mut context.def_interner, &context.debug_instrumenter)? + monomorphize_debug( + main_function, + &mut context.def_interner, + &context.debug_instrumenter, + force_unconstrained, + )? } else { - monomorphize(main_function, &mut context.def_interner)? + monomorphize(main_function, &mut context.def_interner, force_unconstrained)? }; if options.show_monomorphized { @@ -632,7 +639,6 @@ pub fn compile_no_check( } }, enable_brillig_logging: options.show_brillig, - force_brillig_output: options.force_brillig, print_codegen_timings: options.benchmark_codegen, expression_width: if options.bounded_codegen { options.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH) diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 8d22e8e628d..2500b8685a1 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -56,9 +56,6 @@ pub struct SsaEvaluatorOptions { pub enable_brillig_logging: bool, - /// Force Brillig output (for step debugging) - pub force_brillig_output: bool, - /// Pretty print benchmark times of each code generation pass pub print_codegen_timings: bool, @@ -99,7 +96,6 @@ pub(crate) fn optimize_into_acir( let builder = SsaBuilder::new( program, options.ssa_logging.clone(), - options.force_brillig_output, options.print_codegen_timings, &options.emit_ssa, )?; @@ -154,15 +150,16 @@ pub(crate) fn optimize_into_acir( /// Run all SSA passes. fn optimize_all(builder: SsaBuilder, options: &SsaEvaluatorOptions) -> Result { Ok(builder + .run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions") .run_pass(Ssa::defunctionalize, "Defunctionalization") .run_pass(Ssa::remove_paired_rc, "Removing Paired rc_inc & rc_decs") - .run_pass(Ssa::separate_runtime, "Runtime Separation") .run_pass(Ssa::resolve_is_unconstrained, "Resolving IsUnconstrained") .run_pass(|ssa| ssa.inline_functions(options.inliner_aggressiveness), "Inlining (1st)") // Run mem2reg with the CFG separated into blocks .run_pass(Ssa::mem2reg, "Mem2Reg (1st)") .run_pass(Ssa::simplify_cfg, "Simplifying (1st)") .run_pass(Ssa::as_slice_optimization, "`as_slice` optimization") + .run_pass(Ssa::remove_unreachable_functions, "Removing Unreachable Functions") .try_run_pass( Ssa::evaluate_static_assert_and_assert_constant, "`static_assert` and `assert_constant`", @@ -275,19 +272,12 @@ pub fn create_program( (generated_acirs, generated_brillig, brillig_function_names, error_types), ssa_level_warnings, ) = optimize_into_acir(program, options)?; - if options.force_brillig_output { - assert_eq!( - generated_acirs.len(), - 1, - "Only the main ACIR is expected when forcing Brillig output" - ); - } else { - assert_eq!( - generated_acirs.len(), - func_sigs.len(), - "The generated ACIRs should match the supplied function signatures" - ); - } + + assert_eq!( + generated_acirs.len(), + func_sigs.len(), + "The generated ACIRs should match the supplied function signatures" + ); let error_types = error_types .into_iter() @@ -450,11 +440,10 @@ impl SsaBuilder { fn new( program: Program, ssa_logging: SsaLogging, - force_brillig_runtime: bool, print_codegen_timings: bool, emit_ssa: &Option, ) -> Result { - let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?; + let ssa = ssa_gen::generate_ssa(program)?; if let Some(emit_ssa) = emit_ssa { let mut emit_ssa_dir = emit_ssa.clone(); // We expect the full package artifact path to be passed in here, diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 2be8e571cd3..d08d5339237 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -483,29 +483,33 @@ impl FunctionBuilder { /// /// Returns whether a reference count instruction was issued. fn update_array_reference_count(&mut self, value: ValueId, increment: bool) -> bool { - match self.type_of_value(value) { - Type::Numeric(_) => false, - Type::Function => false, - Type::Reference(element) => { - if element.contains_an_array() { - let reference = value; - let value = self.insert_load(reference, element.as_ref().clone()); - self.update_array_reference_count(value, increment); - true - } else { - false + if self.current_function.runtime().is_brillig() { + match self.type_of_value(value) { + Type::Numeric(_) => false, + Type::Function => false, + Type::Reference(element) => { + if element.contains_an_array() { + let reference = value; + let value = self.insert_load(reference, element.as_ref().clone()); + self.update_array_reference_count(value, increment); + true + } else { + false + } } - } - Type::Array(..) | Type::Slice(..) => { - // If there are nested arrays or slices, we wait until ArrayGet - // is issued to increment the count of that array. - if increment { - self.insert_inc_rc(value); - } else { - self.insert_dec_rc(value); + Type::Array(..) | Type::Slice(..) => { + // If there are nested arrays or slices, we wait until ArrayGet + // is issued to increment the count of that array. + if increment { + self.insert_inc_rc(value); + } else { + self.insert_dec_rc(value); + } + true } - true } + } else { + false } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 322639a03d2..598f7c27eff 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -51,9 +51,8 @@ fn value(function: &Function, id: ValueId) -> String { } Value::Function(id) => id.to_string(), Value::Intrinsic(intrinsic) => intrinsic.to_string(), - Value::Param { .. } | Value::Instruction { .. } | Value::ForeignFunction(_) => { - id.to_string() - } + Value::ForeignFunction(function) => function.clone(), + Value::Param { .. } | Value::Instruction { .. } => id.to_string(), } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index e3b8c6e99aa..3a33accf3da 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1550,7 +1550,7 @@ mod test { fn deduplicates_side_effecting_intrinsics() { let src = " // After EnableSideEffectsIf removal: - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: Field, v1: Field, v2: u1): v4 = call is_unconstrained() -> u1 v7 = call to_be_radix(v0, u32 256) -> [u8; 1] // `a.to_be_radix(256)`; @@ -1567,7 +1567,7 @@ mod test { "; let ssa = Ssa::from_str(src).unwrap(); let expected = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: Field, v1: Field, v2: u1): v4 = call is_unconstrained() -> u1 v7 = call to_be_radix(v0, u32 256) -> [u8; 1] diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index 7b38b764eab..7fdcb4c26c2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -637,10 +637,12 @@ mod test { use std::sync::Arc; use im::vector; + use noirc_frontend::monomorphization::ast::InlineType; use crate::ssa::{ function_builder::FunctionBuilder, ir::{ + function::RuntimeType, map::Id, types::{NumericType, Type}, }, @@ -742,7 +744,7 @@ mod test { #[test] fn keep_paired_rcs_with_array_set() { let src = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [Field; 2]): inc_rc v0 v2 = array_set v0, index u32 0, value u32 0 @@ -759,7 +761,7 @@ mod test { #[test] fn keep_inc_rc_on_borrowed_array_store() { - // acir(inline) fn main f0 { + // brillig(inline) fn main f0 { // b0(): // v1 = make_array [u32 0, u32 0] // v2 = allocate @@ -776,6 +778,7 @@ mod test { // Compiling main let mut builder = FunctionBuilder::new("main".into(), main_id); + builder.set_runtime(RuntimeType::Brillig(InlineType::Inline)); let zero = builder.numeric_constant(0u128, NumericType::unsigned(32)); let array_type = Type::Array(Arc::new(vec![Type::unsigned(32)]), 2); let v1 = builder.insert_make_array(vector![zero, zero], array_type.clone()); @@ -810,7 +813,7 @@ mod test { #[test] fn keep_inc_rc_on_borrowed_array_set() { - // acir(inline) fn main f0 { + // brillig(inline) fn main f0 { // b0(v0: [u32; 2]): // inc_rc v0 // v3 = array_set v0, index u32 0, value u32 1 @@ -821,7 +824,7 @@ mod test { // return v4 // } let src = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [u32; 2]): inc_rc v0 v3 = array_set v0, index u32 0, value u32 1 @@ -837,7 +840,7 @@ mod test { // We expect the output to be unchanged // Except for the repeated inc_rc instructions let expected = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [u32; 2]): inc_rc v0 v3 = array_set v0, index u32 0, value u32 1 @@ -875,7 +878,7 @@ mod test { #[test] fn remove_inc_rcs_that_are_never_mutably_borrowed() { let src = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [Field; 2]): inc_rc v0 inc_rc v0 @@ -893,7 +896,7 @@ mod test { assert_eq!(main.dfg[main.entry_block()].instructions().len(), 5); let expected = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [Field; 2]): v2 = array_get v0, index u32 0 -> Field return v2 diff --git a/compiler/noirc_evaluator/src/ssa/opt/hint.rs b/compiler/noirc_evaluator/src/ssa/opt/hint.rs index 567a0795edc..1326c2cc010 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/hint.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/hint.rs @@ -14,7 +14,6 @@ mod tests { let options = &SsaEvaluatorOptions { ssa_logging: SsaLogging::None, enable_brillig_logging: false, - force_brillig_output: false, print_codegen_timings: false, expression_width: ExpressionWidth::default(), emit_ssa: None, diff --git a/compiler/noirc_evaluator/src/ssa/opt/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/mod.rs index bd0c86570e2..58dbc25e869 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mod.rs @@ -20,8 +20,8 @@ mod rc; mod remove_bit_shifts; mod remove_enable_side_effects; mod remove_if_else; +mod remove_unreachable; mod resolve_is_unconstrained; -mod runtime_separation; mod simplify_cfg; mod unrolling; diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index 7f5352155de..56f69a912d4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -181,7 +181,9 @@ impl IdMaps { } Value::Function(id) => { - let new_id = self.function_ids[id]; + let new_id = *self.function_ids.get(id).unwrap_or_else(|| { + unreachable!("Unmapped function with id {id}") + }); new_function.dfg.import_function(new_id) } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable.rs new file mode 100644 index 00000000000..41023b5f376 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable.rs @@ -0,0 +1,80 @@ +use std::collections::BTreeSet; + +use fxhash::FxHashSet as HashSet; + +use crate::ssa::{ + ir::{ + function::{Function, FunctionId}, + instruction::Instruction, + value::Value, + }, + ssa_gen::Ssa, +}; + +impl Ssa { + /// Removes any unreachable functions from the code. These can result from + /// optimizations making existing functions unreachable, e.g. `if false { foo() }`, + /// or even from monomorphizing an unconstrained version of a constrained function + /// where the original constrained version ends up never being used. + pub(crate) fn remove_unreachable_functions(mut self) -> Self { + let mut used_functions = HashSet::default(); + + for function_id in self.functions.keys() { + if self.is_entry_point(*function_id) { + collect_reachable_functions(&self, *function_id, &mut used_functions); + } + } + + self.functions.retain(|id, _| used_functions.contains(id)); + self + } +} + +fn collect_reachable_functions( + ssa: &Ssa, + current_func_id: FunctionId, + reachable_functions: &mut HashSet, +) { + if reachable_functions.contains(¤t_func_id) { + return; + } + reachable_functions.insert(current_func_id); + + // If the debugger is used, its possible for function inlining + // to remove functions that the debugger still references + let Some(func) = ssa.functions.get(¤t_func_id) else { + return; + }; + + let used_functions = used_functions(func); + + for called_func_id in used_functions.iter() { + collect_reachable_functions(ssa, *called_func_id, reachable_functions); + } +} + +fn used_functions(func: &Function) -> BTreeSet { + let mut used_function_ids = BTreeSet::default(); + + let mut find_functions = |value| { + if let Value::Function(function) = func.dfg[func.dfg.resolve(value)] { + used_function_ids.insert(function); + } + }; + + for block_id in func.reachable_blocks() { + let block = &func.dfg[block_id]; + + for instruction_id in block.instructions() { + let instruction = &func.dfg[*instruction_id]; + + if matches!(instruction, Instruction::Store { .. } | Instruction::Call { .. }) { + instruction.for_each_value(&mut find_functions); + } + } + + block.unwrap_terminator().for_each_value(&mut find_functions); + } + + used_function_ids +} diff --git a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs deleted file mode 100644 index b671d5011a1..00000000000 --- a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ /dev/null @@ -1,351 +0,0 @@ -use std::collections::BTreeSet; - -use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; - -use crate::ssa::{ - ir::{ - function::{Function, FunctionId, RuntimeType}, - instruction::Instruction, - value::{Value, ValueId}, - }, - ssa_gen::Ssa, -}; - -impl Ssa { - /// This optimization step separates the runtime of the functions in the SSA. - /// After this step, all functions with runtime `Acir` will be converted to Acir and - /// the functions with runtime `Brillig` will be converted to Brillig. - /// It does so by cloning all ACIR functions called from a Brillig context - /// and changing the runtime of the cloned functions to Brillig. - /// This pass needs to run after functions as values have been resolved (defunctionalization). - #[tracing::instrument(level = "trace", skip(self))] - pub(crate) fn separate_runtime(mut self) -> Self { - RuntimeSeparatorContext::separate_runtime(&mut self); - self - } -} - -#[derive(Debug, Default)] -struct RuntimeSeparatorContext { - // Original functions to clone to brillig - acir_functions_called_from_brillig: BTreeSet, - // Tracks the original => cloned version - mapped_functions: HashMap, -} - -impl RuntimeSeparatorContext { - pub(crate) fn separate_runtime(ssa: &mut Ssa) { - let mut runtime_separator = RuntimeSeparatorContext::default(); - - // We first collect all the acir functions called from a brillig context by exploring the SSA recursively - let mut processed_functions = HashSet::default(); - runtime_separator.collect_acir_functions_called_from_brillig( - ssa, - ssa.main_id, - false, - &mut processed_functions, - ); - - // Now we clone the relevant acir functions and change their runtime to brillig - runtime_separator.convert_acir_functions_called_from_brillig_to_brillig(ssa); - - // Now we update any calls within a brillig context to the mapped functions - runtime_separator.replace_calls_to_mapped_functions(ssa); - - // Some functions might be unreachable now (for example an acir function only called from brillig) - prune_unreachable_functions(ssa); - } - - fn collect_acir_functions_called_from_brillig( - &mut self, - ssa: &Ssa, - current_func_id: FunctionId, - mut within_brillig: bool, - processed_functions: &mut HashSet<(/* within_brillig */ bool, FunctionId)>, - ) { - // Processed functions needs the within brillig flag, since it is possible to call the same function from both brillig and acir - if processed_functions.contains(&(within_brillig, current_func_id)) { - return; - } - processed_functions.insert((within_brillig, current_func_id)); - - let func = &ssa.functions[¤t_func_id]; - if matches!(func.runtime(), RuntimeType::Brillig(_)) { - within_brillig = true; - } - - let called_functions = called_functions(func); - - if within_brillig { - for called_func_id in called_functions.iter() { - let called_func = &ssa.functions[called_func_id]; - if matches!(called_func.runtime(), RuntimeType::Acir(_)) { - self.acir_functions_called_from_brillig.insert(*called_func_id); - } - } - } - - for called_func_id in called_functions.into_iter() { - self.collect_acir_functions_called_from_brillig( - ssa, - called_func_id, - within_brillig, - processed_functions, - ); - } - } - - fn convert_acir_functions_called_from_brillig_to_brillig(&mut self, ssa: &mut Ssa) { - for acir_func_id in self.acir_functions_called_from_brillig.iter() { - let RuntimeType::Acir(inline_type) = ssa.functions[acir_func_id].runtime() else { - unreachable!("Function to transform to brillig should be ACIR") - }; - let cloned_id = ssa.clone_fn(*acir_func_id); - let new_func = - ssa.functions.get_mut(&cloned_id).expect("Cloned function should exist in SSA"); - new_func.set_runtime(RuntimeType::Brillig(inline_type)); - self.mapped_functions.insert(*acir_func_id, cloned_id); - } - } - - fn replace_calls_to_mapped_functions(&self, ssa: &mut Ssa) { - for (_function_id, func) in ssa.functions.iter_mut() { - if matches!(func.runtime(), RuntimeType::Brillig(_)) { - for called_func_value_id in called_functions_values(func).iter() { - let Value::Function(called_func_id) = &func.dfg[*called_func_value_id] else { - unreachable!("Value should be a function") - }; - if let Some(mapped_func_id) = self.mapped_functions.get(called_func_id) { - let mapped_value_id = func.dfg.import_function(*mapped_func_id); - func.dfg.set_value_from_id(*called_func_value_id, mapped_value_id); - } - } - } - } - } -} - -// We only consider direct calls to functions since functions as values should have been resolved -fn called_functions_values(func: &Function) -> BTreeSet { - let mut called_function_ids = BTreeSet::default(); - for block_id in func.reachable_blocks() { - for instruction_id in func.dfg[block_id].instructions() { - let Instruction::Call { func: called_value_id, .. } = &func.dfg[*instruction_id] else { - continue; - }; - - if let Value::Function(_) = func.dfg[*called_value_id] { - called_function_ids.insert(*called_value_id); - } - } - } - - called_function_ids -} - -fn called_functions(func: &Function) -> BTreeSet { - called_functions_values(func) - .into_iter() - .map(|value_id| { - let Value::Function(func_id) = func.dfg[value_id] else { - unreachable!("Value should be a function") - }; - func_id - }) - .collect() -} - -fn collect_reachable_functions( - ssa: &Ssa, - current_func_id: FunctionId, - reachable_functions: &mut HashSet, -) { - if reachable_functions.contains(¤t_func_id) { - return; - } - reachable_functions.insert(current_func_id); - - let func = &ssa.functions[¤t_func_id]; - let called_functions = called_functions(func); - - for called_func_id in called_functions.iter() { - collect_reachable_functions(ssa, *called_func_id, reachable_functions); - } -} - -fn prune_unreachable_functions(ssa: &mut Ssa) { - let mut reachable_functions = HashSet::default(); - collect_reachable_functions(ssa, ssa.main_id, &mut reachable_functions); - - ssa.functions.retain(|id, _value| reachable_functions.contains(id)); -} - -#[cfg(test)] -mod test { - use std::collections::BTreeSet; - - use noirc_frontend::monomorphization::ast::InlineType; - - use crate::ssa::{ - function_builder::FunctionBuilder, - ir::{ - function::{Function, FunctionId, RuntimeType}, - map::Id, - types::Type, - }, - opt::runtime_separation::called_functions, - ssa_gen::Ssa, - }; - - #[test] - fn basic_runtime_separation() { - // brillig fn foo { - // b0(): - // v0 = call bar() - // return v0 - // } - // acir fn bar { - // b0(): - // return 72 - // } - let foo_id = Id::test_new(0); - let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.current_function.set_runtime(RuntimeType::Brillig(InlineType::default())); - - let bar_id = Id::test_new(1); - let bar = builder.import_function(bar_id); - let results = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec(); - builder.terminate_with_return(results); - - builder.new_function("bar".into(), bar_id, InlineType::default()); - let expected_return = 72u128; - let seventy_two = builder.field_constant(expected_return); - builder.terminate_with_return(vec![seventy_two]); - - let ssa = builder.finish(); - assert_eq!(ssa.functions.len(), 2); - - // Expected result - // brillig fn foo { - // b0(): - // v0 = call bar() - // return v0 - // } - // brillig fn bar { - // b0(): - // return 72 - // } - let separated = ssa.separate_runtime(); - - // The original bar function must have been pruned - assert_eq!(separated.functions.len(), 2); - - // All functions should be brillig now - for func in separated.functions.values() { - assert_eq!(func.runtime(), RuntimeType::Brillig(InlineType::default())); - } - } - - fn find_func_by_name<'ssa>( - ssa: &'ssa Ssa, - funcs: &BTreeSet, - name: &str, - ) -> &'ssa Function { - funcs - .iter() - .find_map(|id| { - let func = ssa.functions.get(id).unwrap(); - if func.name() == name { - Some(func) - } else { - None - } - }) - .unwrap() - } - - #[test] - fn same_function_shared_acir_brillig() { - // acir fn foo { - // b0(): - // v0 = call bar() - // v1 = call baz() - // return v0, v1 - // } - // brillig fn bar { - // b0(): - // v0 = call baz() - // return v0 - // } - // acir fn baz { - // b0(): - // return 72 - // } - let foo_id = Id::test_new(0); - let mut builder = FunctionBuilder::new("foo".into(), foo_id); - - let bar_id = Id::test_new(1); - let baz_id = Id::test_new(2); - let bar = builder.import_function(bar_id); - let baz = builder.import_function(baz_id); - let v0 = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec(); - let v1 = builder.insert_call(baz, Vec::new(), vec![Type::field()]).to_vec(); - builder.terminate_with_return(vec![v0[0], v1[0]]); - - builder.new_brillig_function("bar".into(), bar_id, InlineType::default()); - let baz = builder.import_function(baz_id); - let v0 = builder.insert_call(baz, Vec::new(), vec![Type::field()]).to_vec(); - builder.terminate_with_return(v0); - - builder.new_function("baz".into(), baz_id, InlineType::default()); - let expected_return = 72u128; - let seventy_two = builder.field_constant(expected_return); - builder.terminate_with_return(vec![seventy_two]); - - let ssa = builder.finish(); - assert_eq!(ssa.functions.len(), 3); - - // Expected result - // acir fn foo { - // b0(): - // v0 = call bar() - // v1 = call baz() <- baz_acir - // return v0, v1 - // } - // brillig fn bar { - // b0(): - // v0 = call baz() <- baz_brillig - // return v0 - // } - // acir fn baz { - // b0(): - // return 72 - // } - // brillig fn baz { - // b0(): - // return 72 - // } - let separated = ssa.separate_runtime(); - - // The original baz function must have been duplicated - assert_eq!(separated.functions.len(), 4); - - let main_function = separated.functions.get(&separated.main_id).unwrap(); - assert_eq!(main_function.runtime(), RuntimeType::Acir(InlineType::Inline)); - - let main_calls = called_functions(main_function); - assert_eq!(main_calls.len(), 2); - - let bar = find_func_by_name(&separated, &main_calls, "bar"); - let baz_acir = find_func_by_name(&separated, &main_calls, "baz"); - - assert_eq!(baz_acir.runtime(), RuntimeType::Acir(InlineType::Inline)); - assert_eq!(bar.runtime(), RuntimeType::Brillig(InlineType::default())); - - let bar_calls = called_functions(bar); - assert_eq!(bar_calls.len(), 1); - - let baz_brillig = find_func_by_name(&separated, &bar_calls, "baz"); - assert_eq!(baz_brillig.runtime(), RuntimeType::Brillig(InlineType::default())); - } -} diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 899a5a571e7..b5aac13cfd8 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -415,7 +415,7 @@ fn test_store() { #[test] fn test_inc_rc() { let src = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [Field; 3]): inc_rc v0 return @@ -427,7 +427,7 @@ fn test_inc_rc() { #[test] fn test_dec_rc() { let src = " - acir(inline) fn main f0 { + brillig(inline) fn main f0 { b0(v0: [Field; 3]): dec_rc v0 return diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 7807658dabb..e89d1d2a0c3 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -4,7 +4,7 @@ use acvm::{acir::AcirField, FieldElement}; use iter_extended::vecmap; use noirc_errors::Location; use noirc_frontend::ast::{BinaryOpKind, Signedness}; -use noirc_frontend::monomorphization::ast::{self, InlineType, LocalId, Parameters}; +use noirc_frontend::monomorphization::ast::{self, LocalId, Parameters}; use noirc_frontend::monomorphization::ast::{FuncId, Program}; use crate::errors::RuntimeError; @@ -119,14 +119,9 @@ impl<'a> FunctionContext<'a> { /// /// Note that the previous function cannot be resumed after calling this. Developers should /// avoid calling new_function until the previous function is completely finished with ssa-gen. - pub(super) fn new_function( - &mut self, - id: IrFunctionId, - func: &ast::Function, - force_brillig_runtime: bool, - ) { + pub(super) fn new_function(&mut self, id: IrFunctionId, func: &ast::Function) { self.definitions.clear(); - if func.unconstrained || (force_brillig_runtime && func.inline_type != InlineType::Inline) { + if func.unconstrained { self.builder.new_brillig_function(func.name.clone(), id, func.inline_type); } else { self.builder.new_function(func.name.clone(), id, func.inline_type); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index d3821158b80..b3a8ecf1314 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -39,10 +39,7 @@ pub(crate) const SSA_WORD_SIZE: u32 = 32; /// Generates SSA for the given monomorphized program. /// /// This function will generate the SSA but does not perform any optimizations on it. -pub(crate) fn generate_ssa( - program: Program, - force_brillig_runtime: bool, -) -> Result { +pub(crate) fn generate_ssa(program: Program) -> Result { // see which parameter has call_data/return_data attribute let is_databus = DataBusBuilder::is_databus(&program.main_function_signature); @@ -56,16 +53,13 @@ pub(crate) fn generate_ssa( // Queue the main function for compilation context.get_or_queue_function(main_id); - let mut function_context = FunctionContext::new( - main.name.clone(), - &main.parameters, - if force_brillig_runtime || main.unconstrained { - RuntimeType::Brillig(main.inline_type) - } else { - RuntimeType::Acir(main.inline_type) - }, - &context, - ); + let main_runtime = if main.unconstrained { + RuntimeType::Brillig(main.inline_type) + } else { + RuntimeType::Acir(main.inline_type) + }; + let mut function_context = + FunctionContext::new(main.name.clone(), &main.parameters, main_runtime, &context); // Generate the call_data bus from the relevant parameters. We create it *before* processing the function body let call_data = function_context.builder.call_data_bus(is_databus); @@ -122,7 +116,7 @@ pub(crate) fn generate_ssa( // to generate SSA for each function used within the program. while let Some((src_function_id, dest_id)) = context.pop_next_function_in_queue() { let function = &context.program[src_function_id]; - function_context.new_function(dest_id, function, force_brillig_runtime); + function_context.new_function(dest_id, function); function_context.codegen_function_body(&function.body)?; } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 12892efa598..98cdc0adad9 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use serde_with::serde_as; use crate::ssa::ir::{ - function::{Function, FunctionId, RuntimeType}, + function::{Function, FunctionId}, map::AtomicCounter, }; use noirc_frontend::hir_def::types::Type as HirType; @@ -78,29 +78,10 @@ impl Ssa { new_id } - /// Clones an already existing function with a fresh id - pub(crate) fn clone_fn(&mut self, existing_function_id: FunctionId) -> FunctionId { - let new_id = self.next_id.next(); - let function = Function::clone_with_id(new_id, &self.functions[&existing_function_id]); - self.functions.insert(new_id, function); - new_id - } pub(crate) fn generate_entry_point_index(mut self) -> Self { - self.entry_point_to_generated_index = btree_map( - self.functions - .iter() - .filter(|(_, func)| { - let runtime = func.runtime(); - match func.runtime() { - RuntimeType::Acir(_) => { - runtime.is_entry_point() || func.id() == self.main_id - } - RuntimeType::Brillig(_) => false, - } - }) - .enumerate(), - |(i, (id, _))| (*id, i as u32), - ); + let entry_points = + self.functions.keys().filter(|function| self.is_entry_point(**function)).enumerate(); + self.entry_point_to_generated_index = btree_map(entry_points, |(i, id)| (*id, i as u32)); self } @@ -112,6 +93,10 @@ impl Ssa { ); self.entry_point_to_generated_index.get(func_id).copied() } + + pub(crate) fn is_entry_point(&self, function: FunctionId) -> bool { + function == self.main_id || self.functions[&function].runtime().is_entry_point() + } } impl Display for Ssa { diff --git a/compiler/noirc_frontend/Cargo.toml b/compiler/noirc_frontend/Cargo.toml index 5f8f02689c8..041c1b1e015 100644 --- a/compiler/noirc_frontend/Cargo.toml +++ b/compiler/noirc_frontend/Cargo.toml @@ -31,6 +31,7 @@ petgraph = "0.6" rangemap = "1.4.0" strum.workspace = true strum_macros.workspace = true +fxhash.workspace = true [dev-dependencies] diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 8c07d71de21..fbc80cc0ac9 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -25,11 +25,12 @@ use crate::{ Kind, Type, TypeBinding, TypeBindings, }; use acvm::{acir::AcirField, FieldElement}; +use fxhash::FxHashMap as HashMap; use iter_extended::{btree_map, try_vecmap, vecmap}; use noirc_errors::Location; use noirc_printable_type::PrintableType; use std::{ - collections::{BTreeMap, HashMap, VecDeque}, + collections::{BTreeMap, VecDeque}, unreachable, }; @@ -56,14 +57,12 @@ struct LambdaContext { /// This struct holds the FIFO queue of functions to monomorphize, which is added to /// whenever a new (function, type) combination is encountered. struct Monomorphizer<'interner> { - /// Functions are keyed by their unique ID and expected type so that we can monomorphize - /// a new version of the function for each type. - /// We also key by any turbofish generics that are specified. - /// This is necessary for a case where we may have a trait generic that can be instantiated - /// outside of a function parameter or return value. + /// Functions are keyed by their unique ID, whether they're unconstrained, their expected type, + /// and any generics they have so that we can monomorphize a new version of the function for each type. /// - /// Using nested HashMaps here lets us avoid cloning HirTypes when calling .get() - functions: HashMap), FuncId>>, + /// Keying by any turbofish generics that are specified is necessary for a case where we may have a + /// trait generic that can be instantiated outside of a function parameter or return value. + functions: Functions, /// Unlike functions, locals are only keyed by their unique ID because they are never /// duplicated during monomorphization. Doing so would allow them to be used polymorphically @@ -72,8 +71,15 @@ struct Monomorphizer<'interner> { locals: HashMap, /// Queue of functions to monomorphize next each item in the queue is a tuple of: - /// (old_id, new_monomorphized_id, any type bindings to apply, the trait method if old_id is from a trait impl) - queue: VecDeque<(node_interner::FuncId, FuncId, TypeBindings, Option, Location)>, + /// (old_id, new_monomorphized_id, any type bindings to apply, the trait method if old_id is from a trait impl, is_unconstrained, location) + queue: VecDeque<( + node_interner::FuncId, + FuncId, + TypeBindings, + Option, + bool, + Location, + )>, /// When a function finishes being monomorphized, the monomorphized ast::Function is /// stored here along with its FuncId. @@ -92,8 +98,16 @@ struct Monomorphizer<'interner> { return_location: Option, debug_type_tracker: DebugTypeTracker, + + in_unconstrained_function: bool, } +/// Using nested HashMaps here lets us avoid cloning HirTypes when calling .get() +type Functions = HashMap< + (node_interner::FuncId, /*is_unconstrained:*/ bool), + HashMap, FuncId>>, +>; + type HirType = crate::Type; /// Starting from the given `main` function, monomorphize the entire program, @@ -111,24 +125,29 @@ type HirType = crate::Type; pub fn monomorphize( main: node_interner::FuncId, interner: &mut NodeInterner, + force_unconstrained: bool, ) -> Result { - monomorphize_debug(main, interner, &DebugInstrumenter::default()) + monomorphize_debug(main, interner, &DebugInstrumenter::default(), force_unconstrained) } pub fn monomorphize_debug( main: node_interner::FuncId, interner: &mut NodeInterner, debug_instrumenter: &DebugInstrumenter, + force_unconstrained: bool, ) -> Result { let debug_type_tracker = DebugTypeTracker::build_from_debug_instrumenter(debug_instrumenter); let mut monomorphizer = Monomorphizer::new(interner, debug_type_tracker); + monomorphizer.in_unconstrained_function = force_unconstrained; let function_sig = monomorphizer.compile_main(main)?; while !monomorphizer.queue.is_empty() { - let (next_fn_id, new_id, bindings, trait_method, location) = + let (next_fn_id, new_id, bindings, trait_method, is_unconstrained, location) = monomorphizer.queue.pop_front().unwrap(); monomorphizer.locals.clear(); + monomorphizer.in_unconstrained_function = is_unconstrained; + perform_instantiation_bindings(&bindings); let interner = &monomorphizer.interner; let impl_bindings = perform_impl_bindings(interner, trait_method, next_fn_id, location) @@ -143,7 +162,9 @@ pub fn monomorphize_debug( .finished_functions .iter() .flat_map(|(_, f)| { - if f.inline_type.is_entry_point() || f.id == Program::main_id() { + if (!force_unconstrained && f.inline_type.is_entry_point()) + || f.id == Program::main_id() + { Some(f.func_sig.clone()) } else { None @@ -172,8 +193,8 @@ pub fn monomorphize_debug( impl<'interner> Monomorphizer<'interner> { fn new(interner: &'interner mut NodeInterner, debug_type_tracker: DebugTypeTracker) -> Self { Monomorphizer { - functions: HashMap::new(), - locals: HashMap::new(), + functions: HashMap::default(), + locals: HashMap::default(), queue: VecDeque::new(), finished_functions: BTreeMap::new(), next_local_id: 0, @@ -183,6 +204,7 @@ impl<'interner> Monomorphizer<'interner> { is_range_loop: false, return_location: None, debug_type_tracker, + in_unconstrained_function: false, } } @@ -207,14 +229,18 @@ impl<'interner> Monomorphizer<'interner> { id: node_interner::FuncId, expr_id: node_interner::ExprId, typ: &HirType, - turbofish_generics: Vec, + turbofish_generics: &[HirType], trait_method: Option, ) -> Definition { let typ = typ.follow_bindings(); + let turbofish_generics = vecmap(turbofish_generics, |typ| typ.follow_bindings()); + let is_unconstrained = self.is_unconstrained(id); + match self .functions - .get(&id) - .and_then(|inner_map| inner_map.get(&(typ.clone(), turbofish_generics.clone()))) + .get(&(id, is_unconstrained)) + .and_then(|inner_map| inner_map.get(&typ)) + .and_then(|inner_map| inner_map.get(&turbofish_generics)) { Some(id) => Definition::Function(*id), None => { @@ -257,14 +283,21 @@ impl<'interner> Monomorphizer<'interner> { } /// Prerequisite: typ = typ.follow_bindings() + /// and: turbofish_generics = vecmap(turbofish_generics, Type::follow_bindings) fn define_function( &mut self, id: node_interner::FuncId, typ: HirType, turbofish_generics: Vec, + is_unconstrained: bool, new_id: FuncId, ) { - self.functions.entry(id).or_default().insert((typ, turbofish_generics), new_id); + self.functions + .entry((id, is_unconstrained)) + .or_default() + .entry(typ) + .or_default() + .insert(turbofish_generics, new_id); } fn compile_main( @@ -275,6 +308,7 @@ impl<'interner> Monomorphizer<'interner> { assert_eq!(new_main_id, Program::main_id()); let location = self.interner.function_meta(&main_id).location; + self.in_unconstrained_function = self.is_unconstrained(main_id); self.function(main_id, new_main_id, location)?; self.return_location = @@ -324,7 +358,7 @@ impl<'interner> Monomorphizer<'interner> { }; let return_type = Self::convert_type(return_type, meta.location)?; - let unconstrained = modifiers.is_unconstrained; + let unconstrained = self.in_unconstrained_function; let attributes = self.interner.function_attributes(&f); let inline_type = InlineType::from(attributes); @@ -874,7 +908,7 @@ impl<'interner> Monomorphizer<'interner> { *func_id, expr_id, &typ, - generics.unwrap_or_default(), + &generics.unwrap_or_default(), None, ); let typ = Self::convert_type(&typ, ident.location)?; @@ -1281,7 +1315,7 @@ impl<'interner> Monomorphizer<'interner> { .map_err(MonomorphizationError::InterpreterError)?; let func_id = - match self.lookup_function(func_id, expr_id, &function_type, vec![], Some(method)) { + match self.lookup_function(func_id, expr_id, &function_type, &[], Some(method)) { Definition::Function(func_id) => func_id, _ => unreachable!(), }; @@ -1546,12 +1580,19 @@ impl<'interner> Monomorphizer<'interner> { trait_method: Option, ) -> FuncId { let new_id = self.next_function_id(); - self.define_function(id, function_type.clone(), turbofish_generics, new_id); + let is_unconstrained = self.is_unconstrained(id); + self.define_function( + id, + function_type.clone(), + turbofish_generics, + is_unconstrained, + new_id, + ); let location = self.interner.expr_location(&expr_id); let bindings = self.interner.get_instantiation_bindings(expr_id); let bindings = self.follow_bindings(bindings); - self.queue.push_back((id, new_id, bindings, trait_method, location)); + self.queue.push_back((id, new_id, bindings, trait_method, is_unconstrained, location)); new_id } @@ -1638,19 +1679,15 @@ impl<'interner> Monomorphizer<'interner> { let parameters = self.parameters(¶meters)?; let body = self.expr(lambda.body)?; - let id = self.next_function_id(); - let return_type = ret_type.clone(); - let name = lambda_name.to_owned(); - let unconstrained = false; let function = ast::Function { id, - name, + name: lambda_name.to_owned(), parameters, body, - return_type, - unconstrained, + return_type: ret_type.clone(), + unconstrained: self.in_unconstrained_function, inline_type: InlineType::default(), func_sig: FunctionSignature::default(), }; @@ -1773,18 +1810,16 @@ impl<'interner> Monomorphizer<'interner> { typ: lambda_fn_typ.clone(), }); - let mut parameters = vec![]; - parameters.push((env_local_id, true, env_name.to_string(), env_typ.clone())); + let mut parameters = vec![(env_local_id, true, env_name.to_string(), env_typ.clone())]; parameters.append(&mut converted_parameters); - let unconstrained = false; let function = ast::Function { id, name, parameters, body, return_type, - unconstrained, + unconstrained: self.in_unconstrained_function, inline_type: InlineType::default(), func_sig: FunctionSignature::default(), }; @@ -2007,6 +2042,11 @@ impl<'interner> Monomorphizer<'interner> { Ok(ast::Expression::Call(ast::Call { func, arguments, return_type, location })) } + + fn is_unconstrained(&self, func_id: node_interner::FuncId) -> bool { + self.in_unconstrained_function + || self.interner.function_modifiers(&func_id).is_unconstrained + } } fn unwrap_tuple_type(typ: &HirType) -> Vec { diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 3d908d1aa0c..3377c260922 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -1244,7 +1244,7 @@ fn resolve_fmt_strings() { fn monomorphize_program(src: &str) -> Result { let (_program, mut context, _errors) = get_program(src); let main_func_id = context.def_interner.find_function("main").unwrap(); - monomorphize(main_func_id, &mut context.def_interner) + monomorphize(main_func_id, &mut context.def_interner, false) } fn get_monomorphization_error(src: &str) -> Option {