diff --git a/.github/workflows/gates_report.yml b/.github/workflows/gates_report.yml index f3f798fc5ea..ebf17f7374c 100644 --- a/.github/workflows/gates_report.yml +++ b/.github/workflows/gates_report.yml @@ -74,7 +74,7 @@ jobs: - name: Compare gates reports id: gates_diff - uses: TomAFrench/noir-gates-diff@e7cf131b7e7f044c01615f93f0b855f65ddc02d4 + uses: TomAFrench/noir-gates-diff@df05f34e2ab275ddc4f2cac065df1c88f8a05e5d with: report: gates_report.json summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%) diff --git a/compiler/noirc_driver/src/contract.rs b/compiler/noirc_driver/src/contract.rs index 9a0e25a321b..bd78cb23cdc 100644 --- a/compiler/noirc_driver/src/contract.rs +++ b/compiler/noirc_driver/src/contract.rs @@ -51,4 +51,7 @@ pub struct ContractFunction { pub bytecode: Program, pub debug: DebugInfo, + + /// Names of the functions in the program. These are used for more informative debugging and benchmarking. + pub names: Vec, } diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 86d4cd1510d..8fa2d9680c8 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -11,6 +11,7 @@ use noirc_abi::{AbiParameter, AbiType, ContractEvent}; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; use noirc_evaluator::create_program; use noirc_evaluator::errors::RuntimeError; +use noirc_evaluator::ssa::SsaProgramArtifact; use noirc_frontend::debug::build_debug_crate_file; use noirc_frontend::graph::{CrateId, CrateName}; use noirc_frontend::hir::def_map::{Contract, CrateDefMap}; @@ -423,6 +424,7 @@ fn compile_contract_inner( bytecode: function.program, debug: function.debug, is_unconstrained: modifiers.is_unconstrained, + names: function.names, }); } @@ -485,7 +487,14 @@ pub fn compile_no_check( } let visibility = program.return_visibility; - let (program, debug, warnings, input_witnesses, return_witnesses) = create_program( + let SsaProgramArtifact { + program, + debug, + warnings, + main_input_witnesses, + main_return_witnesses, + names, + } = create_program( program, options.show_ssa, options.show_brillig, @@ -493,8 +502,13 @@ pub fn compile_no_check( options.benchmark_codegen, )?; - let abi = - abi_gen::gen_abi(context, &main_function, input_witnesses, return_witnesses, visibility); + let abi = abi_gen::gen_abi( + context, + &main_function, + main_input_witnesses, + main_return_witnesses, + visibility, + ); let file_map = filter_relevant_files(&debug, &context.file_manager); Ok(CompiledProgram { @@ -510,5 +524,6 @@ pub fn compile_no_check( file_map, noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(), warnings, + names, }) } diff --git a/compiler/noirc_driver/src/program.rs b/compiler/noirc_driver/src/program.rs index 6f527297dcb..9ffd2d70dda 100644 --- a/compiler/noirc_driver/src/program.rs +++ b/compiler/noirc_driver/src/program.rs @@ -27,4 +27,6 @@ pub struct CompiledProgram { pub debug: DebugInfo, pub file_map: BTreeMap, pub warnings: Vec, + /// Names of the functions in the program. These are used for more informative debugging and benchmarking. + pub names: Vec, } diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 9ecf6960333..fac7a7c0829 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -88,6 +88,29 @@ fn time(name: &str, print_timings: bool, f: impl FnOnce() -> T) -> T { result } +#[derive(Default)] +pub struct SsaProgramArtifact { + pub program: AcirProgram, + pub debug: Vec, + pub warnings: Vec, + pub main_input_witnesses: Vec, + pub main_return_witnesses: Vec, + pub names: Vec, +} + +impl SsaProgramArtifact { + fn add_circuit(&mut self, mut circuit_artifact: SsaCircuitArtifact, is_main: bool) { + self.program.functions.push(circuit_artifact.circuit); + self.debug.push(circuit_artifact.debug_info); + self.warnings.append(&mut circuit_artifact.warnings); + if is_main { + self.main_input_witnesses = circuit_artifact.input_witnesses; + self.main_return_witnesses = circuit_artifact.return_witnesses; + } + self.names.push(circuit_artifact.name); + } +} + /// Compiles the [`Program`] into [`ACIR``][acvm::acir::circuit::Program]. /// /// The output ACIR is is backend-agnostic and so must go through a transformation pass before usage in proof generation. @@ -99,8 +122,7 @@ pub fn create_program( enable_brillig_logging: bool, force_brillig_output: bool, print_codegen_timings: bool, -) -> Result<(AcirProgram, Vec, Vec, Vec, Vec), RuntimeError> -{ +) -> Result { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); let debug_functions = program.debug_functions.clone(); @@ -121,37 +143,33 @@ pub fn create_program( "The generated ACIRs should match the supplied function signatures" ); - let mut functions = vec![]; - let mut debug_infos = vec![]; - let mut warning_infos = vec![]; - let mut main_input_witnesses = Vec::new(); - let mut main_return_witnesses = Vec::new(); + let mut program_artifact = SsaProgramArtifact::default(); // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) { - let (circuit, debug_info, warnings, input_witnesses, return_witnesses) = - convert_generated_acir_into_circuit( - acir, - func_sig, - recursive, - // TODO: get rid of these clones - debug_variables.clone(), - debug_functions.clone(), - debug_types.clone(), - ); - functions.push(circuit); - debug_infos.push(debug_info); - warning_infos.extend(warnings); - if is_main { - main_input_witnesses = input_witnesses; - main_return_witnesses = return_witnesses; - } + let circuit_artifact = convert_generated_acir_into_circuit( + acir, + func_sig, + recursive, + // TODO: get rid of these clones + debug_variables.clone(), + debug_functions.clone(), + debug_types.clone(), + ); + program_artifact.add_circuit(circuit_artifact, is_main); is_main = false; } - let program = AcirProgram { functions }; + Ok(program_artifact) +} - Ok((program, debug_infos, warning_infos, main_input_witnesses, main_return_witnesses)) +pub struct SsaCircuitArtifact { + name: String, + circuit: Circuit, + debug_info: DebugInfo, + warnings: Vec, + input_witnesses: Vec, + return_witnesses: Vec, } fn convert_generated_acir_into_circuit( @@ -161,7 +179,7 @@ fn convert_generated_acir_into_circuit( debug_variables: DebugVariables, debug_functions: DebugFunctions, debug_types: DebugTypes, -) -> (Circuit, DebugInfo, Vec, Vec, Vec) { +) -> SsaCircuitArtifact { let opcodes = generated_acir.take_opcodes(); let current_witness_index = generated_acir.current_witness_index().0; let GeneratedAcir { @@ -170,6 +188,7 @@ fn convert_generated_acir_into_circuit( input_witnesses, assert_messages, warnings, + name, .. } = generated_acir; @@ -202,7 +221,14 @@ fn convert_generated_acir_into_circuit( let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit); debug_info.update_acir(transformation_map); - (optimized_circuit, debug_info, warnings, input_witnesses, return_witnesses) + SsaCircuitArtifact { + name, + circuit: optimized_circuit, + debug_info, + warnings, + input_witnesses, + return_witnesses, + } } // Takes each function argument and partitions the circuit's inputs witnesses according to its visibility. diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 1d05e998b13..b43110b2f5b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -25,7 +25,7 @@ use iter_extended::vecmap; use num_bigint::BigUint; #[derive(Debug, Default)] -/// The output of the Acir-gen pass +/// The output of the Acir-gen pass, which should only be produced for entry point Acir functions pub(crate) struct GeneratedAcir { /// The next witness index that may be declared. /// If witness index is `None` then we have not yet created a witness @@ -58,6 +58,10 @@ pub(crate) struct GeneratedAcir { pub(crate) assert_messages: BTreeMap, pub(crate) warnings: Vec, + + /// Name for the corresponding entry point represented by this Acir-gen output. + /// Only used for debugging and benchmarking purposes + pub(crate) name: String, } impl GeneratedAcir { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index d4760c29eda..9e597dc8e9a 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -186,7 +186,10 @@ impl Ssa { // TODO: can we parallelise this? for function in self.functions.values() { let context = Context::new(); - if let Some(generated_acir) = context.convert_ssa_function(&self, function, brillig)? { + if let Some(mut generated_acir) = + context.convert_ssa_function(&self, function, brillig)? + { + generated_acir.name = function.name().to_owned(); acirs.push(generated_acir); } } @@ -253,7 +256,7 @@ impl Context { } } } - // We only want to convert entry point functions. This being `main` and those marked with `#[fold]` + // We only want to convert entry point functions. This being `main` and those marked with `InlineType::Fold` Ok(Some(self.convert_acir_main(function, ssa, brillig)?)) } RuntimeType::Brillig => { diff --git a/test_programs/compile_success_contract/fold_non_contract_method/Nargo.toml b/test_programs/compile_success_contract/fold_non_contract_method/Nargo.toml new file mode 100644 index 00000000000..ff64bbb6f1b --- /dev/null +++ b/test_programs/compile_success_contract/fold_non_contract_method/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fold_non_contract_method" +type = "contract" +authors = [""] +compiler_version = ">=0.26.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_success_contract/fold_non_contract_method/src/main.nr b/test_programs/compile_success_contract/fold_non_contract_method/src/main.nr new file mode 100644 index 00000000000..2e00ffa1381 --- /dev/null +++ b/test_programs/compile_success_contract/fold_non_contract_method/src/main.nr @@ -0,0 +1,18 @@ +contract Foo { + use crate::times_10; + + fn double(x: Field) -> pub Field { + x * 2 + } + fn triple(x: Field) -> pub Field { + x * 3 + } + fn times_40(x: Field) -> pub Field { + times_10(x) * 4 + } +} + +#[fold] +fn times_10(x: Field) -> Field { + x * 10 +} diff --git a/tooling/nargo/src/artifacts/program.rs b/tooling/nargo/src/artifacts/program.rs index 046db1cd8fa..9e660cbd359 100644 --- a/tooling/nargo/src/artifacts/program.rs +++ b/tooling/nargo/src/artifacts/program.rs @@ -34,6 +34,8 @@ pub struct ProgramArtifact { /// Map of file Id to the source code so locations in debug info can be mapped to source code they point to. pub file_map: BTreeMap, + + pub names: Vec, } impl From for ProgramArtifact { @@ -45,6 +47,7 @@ impl From for ProgramArtifact { bytecode: compiled_program.program, debug_symbols: compiled_program.debug, file_map: compiled_program.file_map, + names: compiled_program.names, } } } @@ -59,6 +62,7 @@ impl From for CompiledProgram { debug: program.debug_symbols, file_map: program.file_map, warnings: vec![], + names: program.names, } } } diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 16df29e6985..bf97dfb3e96 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -244,7 +244,7 @@ fn compile_success_empty_{test_name}() {{ let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{ panic!("JSON was not well-formatted {{:?}}\n\n{{:?}}", e, std::str::from_utf8(&output.stdout)) }}); - let num_opcodes = &json["programs"][0]["acir_opcodes"]; + let num_opcodes = &json["programs"][0]["functions"][0]["acir_opcodes"]; assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0); }} "#, diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 49924622392..72784013e17 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use acvm::acir::circuit::ExpressionWidth; +use acvm::acir::circuit::{ExpressionWidth, Program}; use backend_interface::BackendError; use clap::Args; use iter_extended::vecmap; @@ -24,9 +24,9 @@ use crate::errors::CliError; use super::{compile_cmd::compile_workspace, NargoConfig}; -/// Provides detailed information on a circuit +/// Provides detailed information on each of a program's function (represented by a single circuit) /// -/// Current information provided: +/// Current information provided per circuit: /// 1. The number of ACIR opcodes /// 2. Counts the final number gates in the circuit used by a backend #[derive(Debug, Clone, Args)] @@ -114,6 +114,7 @@ pub(crate) fn run( let binary_packages = workspace.into_iter().filter(|package| package.is_binary()).zip(compiled_programs); + let program_info = binary_packages .par_bridge() .map(|(package, program)| { @@ -134,10 +135,13 @@ pub(crate) fn run( } else { // Otherwise print human-readable table. if !info_report.programs.is_empty() { - let mut program_table = table!([Fm->"Package", Fm->"Expression Width", Fm->"ACIR Opcodes", Fm->"Backend Circuit Size"]); + let mut program_table = table!([Fm->"Package", Fm->"Function", Fm->"Expression Width", Fm->"ACIR Opcodes", Fm->"Backend Circuit Size"]); - for program in info_report.programs { - program_table.add_row(program.into()); + for program_info in info_report.programs { + let program_rows: Vec = program_info.into(); + for row in program_rows { + program_table.add_row(row); + } } program_table.printstd(); } @@ -223,18 +227,20 @@ struct ProgramInfo { name: String, #[serde(skip)] expression_width: ExpressionWidth, - acir_opcodes: usize, - circuit_size: u32, + functions: Vec, } -impl From for Row { +impl From for Vec { fn from(program_info: ProgramInfo) -> Self { - row![ - Fm->format!("{}", program_info.name), - format!("{:?}", program_info.expression_width), - Fc->format!("{}", program_info.acir_opcodes), - Fc->format!("{}", program_info.circuit_size), - ] + vecmap(program_info.functions, |function| { + row![ + Fm->format!("{}", program_info.name), + Fc->format!("{}", function.name), + format!("{:?}", program_info.expression_width), + Fc->format!("{}", function.acir_opcodes), + Fc->format!("{}", function.circuit_size), + ] + }) } } @@ -243,6 +249,7 @@ struct ContractInfo { name: String, #[serde(skip)] expression_width: ExpressionWidth, + // TODO(https://github.com/noir-lang/noir/issues/4720): Settle on how to display contract functions with non-inlined Acir calls functions: Vec, } @@ -273,13 +280,22 @@ fn count_opcodes_and_gates_in_program( package: &Package, expression_width: ExpressionWidth, ) -> Result { - Ok(ProgramInfo { - name: package.name.to_string(), - expression_width, - // TODO(https://github.com/noir-lang/noir/issues/4428) - acir_opcodes: compiled_program.program.functions[0].opcodes.len(), - circuit_size: backend.get_exact_circuit_size(&compiled_program.program)?, - }) + let functions = compiled_program + .program + .functions + .into_par_iter() + .enumerate() + .map(|(i, function)| -> Result<_, BackendError> { + Ok(FunctionInfo { + name: compiled_program.names[i].clone(), + acir_opcodes: function.opcodes.len(), + circuit_size: backend + .get_exact_circuit_size(&Program { functions: vec![function] })?, + }) + }) + .collect::>()?; + + Ok(ProgramInfo { name: package.name.to_string(), expression_width, functions }) } fn count_opcodes_and_gates_in_contract( @@ -293,7 +309,7 @@ fn count_opcodes_and_gates_in_contract( .map(|function| -> Result<_, BackendError> { Ok(FunctionInfo { name: function.name, - // TODO(https://github.com/noir-lang/noir/issues/4428) + // TODO(https://github.com/noir-lang/noir/issues/4720) acir_opcodes: function.bytecode.functions[0].opcodes.len(), circuit_size: backend.get_exact_circuit_size(&function.bytecode)?, })