From 224bc48e78eec2afbd2325ef4a63972f4d270914 Mon Sep 17 00:00:00 2001 From: Tom French <tom@tomfren.ch> Date: Tue, 12 Sep 2023 10:52:49 +0100 Subject: [PATCH 1/3] chore: defer reporting of errors until after compilation and optimization is finished --- .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 4 +- tooling/nargo_cli/src/cli/compile_cmd.rs | 82 +++++++++++++++---- tooling/nargo_cli/src/cli/execute_cmd.rs | 4 +- tooling/nargo_cli/src/cli/info_cmd.rs | 12 ++- tooling/nargo_cli/src/cli/prove_cmd.rs | 4 +- tooling/nargo_cli/src/cli/verify_cmd.rs | 4 +- 6 files changed, 82 insertions(+), 28 deletions(-) diff --git a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs index 4589c0f7cfb..6199bf0761d 100644 --- a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use super::NargoConfig; use super::{ - compile_cmd::compile_package, + compile_cmd::compile_bin_package, fs::{create_named_dir, program::read_program_from_file, write_to_file}, }; use crate::backends::Backend; @@ -82,7 +82,7 @@ fn smart_contract_for_package( read_program_from_file(circuit_build_path)? } else { let (program, _) = - compile_package(package, compile_options, np_language, &is_opcode_supported)?; + compile_bin_package(package, compile_options, np_language, &is_opcode_supported)?; PreprocessedProgram { backend: String::from(BACKEND_IDENTIFIER), diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 251af055107..7a06b43937a 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -11,9 +11,7 @@ use nargo::artifacts::program::PreprocessedProgram; use nargo::package::Package; use nargo::prepare_package; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; -use noirc_driver::{ - compile_main, CompilationResult, CompileOptions, CompiledContract, CompiledProgram, -}; +use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, CompiledProgram}; use noirc_frontend::graph::CrateName; use clap::Args; @@ -68,12 +66,18 @@ pub(crate) fn run( for package in &workspace { // If `contract` package type, we're compiling every function in a 'contract' rather than just 'main'. if package.is_contract() { - let contracts_with_debug_artifacts = compile_contracts( + let (file_manager, compilation_result) = compile_contracts( package, &args.compile_options, np_language, &is_opcode_supported, + ); + let contracts_with_debug_artifacts = report_errors( + compilation_result, + &file_manager, + args.compile_options.deny_warnings, )?; + save_contracts( contracts_with_debug_artifacts, package, @@ -81,8 +85,14 @@ pub(crate) fn run( args.output_debug, ); } else { - let (program, debug_artifact) = - compile_package(package, &args.compile_options, np_language, &is_opcode_supported)?; + let (file_manager, compilation_result) = + compile_program(package, &args.compile_options, np_language, &is_opcode_supported); + + let (program, debug_artifact) = report_errors( + compilation_result, + &file_manager, + args.compile_options.deny_warnings, + )?; save_program(debug_artifact, program, package, &circuit_dir, args.output_debug); } } @@ -90,7 +100,7 @@ pub(crate) fn run( Ok(()) } -pub(crate) fn compile_package( +pub(crate) fn compile_bin_package( package: &Package, compile_options: &CompileOptions, np_language: Language, @@ -100,9 +110,43 @@ pub(crate) fn compile_package( return Err(CompileError::LibraryCrate(package.name.clone()).into()); } + let (file_manager, compilation_result) = + compile_program(package, compile_options, np_language, &is_opcode_supported); + + let (program, debug_artifact) = + report_errors(compilation_result, &file_manager, compile_options.deny_warnings)?; + + Ok((program, debug_artifact)) +} + +pub(crate) fn compile_contract_package( + package: &Package, + compile_options: &CompileOptions, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, +) -> Result<Vec<(CompiledContract, DebugArtifact)>, CliError> { + let (file_manager, compilation_result) = + compile_contracts(package, compile_options, np_language, &is_opcode_supported); + let contracts_with_debug_artifacts = + report_errors(compilation_result, &file_manager, compile_options.deny_warnings)?; + Ok(contracts_with_debug_artifacts) +} + +fn compile_program( + package: &Package, + compile_options: &CompileOptions, + np_language: Language, + is_opcode_supported: &impl Fn(&Opcode) -> bool, +) -> (FileManager, CompilationResult<(CompiledProgram, DebugArtifact)>) { let (mut context, crate_id) = prepare_package(package); - let result = compile_main(&mut context, crate_id, compile_options); - let program = report_errors(result, &context.file_manager, compile_options.deny_warnings)?; + + let (program, warnings) = + match noirc_driver::compile_main(&mut context, crate_id, compile_options) { + Ok(program_and_warnings) => program_and_warnings, + Err(errors) => { + return (context.file_manager, Err(errors)); + } + }; // Apply backend specific optimizations. let optimized_program = @@ -112,22 +156,28 @@ pub(crate) fn compile_package( let debug_artifact = DebugArtifact::new(vec![optimized_program.debug.clone()], &context.file_manager); - Ok((optimized_program, debug_artifact)) + (context.file_manager, Ok(((optimized_program, debug_artifact), warnings))) } -pub(crate) fn compile_contracts( +fn compile_contracts( package: &Package, compile_options: &CompileOptions, np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, -) -> Result<Vec<(CompiledContract, DebugArtifact)>, CliError> { +) -> (FileManager, CompilationResult<Vec<(CompiledContract, DebugArtifact)>>) { let (mut context, crate_id) = prepare_package(package); - let result = noirc_driver::compile_contracts(&mut context, crate_id, compile_options); - let contracts = report_errors(result, &context.file_manager, compile_options.deny_warnings)?; + let (contracts, warnings) = + match noirc_driver::compile_contracts(&mut context, crate_id, compile_options) { + Ok(contracts_and_warnings) => contracts_and_warnings, + Err(errors) => { + return (context.file_manager, Err(errors)); + } + }; let optimized_contracts = try_vecmap(contracts, |contract| { nargo::ops::optimize_contract(contract, np_language, &is_opcode_supported) - })?; + }) + .expect("Backend does not support an opcode that is in the IR"); let contracts_with_debug_artifacts = vecmap(optimized_contracts, |contract| { let debug_infos = vecmap(&contract.functions, |func| func.debug.clone()); @@ -136,7 +186,7 @@ pub(crate) fn compile_contracts( (contract, debug_artifact) }); - Ok(contracts_with_debug_artifacts) + (context.file_manager, Ok((contracts_with_debug_artifacts, warnings))) } fn save_program( diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index bce14027587..b16f7007182 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -15,7 +15,7 @@ use noirc_driver::{CompileOptions, CompiledProgram}; use noirc_errors::CustomDiagnostic; use noirc_frontend::graph::CrateName; -use super::compile_cmd::compile_package; +use super::compile_cmd::compile_bin_package; use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}; use super::NargoConfig; use crate::backends::Backend; @@ -88,7 +88,7 @@ fn execute_package( is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result<(Option<InputValue>, WitnessMap), CliError> { let (compiled_program, debug_artifact) = - compile_package(package, compile_options, np_language, &is_opcode_supported)?; + compile_bin_package(package, compile_options, np_language, &is_opcode_supported)?; let CompiledProgram { abi, circuit, .. } = compiled_program; // Parse the initial witness values from Prover.toml diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index b030c21f312..e51a0256426 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -11,9 +11,12 @@ use prettytable::{row, table, Row}; use serde::Serialize; use crate::backends::Backend; -use crate::{cli::compile_cmd::compile_package, errors::CliError}; +use crate::errors::CliError; -use super::{compile_cmd::compile_contracts, NargoConfig}; +use super::{ + compile_cmd::{compile_bin_package, compile_contract_package}, + NargoConfig, +}; /// Provides detailed information on a circuit /// @@ -172,7 +175,7 @@ fn count_opcodes_and_gates_in_program( is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result<ProgramInfo, CliError> { let (compiled_program, _) = - compile_package(package, compile_options, np_language, &is_opcode_supported)?; + compile_bin_package(package, compile_options, np_language, &is_opcode_supported)?; let (language, _) = backend.get_backend_info()?; Ok(ProgramInfo { @@ -190,7 +193,8 @@ fn count_opcodes_and_gates_in_contracts( np_language: Language, is_opcode_supported: &impl Fn(&Opcode) -> bool, ) -> Result<Vec<ContractInfo>, CliError> { - let contracts = compile_contracts(package, compile_options, np_language, &is_opcode_supported)?; + let contracts = + compile_contract_package(package, compile_options, np_language, &is_opcode_supported)?; let (language, _) = backend.get_backend_info()?; try_vecmap(contracts, |(contract, _)| { diff --git a/tooling/nargo_cli/src/cli/prove_cmd.rs b/tooling/nargo_cli/src/cli/prove_cmd.rs index b2a2ff1856b..e953732da2f 100644 --- a/tooling/nargo_cli/src/cli/prove_cmd.rs +++ b/tooling/nargo_cli/src/cli/prove_cmd.rs @@ -11,7 +11,7 @@ use noirc_abi::input_parser::Format; use noirc_driver::CompileOptions; use noirc_frontend::graph::CrateName; -use super::compile_cmd::compile_package; +use super::compile_cmd::compile_bin_package; use super::fs::{ inputs::{read_inputs_from_file, write_inputs_to_file}, program::read_program_from_file, @@ -102,7 +102,7 @@ pub(crate) fn prove_package( (program, None) } else { let (program, debug_artifact) = - compile_package(package, compile_options, np_language, &is_opcode_supported)?; + compile_bin_package(package, compile_options, np_language, &is_opcode_supported)?; let preprocessed_program = PreprocessedProgram { backend: String::from(BACKEND_IDENTIFIER), abi: program.abi, diff --git a/tooling/nargo_cli/src/cli/verify_cmd.rs b/tooling/nargo_cli/src/cli/verify_cmd.rs index 4b57c70875f..9d1a98da4da 100644 --- a/tooling/nargo_cli/src/cli/verify_cmd.rs +++ b/tooling/nargo_cli/src/cli/verify_cmd.rs @@ -1,6 +1,6 @@ use super::NargoConfig; use super::{ - compile_cmd::compile_package, + compile_cmd::compile_bin_package, fs::{inputs::read_inputs_from_file, load_hex_data, program::read_program_from_file}, }; use crate::{backends::Backend, errors::CliError}; @@ -86,7 +86,7 @@ fn verify_package( read_program_from_file(circuit_build_path)? } else { let (program, _) = - compile_package(package, compile_options, np_language, &is_opcode_supported)?; + compile_bin_package(package, compile_options, np_language, &is_opcode_supported)?; PreprocessedProgram { backend: String::from(BACKEND_IDENTIFIER), From fbe34c1d2ab6a169c323748570ed4f9dc495a283 Mon Sep 17 00:00:00 2001 From: Tom French <tom@tomfren.ch> Date: Fri, 8 Sep 2023 10:40:01 +0100 Subject: [PATCH 2/3] feat(nargo): compile packages within a workspace in parallel --- Cargo.lock | 1 + tooling/nargo_cli/Cargo.toml | 1 + tooling/nargo_cli/src/cli/compile_cmd.rs | 80 ++++++++++++++---------- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 563681123ed..e84901d48ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2216,6 +2216,7 @@ dependencies = [ "pprof", "predicates 2.1.5", "prettytable-rs", + "rayon", "rustc_version", "serde", "serde_json", diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index e015e0caefd..ec15975613b 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -33,6 +33,7 @@ toml.workspace = true serde.workspace = true serde_json.workspace = true prettytable-rs = "0.10" +rayon = "1.7.0" thiserror.workspace = true tower.workspace = true async-lsp = { version = "0.0.5", default-features = false, features = [ diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 7a06b43937a..a1569b2d76d 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -23,6 +23,7 @@ use super::fs::program::{ save_contract_to_file, save_debug_artifact_to_file, save_program_to_file, }; use super::NargoConfig; +use rayon::prelude::*; // TODO(#1388): pull this from backend. const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg"; @@ -63,38 +64,53 @@ pub(crate) fn run( let circuit_dir = workspace.target_directory_path(); let (np_language, is_opcode_supported) = backend.get_backend_info()?; - for package in &workspace { - // If `contract` package type, we're compiling every function in a 'contract' rather than just 'main'. - if package.is_contract() { - let (file_manager, compilation_result) = compile_contracts( - package, - &args.compile_options, - np_language, - &is_opcode_supported, - ); - let contracts_with_debug_artifacts = report_errors( - compilation_result, - &file_manager, - args.compile_options.deny_warnings, - )?; - - save_contracts( - contracts_with_debug_artifacts, - package, - &circuit_dir, - args.output_debug, - ); - } else { - let (file_manager, compilation_result) = - compile_program(package, &args.compile_options, np_language, &is_opcode_supported); - - let (program, debug_artifact) = report_errors( - compilation_result, - &file_manager, - args.compile_options.deny_warnings, - )?; - save_program(debug_artifact, program, package, &circuit_dir, args.output_debug); - } + + let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace + .members + .iter() + .filter(|package| !package.is_library()) + .partition(|package| package.is_binary()); + + // Compile all of the packages in parallel. + let program_results: Vec<(FileManager, CompilationResult<(CompiledProgram, DebugArtifact)>)> = + binary_packages + .par_iter() + .map(|package| { + compile_program(package, &args.compile_options, np_language, &is_opcode_supported) + }) + .collect(); + let contract_results: Vec<( + FileManager, + CompilationResult<Vec<(CompiledContract, DebugArtifact)>>, + )> = contract_packages + .par_iter() + .map(|package| { + compile_contracts(package, &args.compile_options, np_language, &is_opcode_supported) + }) + .collect(); + + // Report any warnings/errors which were encountered during compilation. + let compiled_programs: Vec<(CompiledProgram, DebugArtifact)> = program_results + .into_iter() + .map(|(file_manager, compilation_result)| { + report_errors(compilation_result, &file_manager, args.compile_options.deny_warnings) + }) + .collect::<Result<_, _>>()?; + let compiled_contracts: Vec<Vec<(CompiledContract, DebugArtifact)>> = contract_results + .into_iter() + .map(|(file_manager, compilation_result)| { + report_errors(compilation_result, &file_manager, args.compile_options.deny_warnings) + }) + .collect::<Result<_, _>>()?; + + // Save build artifacts to disk. + for (package, (program, debug_artifact)) in binary_packages.into_iter().zip(compiled_programs) { + save_program(debug_artifact, program, package, &circuit_dir, args.output_debug); + } + for (package, contracts_with_debug_artifacts) in + contract_packages.into_iter().zip(compiled_contracts) + { + save_contracts(contracts_with_debug_artifacts, package, &circuit_dir, args.output_debug); } Ok(()) From 476f8a987830d8c7dd604fd2664788e11af8687b Mon Sep 17 00:00:00 2001 From: Tom French <tom@tomfren.ch> Date: Tue, 12 Sep 2023 13:58:13 +0100 Subject: [PATCH 3/3] chore: silence clippy --- tooling/nargo_cli/src/cli/compile_cmd.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index a1569b2d76d..61d3b64a47a 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -79,6 +79,7 @@ pub(crate) fn run( compile_program(package, &args.compile_options, np_language, &is_opcode_supported) }) .collect(); + #[allow(clippy::type_complexity)] let contract_results: Vec<( FileManager, CompilationResult<Vec<(CompiledContract, DebugArtifact)>>,