Skip to content

Commit

Permalink
feat: pass in closure to Driver to signal backend opcode support (#…
Browse files Browse the repository at this point in the history
…1349)

* feat: pass in opcode support function

* chore: remove langauge from `NodeInterner`

* chore: add comment and replace non-standard `is_opcode_supported`s

* chore: prefer usage of `impl` over `dyn`

* chore: fix build
  • Loading branch information
TomAFrench committed May 17, 2023
1 parent dffa3c5 commit 1e958c2
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 24 deletions.
7 changes: 6 additions & 1 deletion crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ fn check_from_path<B: Backend, P: AsRef<Path>>(
p: P,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut driver = Resolver::resolve_root_manifest(p.as_ref(), backend.np_language())?;
let mut driver = Resolver::resolve_root_manifest(
p.as_ref(),
backend.np_language(),
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(backend.np_language())),
)?;

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

Expand Down
7 changes: 6 additions & 1 deletion crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ fn setup_driver<B: Backend>(
backend: &B,
program_dir: &Path,
) -> Result<Driver, DependencyResolutionError> {
Resolver::resolve_root_manifest(program_dir, backend.np_language())
Resolver::resolve_root_manifest(
program_dir,
backend.np_language(),
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(backend.np_language())),
)
}

pub(crate) fn compile_circuit<B: Backend>(
Expand Down
6 changes: 5 additions & 1 deletion crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ 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);
let mut driver = Driver::new(
&acvm::Language::R1CS,
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)),
);
driver.create_local_crate(&root_file, CrateType::Binary);
crate::resolver::add_std_lib(&mut driver);
driver.file_compiles()
Expand Down
7 changes: 6 additions & 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,12 @@ fn run_tests<B: Backend>(
test_name: &str,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut driver = Resolver::resolve_root_manifest(program_dir, backend.np_language())?;
let mut driver = Resolver::resolve_root_manifest(
program_dir,
backend.np_language(),
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(backend.np_language())),
)?;

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

Expand Down
5 changes: 3 additions & 2 deletions crates/nargo_cli/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
path::{Path, PathBuf},
};

use acvm::Language;
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 @@ -71,8 +71,9 @@ impl<'a> Resolver<'a> {
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);
let mut driver = Driver::new(&np_language, is_opcode_supported);
let (entry_path, crate_type) = super::lib_or_bin(dir_path)?;

let manifest_path = super::find_package_manifest(dir_path)?;
Expand Down
16 changes: 10 additions & 6 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

use acvm::acir::circuit::Opcode;
use acvm::Language;
use clap::Args;
use contract::ContractFunction;
Expand Down Expand Up @@ -66,19 +67,20 @@ impl Default for CompileOptions {
}

