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

feat: remove query to backend to get expression width #4975

Merged
merged 9 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ pub const NOIR_ARTIFACT_VERSION_STRING: &str =
#[derive(Args, Clone, Debug, Default)]
pub struct CompileOptions {
/// Override the expression width requested by the backend.
#[arg(long, value_parser = parse_expression_width)]
pub expression_width: Option<ExpressionWidth>,
#[arg(long, value_parser = parse_expression_width, default_value = "3")]
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
pub expression_width: ExpressionWidth,

/// Force a full recompilation.
#[arg(long = "force")]
Expand Down
1 change: 0 additions & 1 deletion tooling/backend_interface/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ license.workspace = true
acvm.workspace = true
dirs.workspace = true
thiserror.workspace = true
serde.workspace = true
serde_json.workspace = true
bb_abstraction_leaks.workspace = true
tracing.workspace = true
Expand Down
62 changes: 0 additions & 62 deletions tooling/backend_interface/src/cli/info.rs

This file was deleted.

2 changes: 0 additions & 2 deletions tooling/backend_interface/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

mod contract;
mod gates;
mod info;
mod proof_as_fields;
mod prove;
mod verify;
Expand All @@ -12,7 +11,6 @@ mod write_vk;

pub(crate) use contract::ContractCommand;
pub(crate) use gates::GatesCommand;
pub(crate) use info::InfoCommand;
pub(crate) use proof_as_fields::ProofAsFieldsCommand;
pub(crate) use prove::ProveCommand;
pub(crate) use verify::VerifyCommand;
Expand Down
25 changes: 3 additions & 22 deletions tooling/backend_interface/src/proof_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ use std::io::Write;
use std::path::Path;

use acvm::acir::{
circuit::{ExpressionWidth, Program},
circuit::Program,
native_types::{WitnessMap, WitnessStack},
};
use acvm::FieldElement;
use tempfile::tempdir;
use tracing::warn;

use crate::cli::{
GatesCommand, InfoCommand, ProofAsFieldsCommand, ProveCommand, VerifyCommand,
VkAsFieldsCommand, WriteVkCommand,
GatesCommand, ProofAsFieldsCommand, ProveCommand, VerifyCommand, VkAsFieldsCommand,
WriteVkCommand,
};
use crate::{Backend, BackendError};

Expand All @@ -33,25 +33,6 @@ impl Backend {
.run(binary_path)
}

pub fn get_backend_info(&self) -> Result<ExpressionWidth, BackendError> {
let binary_path = self.assert_binary_exists()?;
self.assert_correct_version()?;
InfoCommand { crs_path: self.crs_directory() }.run(binary_path)
}

/// If we cannot get a valid backend, returns `ExpressionWidth::Bound { width: 4 }``
/// The function also prints a message saying we could not find a backend
pub fn get_backend_info_or_default(&self) -> ExpressionWidth {
if let Ok(expression_width) = self.get_backend_info() {
expression_width
} else {
warn!(
"No valid backend found, ExpressionWidth defaulting to Bounded with a width of 4"
);
ExpressionWidth::Bounded { width: 4 }
}
}

#[tracing::instrument(level = "trace", skip_all)]
pub fn prove(
&self,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use clap::{Parser, Subcommand};

mod contract_cmd;
mod gates_cmd;
mod info_cmd;
mod prove_cmd;
mod verify_cmd;
mod write_vk_cmd;
Expand All @@ -21,7 +20,6 @@ struct BackendCli {

#[derive(Subcommand, Clone, Debug)]
enum BackendCommand {
Info(info_cmd::InfoCommand),
Contract(contract_cmd::ContractCommand),
Gates(gates_cmd::GatesCommand),
Prove(prove_cmd::ProveCommand),
Expand All @@ -34,7 +32,6 @@ fn main() {
let BackendCli { command } = BackendCli::parse();

match command {
BackendCommand::Info(args) => info_cmd::run(args),
BackendCommand::Contract(args) => contract_cmd::run(args),
BackendCommand::Gates(args) => gates_cmd::run(args),
BackendCommand::Prove(args) => prove_cmd::run(args),
Expand Down
7 changes: 1 addition & 6 deletions tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::backends::Backend;
use crate::errors::CliError;

use clap::Args;
Expand Down Expand Up @@ -42,11 +41,7 @@ pub(crate) struct CheckCommand {
compile_options: CompileOptions,
}

pub(crate) fn run(
_backend: &Backend,
args: CheckCommand,
config: NargoConfig,
) -> Result<(), CliError> {
pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliError> {
let toml_path = get_package_manifest(&config.program_dir)?;
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
Expand Down
4 changes: 2 additions & 2 deletions tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::fs::{create_named_dir, write_to_file};
use super::NargoConfig;
use crate::backends::Backend;
use crate::cli::compile_cmd::DEFAULT_EXPRESSION_WIDTH;
use crate::errors::CliError;

use clap::Args;
Expand Down Expand Up @@ -44,7 +45,6 @@ pub(crate) fn run(
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

let expression_width = backend.get_backend_info()?;
let binary_packages = workspace.into_iter().filter(|package| package.is_binary());
for package in binary_packages {
let compilation_result = compile_program(
Expand All @@ -62,7 +62,7 @@ pub(crate) fn run(
args.compile_options.silence_warnings,
)?;

let program = nargo::ops::transform_program(program, expression_width);
let program = nargo::ops::transform_program(program, DEFAULT_EXPRESSION_WIDTH);

// TODO(https://github.com/noir-lang/noir/issues/4428):
// We do not expect to have a smart contract verifier for a foldable program with multiple circuits.
Expand Down
20 changes: 6 additions & 14 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use std::path::Path;
use std::time::Duration;

use acvm::acir::circuit::ExpressionWidth;
use fm::FileManager;
use nargo::artifacts::program::ProgramArtifact;
use nargo::ops::{collect_errors, compile_contract, compile_program, report_errors};
Expand All @@ -20,14 +21,15 @@
use notify::{EventKind, RecursiveMode, Watcher};
use notify_debouncer_full::new_debouncer;

use crate::backends::Backend;
use crate::errors::CliError;

use super::fs::program::only_acir;
use super::fs::program::{read_program_from_file, save_contract_to_file, save_program_to_file};
use super::NargoConfig;
use rayon::prelude::*;

pub(super) const DEFAULT_EXPRESSION_WIDTH: ExpressionWidth = ExpressionWidth::Bounded { width: 4 };

/// Compile the program and its secret execution trace into ACIR format
#[derive(Debug, Clone, Args)]
pub(crate) struct CompileCommand {
Expand All @@ -47,11 +49,7 @@
watch: bool,
}

pub(crate) fn run(
backend: &Backend,
mut args: CompileCommand,
config: NargoConfig,
) -> Result<(), CliError> {
pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliError> {
let toml_path = get_package_manifest(&config.program_dir)?;
let default_selection =
if args.workspace { PackageSelection::All } else { PackageSelection::DefaultOrAll };
Expand All @@ -63,10 +61,6 @@
Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()),
)?;

if args.compile_options.expression_width.is_none() {
args.compile_options.expression_width = Some(backend.get_backend_info_or_default());
};

if args.watch {
watch_workspace(&workspace, &args.compile_options)
.map_err(|err| CliError::Generic(err.to_string()))?;
Expand All @@ -80,7 +74,7 @@
fn watch_workspace(workspace: &Workspace, compile_options: &CompileOptions) -> notify::Result<()> {
let (tx, rx) = std::sync::mpsc::channel();

// No specific tickrate, max debounce time 1 seconds

Check warning on line 77 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (tickrate)
let mut debouncer = new_debouncer(Duration::from_secs(1), None, tx)?;

// Add a path to be watched. All files and directories at that path and
Expand All @@ -88,7 +82,7 @@
debouncer.watcher().watch(&workspace.root_dir, RecursiveMode::Recursive)?;

let mut screen = std::io::stdout();
write!(screen, "{}", termion::cursor::Save).unwrap();

Check warning on line 85 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)
screen.flush().unwrap();
let _ = compile_workspace_full(workspace, compile_options);
for res in rx {
Expand All @@ -109,7 +103,7 @@
});

if noir_files_modified {
write!(screen, "{}{}", termion::cursor::Restore, termion::clear::AfterCursor).unwrap();

Check warning on line 106 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)

Check warning on line 106 in tooling/nargo_cli/src/cli/compile_cmd.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (termion)
screen.flush().unwrap();
let _ = compile_workspace_full(workspace, compile_options);
}
Expand All @@ -128,8 +122,6 @@
insert_all_files_for_workspace_into_file_manager(workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

let expression_width =
compile_options.expression_width.expect("expression width should have been set");
let compiled_workspace =
compile_workspace(&workspace_file_manager, &parsed_files, workspace, compile_options);

Expand All @@ -149,12 +141,12 @@
// Save build artifacts to disk.
let only_acir = compile_options.only_acir;
for (package, program) in binary_packages.into_iter().zip(compiled_programs) {
let program = nargo::ops::transform_program(program, expression_width);
let program = nargo::ops::transform_program(program, compile_options.expression_width);
save_program(program.clone(), &package, &workspace.target_directory_path(), only_acir);
}
let circuit_dir = workspace.target_directory_path();
for (package, contract) in contract_packages.into_iter().zip(compiled_contracts) {
let contract = nargo::ops::transform_contract(contract, expression_width);
let contract = nargo::ops::transform_contract(contract, compile_options.expression_width);
save_contract(contract, &package, &circuit_dir);
}

Expand Down
18 changes: 5 additions & 13 deletions tooling/nargo_cli/src/cli/dap_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use acvm::acir::circuit::ExpressionWidth;
use acvm::acir::native_types::WitnessMap;
use backend_interface::Backend;
use clap::Args;
use nargo::constants::PROVER_INPUT_FILE;
use nargo::workspace::Workspace;
Expand Down Expand Up @@ -29,8 +28,8 @@ use noir_debugger::errors::{DapError, LoadError};
#[derive(Debug, Clone, Args)]
pub(crate) struct DapCommand {
/// Override the expression width requested by the backend.
#[arg(long, value_parser = parse_expression_width)]
expression_width: Option<ExpressionWidth>,
#[arg(long, value_parser = parse_expression_width, default_value = "4")]
expression_width: ExpressionWidth,

#[clap(long)]
preflight_check: bool,
Expand Down Expand Up @@ -249,14 +248,7 @@ fn run_preflight_check(
Ok(())
}

pub(crate) fn run(
backend: &Backend,
args: DapCommand,
_config: NargoConfig,
) -> Result<(), CliError> {
let expression_width =
args.expression_width.unwrap_or_else(|| backend.get_backend_info_or_default());

pub(crate) fn run(args: DapCommand, _config: NargoConfig) -> Result<(), CliError> {
// When the --preflight-check flag is present, we run Noir's DAP server in "pre-flight mode", which test runs
// the DAP initialization code without actually starting the DAP server.
//
Expand All @@ -270,12 +262,12 @@ pub(crate) fn run(
// the DAP loop is established, which otherwise are considered "out of band" by the maintainers of the DAP spec.
// More details here: https://github.com/microsoft/vscode/issues/108138
if args.preflight_check {
return run_preflight_check(expression_width, args).map_err(CliError::DapError);
return run_preflight_check(args.expression_width, args).map_err(CliError::DapError);
}

let output = BufWriter::new(std::io::stdout());
let input = BufReader::new(std::io::stdin());
let server = Server::new(input, output);

loop_uninitialized_dap(server, expression_width).map_err(CliError::DapError)
loop_uninitialized_dap(server, args.expression_width).map_err(CliError::DapError)
}
Loading
Loading