chore: Perform Circuit optimization in nargo

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,
 };
@@ -57,12 +57,7 @@ pub(crate) fn run<B: Backend>(
     if args.contracts {
         let mut context = resolve_root_manifest(&config.program_dir)?;
 
-        let result = compile_contracts(
-            &mut context,
-            backend.np_language(),
-            &|op| backend.supports_opcode(op),
-            &args.compile_options,
-        );
+        let result = compile_contracts(&mut context, &args.compile_options);
         let contracts = report_errors(result, &context, args.compile_options.deny_warnings)?;
 
         // TODO(#1389): I wonder if it is incorrect for nargo-core to know anything about contracts.
@@ -71,22 +66,25 @@ pub(crate) fn run<B: Backend>(
         // This is due to EACH function needing it's own CRS, PKey, and VKey from the backend.
         let preprocessed_contracts: Result<Vec<PreprocessedContract>, CliError<B>> =
             try_vecmap(contracts, |contract| {
-                let preprocessed_contract_functions = try_vecmap(contract.functions, |func| {
-                    common_reference_string = update_common_reference_string(
-                        backend,
-                        &common_reference_string,
-                        &func.bytecode,
-                    )
-                    .map_err(CliError::CommonReferenceStringError)?;
-
-                    preprocess_contract_function(
-                        backend,
-                        args.include_keys,
-                        &common_reference_string,
-                        func,
-                    )
-                    .map_err(CliError::ProofSystemCompilerError)
-                })?;
+                let preprocessed_contract_functions =
+                    try_vecmap(contract.functions, |mut func| {
+                        func.bytecode = optimize_circuit(backend, func.bytecode)?;
+
+                        common_reference_string = update_common_reference_string(
+                            backend,
+                            &common_reference_string,
+                            &func.bytecode,
+                        )
+                        .map_err(CliError::CommonReferenceStringError)?;
+
+                        preprocess_contract_function(
+                            backend,
+                            args.include_keys,
+                            &common_reference_string,
+                            func,
+                        )
+                        .map_err(CliError::ProofSystemCompilerError)
+                    })?;
 
                 Ok(PreprocessedContract {
                     name: contract.name,
@@ -125,13 +123,30 @@ pub(crate) fn compile_circuit<B: Backend>(
     compile_options: &CompileOptions,
 ) -> Result<CompiledProgram, CliError<B>> {
     let mut context = resolve_root_manifest(program_dir)?;
-    let result = compile_main(
-        &mut context,
+    let result = compile_main(&mut context, compile_options);
+    let mut program = report_errors(result, &context, compile_options.deny_warnings)?;
+
+    // Apply backend specific optimizations.
+    program.circuit = optimize_circuit(backend, program.circuit).unwrap();
+    Ok(program)
+}
+
+pub(super) fn optimize_circuit<B: Backend>(
+    backend: &B,
+    circuit: Circuit,
+) -> Result<Circuit, CliError<B>> {
+    // 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(),
-        &|op| backend.supports_opcode(op),
-        compile_options,
-    );
-    report_errors(result, &context, compile_options.deny_warnings).map_err(Into::into)
+        |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>

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)]
@@ -89,10 +89,10 @@ fn run_test<B: Backend>(
     context: &Context,
     config: &CompileOptions,
 ) -> Result<(), CliError<B>> {
-    let program = compile_no_check(context, config, main, backend.np_language(), &|op| {
-        backend.supports_opcode(op)
-    })
-    .map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;
+    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.

crates/noirc_driver/src/lib.rs

@@ -3,9 +3,6 @@
 #![warn(unreachable_pub)]
 #![warn(clippy::semicolon_if_nothing_returned)]
 
-use acvm::acir::circuit::Opcode;
-use acvm::compiler::CircuitSimplifier;
-use acvm::Language;
 use clap::Args;
 use fm::FileId;
 use noirc_abi::FunctionSignature;
@@ -71,11 +68,9 @@ pub type ErrorsAndWarnings = Vec<FileDiagnostic>;
 pub fn compile_file(
     context: &mut Context,
     root_file: PathBuf,
-    np_language: Language,
-    is_opcode_supported: &impl Fn(&Opcode) -> bool,
 ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
     create_local_crate(context, root_file, CrateType::Binary);
-    compile_main(context, np_language, is_opcode_supported, &CompileOptions::default())
+    compile_main(context, &CompileOptions::default())
 }
 
 /// Adds the File with the local crate root to the file system
@@ -194,8 +189,6 @@ pub fn compute_function_signature(context: &Context) -> Option<FunctionSignature
 /// On error this returns the non-empty list of warnings and errors.
 pub fn compile_main(
     context: &mut Context,
-    np_language: Language,
-    is_opcode_supported: &impl Fn(&Opcode) -> bool,
     options: &CompileOptions,
 ) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
     let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
@@ -211,8 +204,7 @@ pub fn compile_main(
         }
     };
 
-    let compiled_program =
-        compile_no_check(context, options, main, np_language, is_opcode_supported)?;
+    let compiled_program = compile_no_check(context, options, main)?;
 
     if options.print_acir {
         println!("Compiled ACIR for main:");
@@ -225,8 +217,6 @@ pub fn compile_main(
 /// Run the frontend to check the crate for errors then compile all contracts if there were none
 pub fn compile_contracts(
     context: &mut Context,
-    np_language: Language,
-    is_opcode_supported: &impl Fn(&Opcode) -> bool,
     options: &CompileOptions,
 ) -> Result<(Vec<CompiledContract>, Warnings), ErrorsAndWarnings> {
     let warnings = check_crate(context, options.deny_warnings, options.experimental_ssa)?;
@@ -236,7 +226,7 @@ pub fn compile_contracts(
     let mut errors = warnings;
 
     for contract in contracts {
-        match compile_contract(context, contract, np_language, is_opcode_supported, options) {
+        match compile_contract(context, contract, options) {
             Ok(contract) => compiled_contracts.push(contract),
             Err(mut more_errors) => errors.append(&mut more_errors),
         }
@@ -274,21 +264,13 @@ fn has_errors(errors: &[FileDiagnostic], deny_warnings: bool) -> bool {
 fn compile_contract(
     context: &Context,
     contract: Contract,
-    np_language: Language,
-    is_opcode_supported: &impl Fn(&Opcode) -> bool,
     options: &CompileOptions,
 ) -> Result<CompiledContract, Vec<FileDiagnostic>> {
     let mut functions = Vec::new();
     let mut errs = Vec::new();
     for function_id in &contract.functions {
         let name = context.function_name(function_id).to_owned();
-        let function = match compile_no_check(
-            context,
-            options,
-            *function_id,
-            np_language,
-            is_opcode_supported,
-        ) {
+        let function = match compile_no_check(context, options, *function_id) {
             Ok(function) => function,
             Err(err) => {
                 errs.push(err);
@@ -327,8 +309,6 @@ pub fn compile_no_check(
     context: &Context,
     options: &CompileOptions,
     main_function: FuncId,
-    np_language: Language,
-    is_opcode_supported: &impl Fn(&Opcode) -> bool,
 ) -> Result<CompiledProgram, FileDiagnostic> {
     let program = monomorphize(main_function, &context.def_interner);
 
@@ -338,16 +318,5 @@ pub fn compile_no_check(
         create_circuit(program, options.show_ssa, options.show_output)?
     };
 
-    let abi_len = abi.field_count();
-
-    let simplifier = CircuitSimplifier::new(abi_len);
-    let optimized_circuit =
-        acvm::compiler::compile(circuit, np_language, is_opcode_supported, &simplifier).map_err(
-            |_| FileDiagnostic {
-                file_id: FileId::dummy(),
-                diagnostic: CustomDiagnostic::from_message("produced an acvm compile error"),
-            },
-        )?;
-
-    Ok(CompiledProgram { circuit: optimized_circuit, abi })
+    Ok(CompiledProgram { circuit, abi })
 }

crates/noirc_driver/src/main.rs

@@ -1,4 +1,3 @@
-use acvm::Language;
 use noirc_driver::{
     add_dep, compile_main, create_local_crate, create_non_local_crate, CompileOptions,
 };
@@ -24,12 +23,5 @@ fn main() {
     add_dep(&mut context, LOCAL_CRATE, crate_id1, "coo4");
     add_dep(&mut context, LOCAL_CRATE, crate_id2, "coo3");
 
-    compile_main(
-        &mut context,
-        Language::R1CS,
-        #[allow(deprecated)]
-        &acvm::pwg::default_is_opcode_supported(Language::R1CS),
-        &CompileOptions::default(),
-    )
-    .ok();
+    compile_main(&mut context, &CompileOptions::default()).ok();
 }

crates/wasm/src/compile.rs

@@ -1,8 +1,9 @@
+use acvm::{acir::circuit::Circuit, compiler::CircuitSimplifier};
 use gloo_utils::format::JsValueSerdeExt;
 use log::debug;
 use noirc_driver::{
     check_crate, compile_contracts, compile_no_check, create_local_crate, create_non_local_crate,
-    propagate_dep, CompileOptions,
+    propagate_dep, CompileOptions, CompiledContract,
 };
 use noirc_frontend::{
     graph::{CrateName, CrateType},
@@ -79,8 +80,6 @@ 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 context = Context::default();
 
     let path = PathBuf::from(&options.entry_point);
@@ -93,29 +92,45 @@ pub fn compile(args: JsValue) -> JsValue {
     check_crate(&mut context, false, false).expect("Crate check failed");
 
     if options.contracts {
-        let compiled_contracts = compile_contracts(
-            &mut context,
-            language,
-            #[allow(deprecated)]
-            &acvm::pwg::default_is_opcode_supported(language),
-            &options.compile_options,
-        )
-        .expect("Contract compilation failed")
-        .0;
-
-        <JsValue as JsValueSerdeExt>::from_serde(&compiled_contracts).unwrap()
+        let compiled_contracts = compile_contracts(&mut context, &options.compile_options)
+            .expect("Contract compilation failed")
+            .0;
+
+        let optimized_contracts: Vec<CompiledContract> =
+            compiled_contracts.into_iter().map(|contract| optimize_contract(contract)).collect();
+
+        <JsValue as JsValueSerdeExt>::from_serde(&optimized_contracts).unwrap()
     } else {
         let main = context.get_main_function(&crate_id).expect("Could not find main function!");
-        let compiled_program = compile_no_check(
-            &context,
-            &options.compile_options,
-            main,
-            language,
-            #[allow(deprecated)]
-            &acvm::pwg::default_is_opcode_supported(language),
-        )
-        .expect("Compilation failed");
+        let mut compiled_program =
+            compile_no_check(&context, &options.compile_options, main).expect("Compilation failed");
+
+        compiled_program.circuit = optimize_circuit(compiled_program.circuit);
 
         <JsValue as JsValueSerdeExt>::from_serde(&compiled_program).unwrap()
     }
 }
+
+fn optimize_contract(contract: CompiledContract) -> CompiledContract {
+    CompiledContract {
+        name: contract.name,
+        functions: contract
+            .functions
+            .into_iter()
+            .map(|mut func| {
+                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")
+}