From e1debf7acabdea50d6067c6c9b3c6bd0c3061450 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 24 May 2023 11:35:08 +0100 Subject: [PATCH 01/14] chore: separate circuit compilation and optimization --- crates/nargo/src/ops/preprocess.rs | 33 +++++++++++++---- crates/nargo_cli/src/cli/check_cmd.rs | 4 +- .../nargo_cli/src/cli/codegen_verifier_cmd.rs | 5 ++- crates/nargo_cli/src/cli/compile_cmd.rs | 21 ++++------- crates/nargo_cli/src/cli/execute_cmd.rs | 2 +- crates/nargo_cli/src/cli/gates_cmd.rs | 2 +- crates/nargo_cli/src/cli/mod.rs | 6 +-- crates/nargo_cli/src/cli/prove_cmd.rs | 4 +- crates/nargo_cli/src/cli/test_cmd.rs | 2 +- crates/nargo_cli/src/cli/verify_cmd.rs | 4 +- crates/nargo_cli/src/resolver.rs | 5 +-- crates/noirc_driver/src/lib.rs | 37 ++++--------------- crates/noirc_driver/src/main.rs | 8 +--- crates/noirc_evaluator/src/lib.rs | 25 ++++--------- crates/noirc_evaluator/src/ssa_refactor.rs | 7 +--- crates/wasm/src/compile.rs | 10 ++--- 16 files changed, 68 insertions(+), 107 deletions(-) diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index b545980b963..c07e1a6240c 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -1,7 +1,12 @@ -use acvm::ProofSystemCompiler; +use acvm::{ + acir::circuit::Circuit, compiler::optimizers::simplify::CircuitSimplifier, ProofSystemCompiler, +}; use noirc_driver::{CompiledProgram, ContractFunction}; -use crate::artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram}; +use crate::{ + artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram}, + NargoError, +}; // TODO(#1388): pull this from backend. const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg"; @@ -11,9 +16,7 @@ pub fn preprocess_program( common_reference_string: &[u8], compiled_program: CompiledProgram, ) -> Result { - // TODO: currently `compiled_program`'s bytecode is already optimized for the backend. - // In future we'll need to apply those optimizations here. - let optimized_bytecode = compiled_program.circuit; + let optimized_bytecode = optimize_circuit(backend, compiled_program.circuit).unwrap(); let (proving_key, verification_key) = backend.preprocess(common_reference_string, &optimized_bytecode)?; @@ -31,9 +34,7 @@ pub fn preprocess_contract_function( common_reference_string: &[u8], func: ContractFunction, ) -> Result { - // TODO: currently `func`'s bytecode is already optimized for the backend. - // In future we'll need to apply those optimizations here. - let optimized_bytecode = func.bytecode; + let optimized_bytecode = optimize_circuit(backend, func.bytecode).unwrap(); let (proving_key, verification_key) = backend.preprocess(common_reference_string, &optimized_bytecode)?; @@ -47,3 +48,19 @@ pub fn preprocess_contract_function( verification_key, }) } + +fn optimize_circuit( + backend: &impl ProofSystemCompiler, + circuit: Circuit, +) -> Result { + let simplifier = CircuitSimplifier::new(circuit.current_witness_index); + let optimized_circuit = acvm::compiler::compile( + circuit, + backend.np_language(), + |opcode| backend.supports_opcode(opcode), + &simplifier, + ) + .unwrap(); + + Ok(optimized_circuit) +} diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index af56766cc56..a217a6d933a 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -28,11 +28,11 @@ pub(crate) fn run( } fn check_from_path>( - backend: &B, + _backend: &B, program_dir: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = setup_driver(backend, program_dir.as_ref())?; + let mut driver = setup_driver(program_dir.as_ref())?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index b65d64bb917..e802a110a21 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -51,8 +51,9 @@ pub(crate) fn run( (common_reference_string, program) } None => { - let program = - compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; + let program = compile_circuit(config.program_dir.as_ref(), &args.compile_options)?; + // TODO: optimize circuit before we update common reference string. + // Circuit size will be different to the value used here. let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index d2b5532cf24..4eaf87eb444 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -48,7 +48,7 @@ pub(crate) fn run( // If contracts is set we're compiling every function in a 'contract' rather than just 'main'. if args.contracts { - let mut driver = setup_driver(backend, &config.program_dir)?; + let mut driver = setup_driver(&config.program_dir)?; let compiled_contracts = driver .compile_contracts(&args.compile_options) .map_err(|_| CliError::CompilationError)?; @@ -85,7 +85,9 @@ pub(crate) fn run( ); } } else { - let program = compile_circuit(backend, &config.program_dir, &args.compile_options)?; + let program = compile_circuit(&config.program_dir, &args.compile_options)?; + // TODO: optimize circuit before we update common reference string. + // Circuit size will be different to the value used here. common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; @@ -100,23 +102,14 @@ pub(crate) fn run( Ok(()) } -pub(super) fn setup_driver( - backend: &B, - program_dir: &Path, -) -> Result { - Resolver::resolve_root_manifest( - program_dir, - backend.np_language(), - // TODO(#1102): Remove need for driver to be aware of backend. - Box::new(|op| B::default().supports_opcode(op)), - ) +pub(super) fn setup_driver(program_dir: &Path) -> Result { + Resolver::resolve_root_manifest(program_dir) } pub(crate) fn compile_circuit( - backend: &B, program_dir: &Path, compile_options: &CompileOptions, ) -> Result> { - let mut driver = setup_driver(backend, program_dir)?; + let mut driver = setup_driver(program_dir)?; driver.compile_main(compile_options).map_err(|_| CliError::CompilationError) } diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index 61e709ce500..a32a334614f 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -52,7 +52,7 @@ fn execute_with_path( program_dir: &Path, compile_options: &CompileOptions, ) -> Result<(Option, WitnessMap), CliError> { - let CompiledProgram { abi, circuit } = compile_circuit(backend, program_dir, compile_options)?; + let CompiledProgram { abi, circuit } = compile_circuit(program_dir, compile_options)?; // Parse the initial witness values from Prover.toml let (inputs_map, _) = diff --git a/crates/nargo_cli/src/cli/gates_cmd.rs b/crates/nargo_cli/src/cli/gates_cmd.rs index 88e11c683eb..f28370efc15 100644 --- a/crates/nargo_cli/src/cli/gates_cmd.rs +++ b/crates/nargo_cli/src/cli/gates_cmd.rs @@ -28,7 +28,7 @@ fn count_gates_with_path>( program_dir: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let compiled_program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let compiled_program = compile_circuit(program_dir.as_ref(), compile_options)?; let num_opcodes = compiled_program.circuit.opcodes.len(); println!( diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index 738b6a8511f..2b32b7c1759 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -126,11 +126,7 @@ mod tests { /// /// This is used for tests. fn file_compiles>(root_file: P) -> bool { - let mut driver = Driver::new( - &acvm::Language::R1CS, - #[allow(deprecated)] - Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)), - ); + let mut driver = Driver::new(); driver.create_local_crate(&root_file, CrateType::Binary); crate::resolver::add_std_lib(&mut driver); driver.file_compiles() diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 1238dbd9f8e..609798ca32d 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -90,7 +90,9 @@ pub(crate) fn prove_with_path>( (common_reference_string, program) } None => { - let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let program = compile_circuit(program_dir.as_ref(), compile_options)?; + // TODO: optimize circuit before we update common reference string. + // Circuit size will be different to the value used here. let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index e4fadfa41c5..00ea92a0e45 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -37,7 +37,7 @@ fn run_tests( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = setup_driver(backend, program_dir)?; + let mut driver = setup_driver(program_dir)?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 26eb39f3f81..4d0d5818f39 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -77,7 +77,9 @@ fn verify_with_path>( (common_reference_string, program) } None => { - let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; + let program = compile_circuit(program_dir.as_ref(), compile_options)?; + // TODO: optimize circuit before we update common reference string. + // Circuit size will be different to the value used here. let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index 2ce26a44b26..799bff53aa5 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,7 +3,6 @@ use std::{ path::{Path, PathBuf}, }; -use acvm::{acir::circuit::Opcode, Language}; use nargo::manifest::{Dependency, PackageManifest}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateId, CrateName, CrateType}; @@ -70,10 +69,8 @@ impl<'a> Resolver<'a> { /// XXX: Need to handle when a local package changes! pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, - np_language: Language, - is_opcode_supported: Box bool>, ) -> Result { - let mut driver = Driver::new(&np_language, is_opcode_supported); + let mut driver = Driver::new(); let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; let manifest_path = super::find_package_manifest(dir_path)?; diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 86fea37c5fc..d2c8fa86279 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -3,8 +3,6 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] -use acvm::acir::circuit::Opcode; -use acvm::Language; use clap::Args; use fm::FileType; use iter_extended::try_vecmap; @@ -27,8 +25,6 @@ pub use program::CompiledProgram; pub struct Driver { context: Context, - language: Language, - is_opcode_supported: Box bool>, } #[derive(Args, Clone, Debug, Serialize, Deserialize)] @@ -67,18 +63,14 @@ impl Default for CompileOptions { } impl Driver { - pub fn new(language: &Language, is_opcode_supported: Box bool>) -> Self { - Driver { context: Context::default(), language: language.clone(), is_opcode_supported } + pub fn new() -> Self { + Driver { context: Context::default() } } // This is here for backwards compatibility // with the restricted version which only uses one file - pub fn compile_file( - root_file: PathBuf, - language: &Language, - is_opcode_supported: Box bool>, - ) -> Result { - let mut driver = Driver::new(language, is_opcode_supported); + pub fn compile_file(root_file: PathBuf) -> Result { + let mut driver = Driver::new(); driver.create_local_crate(root_file, CrateType::Binary); driver.compile_main(&CompileOptions::default()) } @@ -283,24 +275,10 @@ impl Driver { ) -> Result { let program = monomorphize(main_function, &self.context.def_interner); - let np_language = self.language.clone(); - let circuit_abi = if options.experimental_ssa { - experimental_create_circuit( - program, - np_language, - &self.is_opcode_supported, - options.show_ssa, - options.show_output, - ) + experimental_create_circuit(program, options.show_ssa, options.show_output) } else { - create_circuit( - program, - np_language, - &self.is_opcode_supported, - options.show_ssa, - options.show_output, - ) + create_circuit(program, options.show_ssa, options.show_output) }; match circuit_abi { @@ -345,7 +323,6 @@ impl Driver { impl Default for Driver { fn default() -> Self { - #[allow(deprecated)] - Self::new(&Language::R1CS, Box::new(acvm::default_is_opcode_supported(Language::R1CS))) + Self::new() } } diff --git a/crates/noirc_driver/src/main.rs b/crates/noirc_driver/src/main.rs index ca7c5faa808..626e4ec7e24 100644 --- a/crates/noirc_driver/src/main.rs +++ b/crates/noirc_driver/src/main.rs @@ -1,16 +1,12 @@ -use acvm::Language; use noirc_driver::{CompileOptions, Driver}; use noirc_frontend::graph::{CrateType, LOCAL_CRATE}; + fn main() { const EXTERNAL_DIR: &str = "dep_b/lib.nr"; const EXTERNAL_DIR2: &str = "dep_a/lib.nr"; const ROOT_DIR_MAIN: &str = "example_real_project/main.nr"; - let mut driver = Driver::new( - &Language::R1CS, - #[allow(deprecated)] - Box::new(acvm::default_is_opcode_supported(Language::R1CS)), - ); + let mut driver = Driver::new(); // Add local crate to dep graph driver.create_local_crate(ROOT_DIR_MAIN, CrateType::Binary); diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 506b0c2ceba..0103fcb67d0 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -13,8 +13,6 @@ pub mod ssa_refactor; use acvm::{ acir::circuit::{opcodes::Opcode as AcirOpcode, Circuit, PublicInputs}, acir::native_types::{Expression, Witness}, - compiler::optimizers::simplify::CircuitSimplifier, - Language, }; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::btree_map; @@ -65,8 +63,6 @@ pub struct Evaluator { // If we had a composer object, we would not need it pub fn create_circuit( program: Program, - np_language: Language, - is_opcode_supported: &impl Fn(&AcirOpcode) -> bool, enable_logging: bool, show_output: bool, ) -> Result<(Circuit, Abi), RuntimeError> { @@ -83,24 +79,17 @@ pub fn create_circuit( opcodes, .. } = evaluator; - let simplifier = CircuitSimplifier::new(current_witness_index); - let optimized_circuit = acvm::compiler::compile( - Circuit { - current_witness_index, - opcodes, - public_parameters: PublicInputs(public_parameters), - return_values: PublicInputs(return_values.iter().copied().collect()), - }, - np_language, - is_opcode_supported, - &simplifier, - ) - .map_err(|_| RuntimeErrorKind::Spanless(String::from("produced an acvm compile error")))?; + let circuit = Circuit { + current_witness_index, + opcodes, + public_parameters: PublicInputs(public_parameters), + return_values: PublicInputs(return_values.iter().copied().collect()), + }; let (parameters, return_type) = program.main_function_signature; let abi = Abi { parameters, param_witnesses, return_type, return_witnesses: return_values }; - Ok((optimized_circuit, abi)) + Ok((circuit, abi)) } impl Evaluator { diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index af5f89bc122..1d80d3251e4 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -8,10 +8,7 @@ #![allow(dead_code)] use crate::errors::{RuntimeError, RuntimeErrorKind}; -use acvm::{ - acir::circuit::{Circuit, Opcode as AcirOpcode}, - Language, -}; +use acvm::acir::circuit::Circuit; use noirc_abi::Abi; use noirc_frontend::monomorphization::ast::Program; @@ -41,8 +38,6 @@ pub fn optimize_into_acir(program: Program) { /// to use the new ssa module to process Noir code. pub fn experimental_create_circuit( _program: Program, - _np_language: Language, - _is_opcode_supported: &impl Fn(&AcirOpcode) -> bool, _enable_logging: bool, _show_output: bool, ) -> Result<(Circuit, Abi), RuntimeError> { diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index e515372cf3a..214bd1beef7 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -74,13 +74,7 @@ pub fn compile(args: JsValue) -> JsValue { debug!("Compiler configuration {:?}", &options); - // For now we default to plonk width = 3, though we can add it as a parameter - let language = acvm::Language::PLONKCSat { width: 3 }; - let mut driver = noirc_driver::Driver::new( - &language, - #[allow(deprecated)] - Box::new(acvm::default_is_opcode_supported(language.clone())), - ); + let mut driver = noirc_driver::Driver::new(); let path = PathBuf::from(&options.entry_point); driver.create_local_crate(path, CrateType::Binary); @@ -99,6 +93,7 @@ pub fn compile(args: JsValue) -> JsValue { .compile_contracts(&options.compile_options) .unwrap_or_else(|_| panic!("Contract compilation failed")); + // TODO: optimize circuits ::from_serde(&compiled_contracts).unwrap() } else { let main = @@ -107,6 +102,7 @@ pub fn compile(args: JsValue) -> JsValue { .compile_no_check(&options.compile_options, main) .unwrap_or_else(|_| panic!("Compilation failed")); + // TODO: optimize circuit ::from_serde(&compiled_program).unwrap() } } From 4f4ac7970f19b50ae8e467a29605298018b26f43 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 24 May 2023 16:33:47 +0000 Subject: [PATCH 02/14] chore: add hacky fix to broken circuit serialization --- crates/nargo/src/ops/mod.rs | 2 +- crates/nargo/src/ops/preprocess.rs | 12 +++++------- crates/nargo_cli/src/cli/codegen_verifier_cmd.rs | 10 ++++++---- crates/nargo_cli/src/cli/compile_cmd.rs | 8 +++++--- crates/nargo_cli/src/cli/execute_cmd.rs | 2 ++ crates/nargo_cli/src/cli/prove_cmd.rs | 11 +++++++---- crates/nargo_cli/src/cli/verify_cmd.rs | 10 ++++++---- 7 files changed, 32 insertions(+), 23 deletions(-) diff --git a/crates/nargo/src/ops/mod.rs b/crates/nargo/src/ops/mod.rs index 8a0cce4b8c5..1ce94f5d3e3 100644 --- a/crates/nargo/src/ops/mod.rs +++ b/crates/nargo/src/ops/mod.rs @@ -1,6 +1,6 @@ pub use self::codegen_verifier::codegen_verifier; pub use self::execute::execute_circuit; -pub use self::preprocess::{preprocess_contract_function, preprocess_program}; +pub use self::preprocess::{optimize_circuit, preprocess_contract_function, preprocess_program}; pub use self::prove::prove_execution; pub use self::verify::verify_proof; diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index c07e1a6240c..c4368cf0460 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -16,14 +16,13 @@ pub fn preprocess_program( common_reference_string: &[u8], compiled_program: CompiledProgram, ) -> Result { - let optimized_bytecode = optimize_circuit(backend, compiled_program.circuit).unwrap(); let (proving_key, verification_key) = - backend.preprocess(common_reference_string, &optimized_bytecode)?; + backend.preprocess(common_reference_string, &compiled_program.circuit)?; Ok(PreprocessedProgram { backend: String::from(BACKEND_IDENTIFIER), abi: compiled_program.abi, - bytecode: optimized_bytecode, + bytecode: compiled_program.circuit, proving_key, verification_key, }) @@ -34,22 +33,21 @@ pub fn preprocess_contract_function( common_reference_string: &[u8], func: ContractFunction, ) -> Result { - let optimized_bytecode = optimize_circuit(backend, func.bytecode).unwrap(); let (proving_key, verification_key) = - backend.preprocess(common_reference_string, &optimized_bytecode)?; + backend.preprocess(common_reference_string, &func.bytecode)?; Ok(PreprocessedContractFunction { name: func.name, function_type: func.function_type, abi: func.abi, - bytecode: optimized_bytecode, + bytecode: func.bytecode, proving_key, verification_key, }) } -fn optimize_circuit( +pub fn optimize_circuit( backend: &impl ProofSystemCompiler, circuit: Circuit, ) -> Result { diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index e802a110a21..ada65fffdb3 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -14,8 +14,8 @@ use crate::{ }; use acvm::Backend; use clap::Args; -use nargo::ops::{codegen_verifier, preprocess_program}; -use noirc_driver::CompileOptions; +use nargo::ops::{codegen_verifier, optimize_circuit, preprocess_program}; +use noirc_driver::{CompileOptions, CompiledProgram}; /// Generates a Solidity verifier smart contract for the program #[derive(Debug, Clone, Args)] @@ -52,8 +52,10 @@ pub(crate) fn run( } None => { let program = compile_circuit(config.program_dir.as_ref(), &args.compile_options)?; - // TODO: optimize circuit before we update common reference string. - // Circuit size will be different to the value used here. + // TODO: clean this up + let optimized_bytecode = optimize_circuit(backend, program.circuit).unwrap(); + let program = CompiledProgram { circuit: optimized_bytecode, abi: program.abi }; + let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 4eaf87eb444..682d182905b 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -6,7 +6,7 @@ use std::path::Path; use clap::Args; -use nargo::ops::{preprocess_contract_function, preprocess_program}; +use nargo::ops::{optimize_circuit, preprocess_contract_function, preprocess_program}; use crate::resolver::DependencyResolutionError; use crate::{constants::TARGET_DIR, errors::CliError, resolver::Resolver}; @@ -86,8 +86,10 @@ pub(crate) fn run( } } else { let program = compile_circuit(&config.program_dir, &args.compile_options)?; - // TODO: optimize circuit before we update common reference string. - // Circuit size will be different to the value used here. + // TODO: clean this up + let optimized_bytecode = optimize_circuit(backend, program.circuit).unwrap(); + let program = CompiledProgram { circuit: optimized_bytecode, abi: program.abi }; + common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index a32a334614f..bac0f63c489 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -58,6 +58,8 @@ fn execute_with_path( let (inputs_map, _) = read_inputs_from_file(program_dir, PROVER_INPUT_FILE, Format::Toml, &abi)?; + // Note that we're executing an unoptimized circuit here. This is fine for calculating a return value. + // This is not good for generating witnesses. let solved_witness = execute_program(backend, circuit, &abi, &inputs_map)?; let public_abi = abi.public_abi(); diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index 609798ca32d..24f80ffaff4 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -3,9 +3,9 @@ use std::path::{Path, PathBuf}; use acvm::Backend; use clap::Args; use nargo::artifacts::program::PreprocessedProgram; -use nargo::ops::{preprocess_program, prove_execution, verify_proof}; +use nargo::ops::{optimize_circuit, preprocess_program, prove_execution, verify_proof}; use noirc_abi::input_parser::Format; -use noirc_driver::CompileOptions; +use noirc_driver::{CompileOptions, CompiledProgram}; use super::NargoConfig; use super::{ @@ -81,6 +81,7 @@ pub(crate) fn prove_with_path>( let (common_reference_string, preprocessed_program) = match circuit_build_path { Some(circuit_build_path) => { let program = read_program_from_file(circuit_build_path)?; + let common_reference_string = update_common_reference_string( backend, &common_reference_string, @@ -91,8 +92,10 @@ pub(crate) fn prove_with_path>( } None => { let program = compile_circuit(program_dir.as_ref(), compile_options)?; - // TODO: optimize circuit before we update common reference string. - // Circuit size will be different to the value used here. + // TODO: clean this up + let optimized_bytecode = optimize_circuit(backend, program.circuit).unwrap(); + let program = CompiledProgram { circuit: optimized_bytecode, abi: program.abi }; + let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 4d0d5818f39..aafaf550ac6 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -17,9 +17,9 @@ use crate::{ use acvm::Backend; use clap::Args; use nargo::artifacts::program::PreprocessedProgram; -use nargo::ops::{preprocess_program, verify_proof}; +use nargo::ops::{optimize_circuit, preprocess_program, verify_proof}; use noirc_abi::input_parser::Format; -use noirc_driver::CompileOptions; +use noirc_driver::{CompileOptions, CompiledProgram}; use std::path::{Path, PathBuf}; /// Given a proof and a program, verify whether the proof is valid @@ -78,8 +78,10 @@ fn verify_with_path>( } None => { let program = compile_circuit(program_dir.as_ref(), compile_options)?; - // TODO: optimize circuit before we update common reference string. - // Circuit size will be different to the value used here. + // TODO: clean this up + let optimized_bytecode = optimize_circuit(backend, program.circuit).unwrap(); + let program = CompiledProgram { circuit: optimized_bytecode, abi: program.abi }; + let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; From fd312ae0b25a3b130d0073e5d2108ccd342f0009 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 24 May 2023 16:40:00 +0000 Subject: [PATCH 03/14] chore: derive `Default` for `Driver` --- crates/noirc_driver/src/lib.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index d2c8fa86279..10b595894d4 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -23,6 +23,7 @@ mod program; pub use contract::{CompiledContract, ContractFunction, ContractFunctionType}; pub use program::CompiledProgram; +#[derive(Default)] pub struct Driver { context: Context, } @@ -64,7 +65,7 @@ impl Default for CompileOptions { impl Driver { pub fn new() -> Self { - Driver { context: Context::default() } + Driver::default() } // This is here for backwards compatibility @@ -320,9 +321,3 @@ impl Driver { self.context.def_interner.function_name(&id) } } - -impl Default for Driver { - fn default() -> Self { - Self::new() - } -} From 916b8075d29b9b1899d4876a2f660663bcc27a5a Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 10 Jul 2023 23:51:41 +0000 Subject: [PATCH 04/14] chore: remove double optimization --- crates/nargo_cli/src/cli/codegen_verifier_cmd.rs | 3 +-- crates/nargo_cli/src/cli/prove_cmd.rs | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index 11eb5eae248..b633533bcc8 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -51,9 +51,8 @@ pub(crate) fn run( (common_reference_string, program) } None => { - let mut program = + let program = compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; - program.circuit = optimize_circuit(backend, program.circuit).unwrap(); let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index ee5c918f579..dd64ee3e0d1 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -94,7 +94,6 @@ pub(crate) fn prove_with_path>( let (common_reference_string, preprocessed_program) = match circuit_build_path { Some(circuit_build_path) => { let program = read_program_from_file(circuit_build_path)?; - let common_reference_string = update_common_reference_string( backend, &common_reference_string, @@ -105,7 +104,6 @@ pub(crate) fn prove_with_path>( } None => { let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; - let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; From 21f647f1f6704a992ea1b5601c20c2d3710e3b66 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 10 Jul 2023 23:53:14 +0000 Subject: [PATCH 05/14] chore: clippy --- crates/nargo_cli/src/cli/codegen_verifier_cmd.rs | 4 ++-- crates/nargo_cli/src/cli/compile_cmd.rs | 4 ++-- crates/nargo_cli/src/cli/prove_cmd.rs | 4 ++-- crates/nargo_cli/src/cli/verify_cmd.rs | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index b633533bcc8..07ec2f85af9 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -14,8 +14,8 @@ use crate::{ }; use acvm::Backend; use clap::Args; -use nargo::ops::{codegen_verifier, optimize_circuit, preprocess_program}; -use noirc_driver::{CompileOptions, CompiledProgram}; +use nargo::ops::{codegen_verifier, preprocess_program}; +use noirc_driver::CompileOptions; /// Generates a Solidity verifier smart contract for the program #[derive(Debug, Clone, Args)] diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 15843d363a0..639fbd488bd 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -100,7 +100,7 @@ pub(crate) fn run( ); } } else { - let mut program = compile_circuit(backend, &config.program_dir, &args.compile_options)?; + let program = compile_circuit(backend, &config.program_dir, &args.compile_options)?; common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) @@ -118,7 +118,7 @@ pub(crate) fn run( } pub(crate) fn compile_circuit( - _backend: &B, + backend: &B, program_dir: &Path, compile_options: &CompileOptions, ) -> Result> { diff --git a/crates/nargo_cli/src/cli/prove_cmd.rs b/crates/nargo_cli/src/cli/prove_cmd.rs index dd64ee3e0d1..0200b417396 100644 --- a/crates/nargo_cli/src/cli/prove_cmd.rs +++ b/crates/nargo_cli/src/cli/prove_cmd.rs @@ -3,9 +3,9 @@ use std::path::{Path, PathBuf}; use acvm::Backend; use clap::Args; use nargo::artifacts::program::PreprocessedProgram; -use nargo::ops::{optimize_circuit, preprocess_program, prove_execution, verify_proof}; +use nargo::ops::{preprocess_program, prove_execution, verify_proof}; use noirc_abi::input_parser::Format; -use noirc_driver::{CompileOptions, CompiledProgram}; +use noirc_driver::CompileOptions; use super::NargoConfig; use super::{ diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 1a89f60d1fe..08179dded46 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -17,9 +17,9 @@ use crate::{ use acvm::Backend; use clap::Args; use nargo::artifacts::program::PreprocessedProgram; -use nargo::ops::{optimize_circuit, preprocess_program, verify_proof}; +use nargo::ops::{preprocess_program, verify_proof}; use noirc_abi::input_parser::Format; -use noirc_driver::{CompileOptions, CompiledProgram}; +use noirc_driver::CompileOptions; use std::path::{Path, PathBuf}; /// Given a proof and a program, verify whether the proof is valid From c8d6a22f8e9a468a4be4b82c6e45032d72b0eedf Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 10 Jul 2023 23:55:34 +0000 Subject: [PATCH 06/14] chore: remove unnecessary diffs --- crates/nargo_cli/src/cli/verify_cmd.rs | 1 - crates/noirc_evaluator/src/lib.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/nargo_cli/src/cli/verify_cmd.rs b/crates/nargo_cli/src/cli/verify_cmd.rs index 08179dded46..c962e9fd081 100644 --- a/crates/nargo_cli/src/cli/verify_cmd.rs +++ b/crates/nargo_cli/src/cli/verify_cmd.rs @@ -84,7 +84,6 @@ fn verify_with_path>( } None => { let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; - let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 72136ed6d1f..a77ded3e2de 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -81,13 +81,13 @@ pub fn create_circuit( opcodes, .. } = evaluator; - let circuit = Circuit { current_witness_index, opcodes, public_parameters: PublicInputs(public_parameters), return_values: PublicInputs(return_values.iter().copied().collect()), }; + let (parameters, return_type) = program.main_function_signature; let abi = Abi { parameters, param_witnesses, return_type, return_witnesses: return_values }; From e785c7c3f54f5770ef6abda5a4187fe39ec59e27 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:01:29 +0000 Subject: [PATCH 07/14] chore: small fixes --- crates/nargo/src/ops/preprocess.rs | 6 +++-- crates/wasm/src/compile.rs | 35 +++++++++++++----------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index 7c26ab25f08..a72f9da6718 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -69,14 +69,16 @@ pub fn optimize_circuit( backend: &impl ProofSystemCompiler, circuit: Circuit, ) -> Result { - let simplifier = CircuitSimplifier::new(circuit.current_witness_index); + // Note that this makes the `CircuitSimplifier` a noop. + // The `CircuitSimplifier` should be reworked to not rely on values being inserted during ACIR gen. + let simplifier = CircuitSimplifier::new(0); let optimized_circuit = acvm::compiler::compile( circuit, backend.np_language(), |opcode| backend.supports_opcode(opcode), &simplifier, ) - .unwrap(); + .map_err(|_| NargoError::CompilationError)?; Ok(optimized_circuit) } diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index 56d70d4ee47..b1f50cd04f2 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -91,25 +91,13 @@ pub fn compile(args: JsValue) -> JsValue { check_crate(&mut context, false, false).expect("Crate check failed"); - let optimize_acir = |circuit: Circuit| { - // For now we default to plonk width = 3, though we can add it as a parameter - let language = acvm::Language::PLONKCSat { width: 3 }; - #[allow(deprecated)] - let opcode_supported = acvm::pwg::default_is_opcode_supported(language); - let simplifier = CircuitSimplifier::new(0); - acvm::compiler::compile(circuit, language, &opcode_supported, &simplifier) - .expect("Circuit optimization failed") - }; - if options.contracts { let compiled_contracts = compile_contracts(&mut context, &options.compile_options) .expect("Contract compilation failed") .0; - let optimized_contracts: Vec = compiled_contracts - .into_iter() - .map(|contract| optimize_contract(contract, &optimize_acir)) - .collect(); + let optimized_contracts: Vec = + compiled_contracts.into_iter().map(|contract| optimize_contract(contract)).collect(); // TODO: optimize circuits ::from_serde(&optimized_contracts).unwrap() @@ -118,26 +106,33 @@ pub fn compile(args: JsValue) -> JsValue { let mut compiled_program = compile_no_check(&context, &options.compile_options, main).expect("Compilation failed"); - compiled_program.circuit = optimize_acir(compiled_program.circuit); + compiled_program.circuit = optimize_circuit(compiled_program.circuit); // TODO: optimize circuit ::from_serde(&compiled_program).unwrap() } } -fn optimize_contract( - contract: CompiledContract, - acir_optimizer: &impl Fn(Circuit) -> Circuit, -) -> CompiledContract { +fn optimize_contract(contract: CompiledContract) -> CompiledContract { CompiledContract { name: contract.name, functions: contract .functions .into_iter() .map(|mut func| { - func.bytecode = acir_optimizer(func.bytecode); + func.bytecode = optimize_circuit(func.bytecode); func }) .collect(), } } + +fn optimize_circuit(circuit: Circuit) -> Circuit { + // For now we default to plonk width = 3, though we can add it as a parameter + let language = acvm::Language::PLONKCSat { width: 3 }; + #[allow(deprecated)] + let opcode_supported = acvm::pwg::default_is_opcode_supported(language); + let simplifier = CircuitSimplifier::new(0); + acvm::compiler::compile(circuit, language, &opcode_supported, &simplifier) + .expect("Circuit optimization failed") +} From cbc7dcb114ec6feceaf2df3ebd77cb6ff0c92c41 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:03:32 +0000 Subject: [PATCH 08/14] chore: remove stale comment --- crates/nargo_cli/src/cli/execute_cmd.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/nargo_cli/src/cli/execute_cmd.rs b/crates/nargo_cli/src/cli/execute_cmd.rs index 151b2bd30ac..c175c0336f1 100644 --- a/crates/nargo_cli/src/cli/execute_cmd.rs +++ b/crates/nargo_cli/src/cli/execute_cmd.rs @@ -63,8 +63,6 @@ fn execute_with_path( let (inputs_map, _) = read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?; - // Note that we're executing an unoptimized circuit here. This is fine for calculating a return value. - // This is not good for generating witnesses. let solved_witness = execute_program(backend, circuit, &abi, &inputs_map)?; let public_abi = abi.public_abi(); From 911bd4625521a632f2c0f1551d96559620b2d26e Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:04:14 +0000 Subject: [PATCH 09/14] chore: reduce diff --- crates/nargo_cli/src/cli/codegen_verifier_cmd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs index 07ec2f85af9..ae6fa411c7c 100644 --- a/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs +++ b/crates/nargo_cli/src/cli/codegen_verifier_cmd.rs @@ -53,7 +53,6 @@ pub(crate) fn run( None => { let program = compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?; - let common_reference_string = update_common_reference_string(backend, &common_reference_string, &program.circuit) .map_err(CliError::CommonReferenceStringError)?; From cca47f26898d3d9bc52d6bed5388bdfde7f6523b Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:08:23 +0000 Subject: [PATCH 10/14] chore: move optimization into `nargo_cli` --- crates/nargo/src/ops/mod.rs | 2 +- crates/nargo/src/ops/preprocess.rs | 25 ++----------------------- crates/nargo_cli/src/cli/compile_cmd.rs | 21 ++++++++++++++++++--- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/crates/nargo/src/ops/mod.rs b/crates/nargo/src/ops/mod.rs index 1ce94f5d3e3..8a0cce4b8c5 100644 --- a/crates/nargo/src/ops/mod.rs +++ b/crates/nargo/src/ops/mod.rs @@ -1,6 +1,6 @@ pub use self::codegen_verifier::codegen_verifier; pub use self::execute::execute_circuit; -pub use self::preprocess::{optimize_circuit, preprocess_contract_function, preprocess_program}; +pub use self::preprocess::{preprocess_contract_function, preprocess_program}; pub use self::prove::prove_execution; pub use self::verify::verify_proof; diff --git a/crates/nargo/src/ops/preprocess.rs b/crates/nargo/src/ops/preprocess.rs index a72f9da6718..b1613ea3195 100644 --- a/crates/nargo/src/ops/preprocess.rs +++ b/crates/nargo/src/ops/preprocess.rs @@ -1,10 +1,7 @@ -use acvm::{acir::circuit::Circuit, compiler::CircuitSimplifier, ProofSystemCompiler}; +use acvm::ProofSystemCompiler; use noirc_driver::{CompiledProgram, ContractFunction}; -use crate::{ - artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram}, - NargoError, -}; +use crate::artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram}; // TODO(#1388): pull this from backend. const BACKEND_IDENTIFIER: &str = "acvm-backend-barretenberg"; @@ -64,21 +61,3 @@ pub fn preprocess_contract_function( verification_key, }) } - -pub fn optimize_circuit( - backend: &impl ProofSystemCompiler, - circuit: Circuit, -) -> Result { - // Note that this makes the `CircuitSimplifier` a noop. - // The `CircuitSimplifier` should be reworked to not rely on values being inserted during ACIR gen. - let simplifier = CircuitSimplifier::new(0); - let optimized_circuit = acvm::compiler::compile( - circuit, - backend.np_language(), - |opcode| backend.supports_opcode(opcode), - &simplifier, - ) - .map_err(|_| NargoError::CompilationError)?; - - Ok(optimized_circuit) -} diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 639fbd488bd..7185b2c9450 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -1,6 +1,6 @@ -use acvm::Backend; +use acvm::{acir::circuit::Circuit, compiler::CircuitSimplifier, Backend}; use iter_extended::try_vecmap; -use nargo::artifacts::contract::PreprocessedContract; +use nargo::{artifacts::contract::PreprocessedContract, NargoError}; use noirc_driver::{ compile_contracts, compile_main, CompileOptions, CompiledProgram, ErrorsAndWarnings, Warnings, }; @@ -10,7 +10,7 @@ use std::path::Path; use clap::Args; -use nargo::ops::{optimize_circuit, preprocess_contract_function, preprocess_program}; +use nargo::ops::{preprocess_contract_function, preprocess_program}; use crate::{constants::TARGET_DIR, errors::CliError, resolver::resolve_root_manifest}; @@ -131,6 +131,21 @@ pub(crate) fn compile_circuit( Ok(program) } +fn optimize_circuit(backend: &B, circuit: Circuit) -> Result> { + // Note that this makes the `CircuitSimplifier` a noop. + // The `CircuitSimplifier` should be reworked to not rely on values being inserted during ACIR gen. + let simplifier = CircuitSimplifier::new(0); + let optimized_circuit = acvm::compiler::compile( + circuit, + backend.np_language(), + |opcode| backend.supports_opcode(opcode), + &simplifier, + ) + .map_err(|_| NargoError::CompilationError)?; + + Ok(optimized_circuit) +} + /// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings> /// structure that is commonly used as a return result in this file. pub(crate) fn report_errors( From ddef899ed95e8b508830dac7ab89930ef8e23b65 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:09:25 +0000 Subject: [PATCH 11/14] chore: remove stale comments --- crates/wasm/src/compile.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index b1f50cd04f2..e4370e3e678 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -99,7 +99,6 @@ pub fn compile(args: JsValue) -> JsValue { let optimized_contracts: Vec = compiled_contracts.into_iter().map(|contract| optimize_contract(contract)).collect(); - // TODO: optimize circuits ::from_serde(&optimized_contracts).unwrap() } else { let main = context.get_main_function(&crate_id).expect("Could not find main function!"); @@ -108,7 +107,6 @@ pub fn compile(args: JsValue) -> JsValue { compiled_program.circuit = optimize_circuit(compiled_program.circuit); - // TODO: optimize circuit ::from_serde(&compiled_program).unwrap() } } From 04b9e40309466fa7b5910aec6823ac4e2a59fb25 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:11:31 +0000 Subject: [PATCH 12/14] chore: remove `.unwrap()` --- crates/nargo_cli/src/cli/compile_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 7185b2c9450..d54b6b4e3f9 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -68,7 +68,7 @@ pub(crate) fn run( try_vecmap(contracts, |contract| { let preprocessed_contract_functions = try_vecmap(contract.functions, |mut func| { - func.bytecode = optimize_circuit(backend, func.bytecode).unwrap(); + func.bytecode = optimize_circuit(backend, func.bytecode)?; common_reference_string = update_common_reference_string( backend, From dfc12d2e1b85feecf5de776c70fe7db546598365 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:21:37 +0000 Subject: [PATCH 13/14] chore: optimize ACIR used in `nargo test` --- crates/nargo_cli/src/cli/compile_cmd.rs | 5 ++++- crates/nargo_cli/src/cli/test_cmd.rs | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index d54b6b4e3f9..19360efc896 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -131,7 +131,10 @@ pub(crate) fn compile_circuit( Ok(program) } -fn optimize_circuit(backend: &B, circuit: Circuit) -> Result> { +pub(super) fn optimize_circuit( + backend: &B, + circuit: Circuit, +) -> Result> { // Note that this makes the `CircuitSimplifier` a noop. // The `CircuitSimplifier` should be reworked to not rely on values being inserted during ACIR gen. let simplifier = CircuitSimplifier::new(0); diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index 170f14591a2..a2ec9d7968a 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -89,8 +89,10 @@ fn run_test( context: &Context, config: &CompileOptions, ) -> Result<(), CliError> { - let program = compile_no_check(context, config, main) + let mut program = compile_no_check(context, config, main) .map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?; + // Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`. + program.circuit = optimize_circuit(backend, program.circuit).unwrap(); // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, // otherwise constraints involving these expressions will not error. From 58688ff0fc57feff7385fdac5977dfd252427af6 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Tue, 11 Jul 2023 00:23:48 +0000 Subject: [PATCH 14/14] chore: add missing import --- crates/nargo_cli/src/cli/test_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index a2ec9d7968a..e129b38cac9 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -12,7 +12,7 @@ use crate::{ resolver::resolve_root_manifest, }; -use super::NargoConfig; +use super::{compile_cmd::optimize_circuit, NargoConfig}; /// Run the tests for this program #[derive(Debug, Clone, Args)]