impl Driver {
pub fn new(np_language: &Language) -> Self {
let mut driver = Driver { context: Context::default(), language: np_language.clone() };
driver.context.def_interner.set_language(np_language);
pub fn new(language: &Language, is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>) -> Self {
let mut driver = Driver { context: Context::default(), language: language.clone() };
driver.context.def_interner.set_opcode_support(is_opcode_supported);
driver
}

// This is here for backwards compatibility
// with the restricted version which only uses one file
pub fn compile_file(
root_file: PathBuf,
np_language: acvm::Language,
language: &Language,
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,
) -> Result<CompiledProgram, ReportedError> {
let mut driver = Driver::new(&np_language);
let mut driver = Driver::new(language, is_opcode_supported);
driver.create_local_crate(root_file, CrateType::Binary);
driver.compile_main(&CompileOptions::default())
}
Expand Down Expand Up @@ -284,6 +286,7 @@ impl Driver {
let program = monomorphize(main_function, &self.context.def_interner);

let np_language = self.language.clone();
// TODO: use proper `is_opcode_supported` implementation.
let is_opcode_supported = acvm::default_is_opcode_supported(np_language.clone());

let circuit_abi = if options.experimental_ssa {
Expand Down Expand Up @@ -346,6 +349,7 @@ impl Driver {

impl Default for Driver {
fn default() -> Self {
Self::new(&Language::R1CS)
#[allow(deprecated)]
Self::new(&Language::R1CS, Box::new(acvm::default_is_opcode_supported(Language::R1CS)))
}
}
7 changes: 6 additions & 1 deletion crates/noirc_driver/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
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(&acvm::Language::R1CS);
let mut driver = Driver::new(
&Language::R1CS,
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(Language::R1CS)),
);

// Add local crate to dep graph
driver.create_local_crate(ROOT_DIR_MAIN, CrateType::Binary);
Expand Down
10 changes: 7 additions & 3 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub mod type_check;

use crate::graph::{CrateGraph, CrateId};
use crate::node_interner::NodeInterner;
use acvm::Language;
use acvm::acir::circuit::Opcode;
use def_map::CrateDefMap;
use fm::FileManager;
use std::collections::HashMap;
Expand All @@ -29,15 +29,19 @@ pub struct Context {
pub type StorageSlot = u32;

impl Context {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph, language: Language) -> Context {
pub fn new(
file_manager: FileManager,
crate_graph: CrateGraph,
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,
) -> Context {
let mut ctx = Context {
def_interner: NodeInterner::default(),
def_maps: HashMap::new(),
crate_graph,
file_manager,
storage_slots: HashMap::new(),
};
ctx.def_interner.set_language(&language);
ctx.def_interner.set_opcode_support(is_opcode_supported);
ctx
}

Expand Down
7 changes: 6 additions & 1 deletion crates/noirc_frontend/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ fn main() {
let crate_id = crate_graph.add_crate_root(CrateType::Library, root_file_id);

// initiate context with file manager and crate graph
let mut context = Context::new(fm, crate_graph, acvm::Language::R1CS);
let mut context = Context::new(
fm,
crate_graph,
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)),
);

// Now create the CrateDefMap
// This is preamble for analysis
Expand Down
12 changes: 6 additions & 6 deletions crates/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub struct NodeInterner {
next_type_variable_id: usize,

//used for fallback mechanism
language: Language,
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,

delayed_type_checks: Vec<TypeCheckFn>,

Expand Down Expand Up @@ -258,7 +258,8 @@ impl Default for NodeInterner {
field_indices: HashMap::new(),
next_type_variable_id: 0,
globals: HashMap::new(),
language: Language::R1CS,
#[allow(deprecated)]
is_opcode_supported: Box::new(acvm::default_is_opcode_supported(Language::R1CS)),
delayed_type_checks: vec![],
struct_methods: HashMap::new(),
primitive_methods: HashMap::new(),
Expand Down Expand Up @@ -579,18 +580,17 @@ impl NodeInterner {
self.function_definition_ids[&function]
}

pub fn set_language(&mut self, language: &Language) {
self.language = language.clone();
pub fn set_opcode_support(&mut self, is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>) {
self.is_opcode_supported = is_opcode_supported;
}

#[allow(deprecated)]
pub fn foreign(&self, opcode: &str) -> bool {
let is_supported = acvm::default_is_opcode_supported(self.language.clone());
let black_box_func = match acvm::acir::BlackBoxFunc::lookup(opcode) {
Some(black_box_func) => black_box_func,
None => return false,
};
is_supported(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall {
(self.is_opcode_supported)(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall {
name: black_box_func,
inputs: Vec::new(),
outputs: Vec::new(),
Expand Down
6 changes: 5 additions & 1 deletion crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ pub fn compile(args: JsValue) -> JsValue {

// 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);
let mut driver = noirc_driver::Driver::new(
&language,
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(language.clone())),
);

let path = PathBuf::from(&options.entry_point);
driver.create_local_crate(path, CrateType::Binary);
Expand Down

0 comments on commit 1e958c2

Please sign in to comment.