From 62a4e37ef2274af2839011c3bab7bfdbf9f164fa Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 23 Jan 2024 13:43:28 +0000 Subject: [PATCH] feat: Separate compilation and expression narrowing in `nargo` interface (#4100) # Description ## Problem\* ## Summary\* This PR moves the responsibility for transforming a circuit into a certain expression width into `nargo_cli`. This gives us better separation of concerns where we no longer need to know the target backend before we compile a circuit (i.e. we can save a generic artifact and then target multiple backends using this) ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- compiler/noirc_driver/src/contract.rs | 4 +-- compiler/wasm/src/compile.rs | 5 +-- compiler/wasm/src/compile_new.rs | 4 +-- tooling/lsp/src/requests/profile_run.rs | 20 +++++++---- tooling/nargo/src/artifacts/debug.rs | 12 +++---- tooling/nargo/src/ops/compile.rs | 35 +++---------------- tooling/nargo/src/ops/mod.rs | 3 ++ tooling/nargo/src/ops/optimize.rs | 18 +++------- tooling/nargo/src/ops/transform.rs | 30 ++++++++++++++++ .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 3 +- tooling/nargo_cli/src/cli/compile_cmd.rs | 19 +++------- tooling/nargo_cli/src/cli/dap_cmd.rs | 12 +++---- tooling/nargo_cli/src/cli/debug_cmd.rs | 3 +- tooling/nargo_cli/src/cli/execute_cmd.rs | 3 +- tooling/nargo_cli/src/cli/info_cmd.rs | 12 +++++-- tooling/nargo_cli/src/cli/prove_cmd.rs | 3 +- tooling/nargo_cli/src/cli/verify_cmd.rs | 3 +- tooling/noirc_abi/src/lib.rs | 2 +- 18 files changed, 95 insertions(+), 96 deletions(-) create mode 100644 tooling/nargo/src/ops/transform.rs diff --git a/compiler/noirc_driver/src/contract.rs b/compiler/noirc_driver/src/contract.rs index 4d6d57ba9b6..5f4b66e7dd2 100644 --- a/compiler/noirc_driver/src/contract.rs +++ b/compiler/noirc_driver/src/contract.rs @@ -26,7 +26,7 @@ pub enum ContractFunctionType { Unconstrained, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct CompiledContract { pub noir_version: String, @@ -51,7 +51,7 @@ pub struct CompiledContract { /// A contract function unlike a regular Noir program /// however can have additional properties. /// One of these being a function type. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct ContractFunction { pub name: String, diff --git a/compiler/wasm/src/compile.rs b/compiler/wasm/src/compile.rs index 498ffe447ce..b39a27a7931 100644 --- a/compiler/wasm/src/compile.rs +++ b/compiler/wasm/src/compile.rs @@ -190,7 +190,8 @@ pub fn compile( })? .0; - let optimized_contract = nargo::ops::optimize_contract(compiled_contract, expression_width); + let optimized_contract = + nargo::ops::transform_contract(compiled_contract, expression_width); let compile_output = generate_contract_artifact(optimized_contract); Ok(JsCompileResult::new(compile_output)) @@ -205,7 +206,7 @@ pub fn compile( })? .0; - let optimized_program = nargo::ops::optimize_program(compiled_program, expression_width); + let optimized_program = nargo::ops::transform_program(compiled_program, expression_width); let compile_output = generate_program_artifact(optimized_program); Ok(JsCompileResult::new(compile_output)) diff --git a/compiler/wasm/src/compile_new.rs b/compiler/wasm/src/compile_new.rs index 6476f6d29bc..4616004ae2b 100644 --- a/compiler/wasm/src/compile_new.rs +++ b/compiler/wasm/src/compile_new.rs @@ -109,7 +109,7 @@ impl CompilerContext { })? .0; - let optimized_program = nargo::ops::optimize_program(compiled_program, np_language); + let optimized_program = nargo::ops::transform_program(compiled_program, np_language); let compile_output = generate_program_artifact(optimized_program); Ok(JsCompileResult::new(compile_output)) @@ -134,7 +134,7 @@ impl CompilerContext { })? .0; - let optimized_contract = nargo::ops::optimize_contract(compiled_contract, np_language); + let optimized_contract = nargo::ops::transform_contract(compiled_contract, np_language); let compile_output = generate_contract_artifact(optimized_contract); Ok(JsCompileResult::new(compile_output)) diff --git a/tooling/lsp/src/requests/profile_run.rs b/tooling/lsp/src/requests/profile_run.rs index 8ba91338f55..d866be8988b 100644 --- a/tooling/lsp/src/requests/profile_run.rs +++ b/tooling/lsp/src/requests/profile_run.rs @@ -64,26 +64,32 @@ fn on_profile_run_request_inner( &workspace_file_manager, &parsed_files, &workspace, - expression_width, &CompileOptions::default(), ) .map_err(|err| ResponseError::new(ErrorCode::REQUEST_FAILED, err))?; let mut opcodes_counts: HashMap = HashMap::new(); let mut file_map: BTreeMap = BTreeMap::new(); - for compiled_program in &compiled_programs { + for compiled_program in compiled_programs { + let compiled_program = + nargo::ops::transform_program(compiled_program, expression_width); + let span_opcodes = compiled_program.debug.count_span_opcodes(); let debug_artifact: DebugArtifact = compiled_program.clone().into(); opcodes_counts.extend(span_opcodes); file_map.extend(debug_artifact.file_map); } - for compiled_contract in &compiled_contracts { - let functions = &compiled_contract.functions; - let debug_artifact: DebugArtifact = compiled_contract.clone().into(); + for compiled_contract in compiled_contracts { + let compiled_contract = + nargo::ops::transform_contract(compiled_contract, expression_width); + + let function_debug_info: Vec<_> = + compiled_contract.functions.iter().map(|func| &func.debug).cloned().collect(); + let debug_artifact: DebugArtifact = compiled_contract.into(); file_map.extend(debug_artifact.file_map); - for contract_function in functions { - let span_opcodes = contract_function.debug.count_span_opcodes(); + for contract_function_debug in function_debug_info { + let span_opcodes = contract_function_debug.count_span_opcodes(); opcodes_counts.extend(span_opcodes); } } diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index 3f5df801b66..2e2d98f279e 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -126,18 +126,18 @@ impl From for DebugArtifact { } } -impl From<&CompiledContract> for DebugArtifact { - fn from(compiled_artifact: &CompiledContract) -> Self { +impl From for DebugArtifact { + fn from(compiled_artifact: CompiledContract) -> Self { let all_functions_debug: Vec = compiled_artifact .functions - .iter() - .map(|contract_function| contract_function.debug.clone()) + .into_iter() + .map(|contract_function| contract_function.debug) .collect(); DebugArtifact { debug_symbols: all_functions_debug, - file_map: compiled_artifact.file_map.clone(), - warnings: compiled_artifact.warnings.clone(), + file_map: compiled_artifact.file_map, + warnings: compiled_artifact.warnings, } } } diff --git a/tooling/nargo/src/ops/compile.rs b/tooling/nargo/src/ops/compile.rs index 866bfe39d7b..dccd2cedbf5 100644 --- a/tooling/nargo/src/ops/compile.rs +++ b/tooling/nargo/src/ops/compile.rs @@ -1,4 +1,3 @@ -use acvm::ExpressionWidth; use fm::FileManager; use noirc_driver::{CompilationResult, CompileOptions, CompiledContract, CompiledProgram}; use noirc_frontend::hir::ParsedFiles; @@ -18,7 +17,6 @@ pub fn compile_workspace( file_manager: &FileManager, parsed_files: &ParsedFiles, workspace: &Workspace, - expression_width: ExpressionWidth, compile_options: &CompileOptions, ) -> Result<(Vec, Vec), CompileError> { let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace @@ -30,22 +28,11 @@ pub fn compile_workspace( // Compile all of the packages in parallel. let program_results: Vec> = binary_packages .par_iter() - .map(|package| { - compile_program( - file_manager, - parsed_files, - package, - compile_options, - expression_width, - None, - ) - }) + .map(|package| compile_program(file_manager, parsed_files, package, compile_options, None)) .collect(); let contract_results: Vec> = contract_packages .par_iter() - .map(|package| { - compile_contract(file_manager, parsed_files, package, compile_options, expression_width) - }) + .map(|package| compile_contract(file_manager, parsed_files, package, compile_options)) .collect(); // Report any warnings/errors which were encountered during compilation. @@ -80,18 +67,10 @@ pub fn compile_program( parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, - expression_width: ExpressionWidth, cached_program: Option, ) -> CompilationResult { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); - - let (program, warnings) = - noirc_driver::compile_main(&mut context, crate_id, compile_options, cached_program)?; - - // Apply backend specific optimizations. - let optimized_program = crate::ops::optimize_program(program, expression_width); - - Ok((optimized_program, warnings)) + noirc_driver::compile_main(&mut context, crate_id, compile_options, cached_program) } pub fn compile_contract( @@ -99,15 +78,9 @@ pub fn compile_contract( parsed_files: &ParsedFiles, package: &Package, compile_options: &CompileOptions, - expression_width: ExpressionWidth, ) -> CompilationResult { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); - let (contract, warnings) = - noirc_driver::compile_contract(&mut context, crate_id, compile_options)?; - - let optimized_contract = crate::ops::optimize_contract(contract, expression_width); - - Ok((optimized_contract, warnings)) + noirc_driver::compile_contract(&mut context, crate_id, compile_options) } pub(crate) fn report_errors( diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index 4912c84839e..4f92faa73a4 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -2,6 +2,8 @@ pub use self::compile::{compile_contract, compile_program, compile_workspace}; pub use self::execute::execute_circuit; pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor}; pub use self::optimize::{optimize_contract, optimize_program}; +pub use self::transform::{transform_contract, transform_program}; + pub use self::test::{run_test, TestStatus}; mod compile; @@ -9,3 +11,4 @@ mod execute; mod foreign_calls; mod optimize; mod test; +mod transform; diff --git a/tooling/nargo/src/ops/optimize.rs b/tooling/nargo/src/ops/optimize.rs index d3a36dd65ac..2d0c4c43d25 100644 --- a/tooling/nargo/src/ops/optimize.rs +++ b/tooling/nargo/src/ops/optimize.rs @@ -1,26 +1,16 @@ -use acvm::ExpressionWidth; use iter_extended::vecmap; use noirc_driver::{CompiledContract, CompiledProgram}; -pub fn optimize_program( - mut program: CompiledProgram, - expression_width: ExpressionWidth, -) -> CompiledProgram { - let (optimized_circuit, location_map) = - acvm::compiler::compile(program.circuit, expression_width); - +pub fn optimize_program(mut program: CompiledProgram) -> CompiledProgram { + let (optimized_circuit, location_map) = acvm::compiler::optimize(program.circuit); program.circuit = optimized_circuit; program.debug.update_acir(location_map); program } -pub fn optimize_contract( - contract: CompiledContract, - expression_width: ExpressionWidth, -) -> CompiledContract { +pub fn optimize_contract(contract: CompiledContract) -> CompiledContract { let functions = vecmap(contract.functions, |mut func| { - let (optimized_bytecode, location_map) = - acvm::compiler::compile(func.bytecode, expression_width); + let (optimized_bytecode, location_map) = acvm::compiler::optimize(func.bytecode); func.bytecode = optimized_bytecode; func.debug.update_acir(location_map); func diff --git a/tooling/nargo/src/ops/transform.rs b/tooling/nargo/src/ops/transform.rs new file mode 100644 index 00000000000..f3efd82333e --- /dev/null +++ b/tooling/nargo/src/ops/transform.rs @@ -0,0 +1,30 @@ +use acvm::ExpressionWidth; +use iter_extended::vecmap; +use noirc_driver::{CompiledContract, CompiledProgram}; + +pub fn transform_program( + mut program: CompiledProgram, + expression_width: ExpressionWidth, +) -> CompiledProgram { + let (optimized_circuit, location_map) = + acvm::compiler::compile(program.circuit, expression_width); + + program.circuit = optimized_circuit; + program.debug.update_acir(location_map); + program +} + +pub fn transform_contract( + contract: CompiledContract, + expression_width: ExpressionWidth, +) -> CompiledContract { + let functions = vecmap(contract.functions, |mut func| { + let (optimized_bytecode, location_map) = + acvm::compiler::compile(func.bytecode, expression_width); + func.bytecode = optimized_bytecode; + func.debug.update_acir(location_map); + func + }); + + CompiledContract { functions, ..contract } +} diff --git a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs index e7ab86f343a..63d27e30836 100644 --- a/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -53,7 +53,6 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - expression_width, None, ); @@ -64,6 +63,8 @@ pub(crate) fn run( args.compile_options.silence_warnings, )?; + let program = nargo::ops::transform_program(program, expression_width); + let smart_contract_string = backend.eth_contract(&program.circuit)?; let contract_dir = workspace.contracts_directory_path(package); diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 29e6012996a..25cf06a7310 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -1,7 +1,5 @@ use std::path::Path; -use acvm::ExpressionWidth; - use fm::FileManager; use nargo::artifacts::program::ProgramArtifact; use nargo::errors::CompileError; @@ -68,7 +66,6 @@ pub(crate) fn run( &workspace_file_manager, &parsed_files, &workspace, - expression_width, &args.compile_options, )?; @@ -81,9 +78,11 @@ pub(crate) fn run( // Save build artifacts to disk. let only_acir = args.compile_options.only_acir; for (package, program) in binary_packages.into_iter().zip(compiled_program) { + let program = nargo::ops::transform_program(program, expression_width); save_program(program.clone(), &package, &workspace.target_directory_path(), only_acir); } for (package, contract) in contract_packages.into_iter().zip(compiled_contracts) { + let contract = nargo::ops::transform_contract(contract, expression_width); save_contract(contract, &package, &circuit_dir); } @@ -94,7 +93,6 @@ pub(super) fn compile_workspace( file_manager: &FileManager, parsed_files: &ParsedFiles, workspace: &Workspace, - expression_width: ExpressionWidth, compile_options: &CompileOptions, ) -> Result<(Vec, Vec), CliError> { let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace @@ -114,21 +112,12 @@ pub(super) fn compile_workspace( .filter(|p| p.noir_version == NOIR_ARTIFACT_VERSION_STRING) .map(|p| p.into()); - compile_program( - file_manager, - parsed_files, - package, - compile_options, - expression_width, - cached_program, - ) + compile_program(file_manager, parsed_files, package, compile_options, cached_program) }) .collect(); let contract_results: Vec> = contract_packages .par_iter() - .map(|package| { - compile_contract(file_manager, parsed_files, package, compile_options, expression_width) - }) + .map(|package| compile_contract(file_manager, parsed_files, package, compile_options)) .collect(); // Report any warnings/errors which were encountered during compilation. diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index c028545c11c..a02cd66fd4c 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -74,14 +74,8 @@ fn load_and_compile_project( let parsed_files = parse_all(&workspace_file_manager); let compile_options = CompileOptions::default(); - let compilation_result = compile_program( - &workspace_file_manager, - &parsed_files, - package, - &compile_options, - expression_width, - None, - ); + let compilation_result = + compile_program(&workspace_file_manager, &parsed_files, package, &compile_options, None); let compiled_program = report_errors( compilation_result, @@ -91,6 +85,8 @@ fn load_and_compile_project( ) .map_err(|_| LoadError("Failed to compile project"))?; + let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); + let (inputs_map, _) = read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &compiled_program.abi) .map_err(|_| LoadError("Failed to read program inputs"))?; diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index ce89324a3b9..58cc453e01b 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -72,7 +72,6 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - expression_width, None, ); @@ -83,6 +82,8 @@ pub(crate) fn run( args.compile_options.silence_warnings, )?; + let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); + run_async(package, compiled_program, &args.prover_name, &args.witness_name, target_dir) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index a84b2821f1e..c1fd398350c 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -76,7 +76,6 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - expression_width, None, ); @@ -87,6 +86,8 @@ pub(crate) fn run( args.compile_options.silence_warnings, )?; + let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); + let (return_value, solved_witness) = execute_program_and_decode( compiled_program, package, diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 8dfff67b47f..0e9feaca1ca 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -74,19 +74,25 @@ pub(crate) fn run( &workspace_file_manager, &parsed_files, &workspace, - expression_width, &args.compile_options, )?; + let compiled_programs = vecmap(compiled_programs, |program| { + nargo::ops::transform_program(program, expression_width) + }); + let compiled_contracts = vecmap(compiled_contracts, |contract| { + nargo::ops::transform_contract(contract, expression_width) + }); + if args.profile_info { for compiled_program in &compiled_programs { let span_opcodes = compiled_program.debug.count_span_opcodes(); - let debug_artifact: DebugArtifact = compiled_program.clone().into(); + let debug_artifact = DebugArtifact::from(compiled_program.clone()); print_span_opcodes(span_opcodes, &debug_artifact); } for compiled_contract in &compiled_contracts { - let debug_artifact: DebugArtifact = compiled_contract.clone().into(); + let debug_artifact = DebugArtifact::from(compiled_contract.clone()); let functions = &compiled_contract.functions; for contract_function in functions { let span_opcodes = contract_function.debug.count_span_opcodes(); diff --git a/tooling/nargo_cli/src/cli/prove_cmd.rs b/tooling/nargo_cli/src/cli/prove_cmd.rs index a79c21c81c9..479ca2c9452 100644 --- a/tooling/nargo_cli/src/cli/prove_cmd.rs +++ b/tooling/nargo_cli/src/cli/prove_cmd.rs @@ -77,7 +77,6 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - expression_width, None, ); @@ -88,6 +87,8 @@ pub(crate) fn run( args.compile_options.silence_warnings, )?; + let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); + prove_package( backend, &workspace, diff --git a/tooling/nargo_cli/src/cli/verify_cmd.rs b/tooling/nargo_cli/src/cli/verify_cmd.rs index daf623c10c6..582fa32bd8b 100644 --- a/tooling/nargo_cli/src/cli/verify_cmd.rs +++ b/tooling/nargo_cli/src/cli/verify_cmd.rs @@ -62,7 +62,6 @@ pub(crate) fn run( &parsed_files, package, &args.compile_options, - expression_width, None, ); @@ -73,6 +72,8 @@ pub(crate) fn run( args.compile_options.silence_warnings, )?; + let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); + verify_package(backend, &workspace, package, compiled_program, &args.verifier_name)?; } diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 066b1635ced..1fc257c1676 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -512,7 +512,7 @@ fn decode_string_value(field_elements: &[FieldElement]) -> String { final_string.to_owned() } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct ContractEvent { /// Event name name: String,