Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Perform Circuit optimization in nargo #1394

Merged
merged 16 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 25 additions & 8 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -11,9 +16,7 @@ pub fn preprocess_program<B: ProofSystemCompiler>(
common_reference_string: &[u8],
compiled_program: CompiledProgram,
) -> Result<PreprocessedProgram, B::Error> {
// 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)?;

Expand All @@ -31,9 +34,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
common_reference_string: &[u8],
func: ContractFunction,
) -> Result<PreprocessedContractFunction, B::Error> {
// 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)?;

Expand All @@ -47,3 +48,19 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
verification_key,
})
}

fn optimize_circuit(
backend: &impl ProofSystemCompiler,
circuit: Circuit,
) -> Result<Circuit, NargoError> {
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)
}
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ pub(crate) fn run<B: Backend>(
}

fn check_from_path<B: Backend, P: AsRef<Path>>(
backend: &B,
_backend: &B,
program_dir: P,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
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)?;

Expand Down
5 changes: 3 additions & 2 deletions crates/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ pub(crate) fn run<B: Backend>(
(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)?;
Expand Down
21 changes: 7 additions & 14 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub(crate) fn run<B: Backend>(

// 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)?;
Expand Down Expand Up @@ -85,7 +85,9 @@ pub(crate) fn run<B: Backend>(
);
}
} 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.
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
Expand All @@ -100,23 +102,14 @@ pub(crate) fn run<B: Backend>(
Ok(())
}

pub(super) fn setup_driver<B: Backend>(
backend: &B,
program_dir: &Path,
) -> Result<Driver, DependencyResolutionError> {
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<Driver, DependencyResolutionError> {
Resolver::resolve_root_manifest(program_dir)
}

pub(crate) fn compile_circuit<B: Backend>(
backend: &B,
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<CompiledProgram, CliError<B>> {
let mut driver = setup_driver(backend, program_dir)?;
let mut driver = setup_driver(program_dir)?;
driver.compile_main(compile_options).map_err(|_| CliError::CompilationError)
}
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn execute_with_path<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<(Option<InputValue>, WitnessMap), CliError<B>> {
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, _) =
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/gates_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn count_gates_with_path<B: Backend, P: AsRef<Path>>(
program_dir: P,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
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!(
Expand Down
6 changes: 1 addition & 5 deletions crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,7 @@ mod tests {
///
/// This is used for tests.
fn file_compiles<P: AsRef<Path>>(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()
Expand Down
4 changes: 3 additions & 1 deletion crates/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<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)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn run_tests<B: Backend>(
test_name: &str,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut driver = setup_driver(backend, program_dir)?;
let mut driver = setup_driver(program_dir)?;

driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?;

Expand Down
4 changes: 3 additions & 1 deletion crates/nargo_cli/src/cli/verify_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ fn verify_with_path<B: Backend, P: AsRef<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)?;
Expand Down
5 changes: 1 addition & 4 deletions crates/nargo_cli/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<dyn Fn(&Opcode) -> bool>,
) -> Result<Driver, DependencyResolutionError> {
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)?;
Expand Down
37 changes: 7 additions & 30 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,8 +25,6 @@ pub use program::CompiledProgram;

pub struct Driver {
context: Context,
language: Language,
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,
}

#[derive(Args, Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -67,18 +63,14 @@ impl Default for CompileOptions {
}

impl Driver {
pub fn new(language: &Language, is_opcode_supported: Box<dyn Fn(&Opcode) -> 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<dyn Fn(&Opcode) -> bool>,
) -> Result<CompiledProgram, ReportedError> {
let mut driver = Driver::new(language, is_opcode_supported);
pub fn compile_file(root_file: PathBuf) -> Result<CompiledProgram, ReportedError> {
let mut driver = Driver::new();
driver.create_local_crate(root_file, CrateType::Binary);
driver.compile_main(&CompileOptions::default())
}
Expand Down Expand Up @@ -283,24 +275,10 @@ impl Driver {
) -> Result<CompiledProgram, ReportedError> {
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 {
Expand Down Expand Up @@ -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()
}
}
8 changes: 2 additions & 6 deletions crates/noirc_driver/src/main.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
25 changes: 7 additions & 18 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand All @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions crates/noirc_evaluator/src/ssa_refactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down
Loading