Skip to content

Commit

Permalink
feat: Update to acvm 0.22.0 (#2363)
Browse files Browse the repository at this point in the history
Co-authored-by: kevaundray <[email protected]>
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
3 people authored Aug 21, 2023
1 parent 1432b01 commit e050fab
Show file tree
Hide file tree
Showing 8 changed files with 295 additions and 275 deletions.
453 changes: 237 additions & 216 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ edition = "2021"
rust-version = "1.66"

[workspace.dependencies]
acvm = "0.21.0"
acvm = "0.22.0"
arena = { path = "crates/arena" }
fm = { path = "crates/fm" }
iter-extended = { path = "crates/iter-extended" }
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ tokio = { version = "1.0", features = ["io-std"] }
tokio-util = { version = "0.7.8", features = ["compat"] }

# Backends
acvm-backend-barretenberg = { version = "0.10.0", default-features = false }
acvm-backend-barretenberg = { version = "0.11.0", default-features = false }

[dev-dependencies]
tempdir = "0.3.7"
Expand Down
28 changes: 6 additions & 22 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use acvm::acir::circuit::OpcodeLabel;
use acvm::{acir::circuit::Circuit, Backend};
use acvm::{acir::circuit::Circuit, compiler::AcirTransformationMap, Backend};
use iter_extended::try_vecmap;
use iter_extended::vecmap;
use nargo::artifacts::debug::DebugArtifact;
use nargo::package::Package;
use nargo::prepare_package;
Expand Down Expand Up @@ -171,27 +169,20 @@ pub(crate) fn compile_package<B: Backend>(
let result = compile_main(&mut context, crate_id, compile_options);
let mut program = report_errors(result, &context, compile_options.deny_warnings)?;
// Apply backend specific optimizations.
let (optimized_circuit, opcode_labels) = optimize_circuit(backend, program.circuit)
let (optimized_circuit, location_map) = optimize_circuit(backend, program.circuit)
.expect("Backend does not support an opcode that is in the IR");

// TODO(#2110): Why does this set `program.circuit` to `optimized_circuit` instead of the function taking ownership
// and requiring we use `optimized_circuit` everywhere after
program.circuit = optimized_circuit;
let opcode_ids = vecmap(opcode_labels, |label| match label {
OpcodeLabel::Unresolved => {
unreachable!("Compiled circuit opcodes must resolve to some index")
}
OpcodeLabel::Resolved(index) => index as usize,
});
program.debug.update_acir(opcode_ids);
program.debug.update_acir(location_map);

Ok((context, program))
}

pub(super) fn optimize_circuit<B: Backend>(
backend: &B,
circuit: Circuit,
) -> Result<(Circuit, Vec<OpcodeLabel>), CliError<B>> {
) -> Result<(Circuit, AcirTransformationMap), CliError<B>> {
let result = acvm::compiler::compile(circuit, backend.np_language(), |opcode| {
backend.supports_opcode(opcode)
})
Expand All @@ -205,16 +196,9 @@ pub(super) fn optimize_contract<B: Backend>(
contract: CompiledContract,
) -> Result<CompiledContract, CliError<B>> {
let functions = try_vecmap(contract.functions, |mut func| {
let (optimized_bytecode, opcode_labels) = optimize_circuit(backend, func.bytecode)?;
let opcode_ids = vecmap(opcode_labels, |label| match label {
OpcodeLabel::Unresolved => {
unreachable!("Compiled circuit opcodes must resolve to some index")
}
OpcodeLabel::Resolved(index) => index as usize,
});

let (optimized_bytecode, location_map) = optimize_circuit(backend, func.bytecode)?;
func.bytecode = optimized_bytecode;
func.debug.update_acir(opcode_ids);
func.debug.update_acir(location_map);
Ok::<_, CliError<B>>(func)
})?;

Expand Down
31 changes: 17 additions & 14 deletions crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use acvm::acir::circuit::OpcodeLabel;
use acvm::acir::circuit::OpcodeLocation;
use acvm::acir::{circuit::Circuit, native_types::WitnessMap};
use acvm::pwg::ErrorLocation;
use acvm::Backend;
use clap::Args;
use nargo::constants::PROVER_INPUT_FILE;
Expand Down Expand Up @@ -90,32 +91,34 @@ fn execute_package<B: Backend>(
Ok((return_value, solved_witness))
}

fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Option<usize> {
fn extract_unsatisfied_constraint_from_nargo_error(
nargo_err: &NargoError,
) -> Option<OpcodeLocation> {
let solving_err = match nargo_err {
nargo::NargoError::SolvingError(err) => err,
_ => return None,
};

match solving_err {
acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_label } => {
match opcode_label {
OpcodeLabel::Unresolved => {
unreachable!("Cannot resolve index for unsatisfied constraint")
}
OpcodeLabel::Resolved(opcode_index) => Some(*opcode_index as usize),
acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain {
opcode_location: error_location,
} => match error_location {
ErrorLocation::Unresolved => {
unreachable!("Cannot resolve index for unsatisfied constraint")
}
}
ErrorLocation::Resolved(opcode_location) => Some(*opcode_location),
},
_ => None,
}
}

fn report_unsatisfied_constraint_error(
opcode_idx: Option<usize>,
opcode_location: Option<OpcodeLocation>,
debug: &DebugInfo,
context: &Context,
) {
if let Some(opcode_index) = opcode_idx {
if let Some(locations) = debug.opcode_location(opcode_index) {
if let Some(opcode_location) = opcode_location {
if let Some(locations) = debug.opcode_location(&opcode_location) {
// The location of the error itself will be the location at the top
// of the call stack (the last item in the Vec).
if let Some(location) = locations.last() {
Expand All @@ -142,8 +145,8 @@ pub(crate) fn execute_program<B: Backend>(
Ok(solved_witness) => Ok(solved_witness),
Err(err) => {
if let Some((debug, context)) = debug_data {
let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err);
report_unsatisfied_constraint_error(opcode_idx, &debug, &context);
let opcode_location = extract_unsatisfied_constraint_from_nargo_error(&err);
report_unsatisfied_constraint_error(opcode_location, &debug, &context);
}

Err(crate::errors::CliError::NargoError(err))
Expand Down
4 changes: 3 additions & 1 deletion crates/noirc_errors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ edition.workspace = true
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
acvm.workspace = true
codespan-reporting.workspace = true
codespan.workspace = true
fm.workspace = true
chumsky.workspace = true
serde.workspace = true
serde.workspace = true
serde_with = "3.2.0"
44 changes: 26 additions & 18 deletions crates/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,39 +1,47 @@
use acvm::acir::circuit::OpcodeLocation;
use acvm::compiler::AcirTransformationMap;

use serde_with::serde_as;
use serde_with::DisplayFromStr;
use std::collections::BTreeMap;

use crate::Location;
use serde::{Deserialize, Serialize};

#[serde_as]
#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
/// Map opcode index of an ACIR circuit into the source code location
pub locations: BTreeMap<usize, Vec<Location>>,
/// Serde does not support mapping keys being enums for json, so we indicate
/// that they should be serialized to/from strings.
#[serde_as(as = "BTreeMap<DisplayFromStr, _>")]
pub locations: BTreeMap<OpcodeLocation, Vec<Location>>,
}

impl DebugInfo {
pub fn new(locations: BTreeMap<usize, Vec<Location>>) -> Self {
pub fn new(locations: BTreeMap<OpcodeLocation, Vec<Location>>) -> Self {
DebugInfo { locations }
}

/// Updates the locations map when the circuit is modified
/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
///
/// When the circuit is generated, the indices are 0,1,..,n
/// When the circuit is modified, the opcodes are eventually
/// mixed, removed, or with new ones. For instance 5,2,6,n+1,0,12,..
/// Since new opcodes (n+1 in the ex) don't have a location
/// we use the index of the old opcode that they replace.
/// This is the case during fallback or width 'optimization'
/// opcode_indices is this list of mixed indices
pub fn update_acir(&mut self, opcode_indices: Vec<usize>) {
let mut new_locations = BTreeMap::new();
for (i, idx) in opcode_indices.iter().enumerate() {
if self.locations.contains_key(idx) {
new_locations.insert(i, self.locations[idx].clone());
/// The [`OpcodeLocation`]s are generated with the ACIR, but passing the ACIR through a transformation step
/// renders the old `OpcodeLocation`s invalid. The AcirTransformationMap is able to map the old `OpcodeLocation` to the new ones.
/// Note: One old `OpcodeLocation` might have transformed into more than one new `OpcodeLocation`.
pub fn update_acir(&mut self, update_map: AcirTransformationMap) {
let mut new_locations_map = BTreeMap::new();

for (old_opcode_location, source_locations) in &self.locations {
let new_opcode_locations = update_map.new_locations(*old_opcode_location);
for new_opcode_location in new_opcode_locations {
new_locations_map.insert(new_opcode_location, source_locations.clone());
}
}
self.locations = new_locations;

self.locations = new_locations_map;
}

pub fn opcode_location(&self, idx: usize) -> Option<Vec<Location>> {
self.locations.get(&idx).cloned()
pub fn opcode_location(&self, loc: &OpcodeLocation) -> Option<Vec<Location>> {
self.locations.get(loc).cloned()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use acvm::acir::{
circuit::{
brillig::{Brillig as AcvmBrillig, BrilligInputs, BrilligOutputs},
opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode},
OpcodeLocation,
},
native_types::Witness,
BlackBoxFunc,
Expand Down Expand Up @@ -45,7 +46,7 @@ pub(crate) struct GeneratedAcir {
pub(crate) input_witnesses: Vec<Witness>,

/// Correspondance between an opcode index (in opcodes) and the source code call stack which generated it
pub(crate) locations: BTreeMap<usize, CallStack>,
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,

/// Source code location of the current instruction being processed
/// None if we do not know the location
Expand All @@ -62,7 +63,8 @@ impl GeneratedAcir {
fn push_opcode(&mut self, opcode: AcirOpcode) {
self.opcodes.push(opcode);
if !self.call_stack.is_empty() {
self.locations.insert(self.opcodes.len() - 1, self.call_stack.clone());
self.locations
.insert(OpcodeLocation::Acir(self.opcodes.len() - 1), self.call_stack.clone());
}
}

Expand Down

0 comments on commit e050fab

Please sign in to comment.