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: optionally output a debug artifact on compile #2260

Merged
merged 8 commits into from
Aug 14, 2023
4 changes: 3 additions & 1 deletion crates/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ impl From<&PathBuf> for PathString {
pub struct FileMap(SimpleFiles<PathString, String>);

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)]
#[derive(
Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize, PartialOrd, Ord,
)]
pub struct FileId(usize);

impl FileId {
Expand Down
2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
assert!(old_value.is_none(), "ice: the same path was inserted into the file manager twice");
}

pub fn fetch_file(&mut self, file_id: FileId) -> File {
pub fn fetch_file(&self, file_id: FileId) -> File {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
Expand Down
47 changes: 47 additions & 0 deletions crates/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet};

use fm::FileId;
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::hir::Context;

#[derive(Debug, Serialize, Deserialize)]
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
pub struct DebugFile {
pub source: String,
pub path: String,
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
}

#[derive(Debug, Serialize, Deserialize)]
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
pub struct DebugArtifact {
pub debug_symbols: Vec<DebugInfo>,
pub file_map: BTreeMap<FileId, DebugFile>,
}

impl DebugArtifact {
pub fn new(debug_symbols: Vec<DebugInfo>, compilation_context: &Context) -> Self {
let mut file_map = BTreeMap::new();

let files_with_debug_symbols: BTreeSet<FileId> = debug_symbols
.iter()
.flat_map(|function_symbols| function_symbols.locations.values().map(|loc| loc.file))
.collect();

for file_id in files_with_debug_symbols {
let file_source = compilation_context.file_manager.fetch_file(file_id).source();

file_map.insert(
file_id,
DebugFile {
source: file_source.to_string(),
path: compilation_context
.file_manager
.path(file_id)
.to_string_lossy()
.to_string(),
},
);
}

Self { debug_symbols, file_map }
}
}
1 change: 1 addition & 0 deletions crates/nargo/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use base64::Engine;
use serde::{Deserializer, Serializer};

pub mod contract;
pub mod debug;
pub mod program;

// TODO: move these down into ACVM.
Expand Down
23 changes: 13 additions & 10 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
include_keys: bool,
common_reference_string: &[u8],
func: ContractFunction,
) -> Result<PreprocessedContractFunction, B::Error> {
) -> Result<(PreprocessedContractFunction, DebugInfo), B::Error> {
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
// 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;
Expand All @@ -54,14 +54,17 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
(None, None)
};

Ok(PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,
Ok((
PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,

bytecode: optimized_bytecode,
proving_key,
verification_key,
})
bytecode: optimized_bytecode,
proving_key,
verification_key,
},
func.debug,
))
}
97 changes: 68 additions & 29 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ use acvm::acir::circuit::OpcodeLabel;
use acvm::{acir::circuit::Circuit, Backend};
use iter_extended::try_vecmap;
use iter_extended::vecmap;
use nargo::artifacts::debug::DebugArtifact;
use nargo::package::Package;
use nargo::prepare_package;
use nargo::{artifacts::contract::PreprocessedContract, NargoError};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use noirc_driver::{
compile_contracts, compile_main, CompileOptions, CompiledContract, CompiledProgram,
ContractFunction, ErrorsAndWarnings, Warnings,
ErrorsAndWarnings, Warnings,
};
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::graph::CrateName;
use noirc_frontend::hir::Context;

Expand All @@ -19,6 +21,7 @@ use nargo::ops::{preprocess_contract_function, preprocess_program};

use crate::errors::{CliError, CompileError};

use super::fs::program::save_debug_artifact_to_file;
use super::fs::{
common_reference_string::{
read_cached_common_reference_string, update_common_reference_string,
Expand All @@ -38,6 +41,10 @@ pub(crate) struct CompileCommand {
#[arg(long)]
include_keys: bool,

/// Output debug files
#[arg(long, hide = true)]
output_debug: bool,

/// The name of the package to compile
#[clap(long)]
package: Option<CrateName>,
Expand Down Expand Up @@ -70,49 +77,72 @@ pub(crate) fn run<B: Backend>(
// As can be seen here, It seems like a leaky abstraction where ContractFunctions (essentially CompiledPrograms)
// are compiled via nargo-core and then the PreprocessedContract is constructed here.
// 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(optimized_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)
})?;

Ok(PreprocessedContract {
let preprocessed_contracts: Result<
Vec<(PreprocessedContract, Vec<DebugInfo>)>,
CliError<B>,
> = try_vecmap(optimized_contracts, |contract| {
let preprocess_result = 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, debug_infos): (Vec<_>, Vec<_>) =
preprocess_result.into_iter().unzip();

Ok((
PreprocessedContract {
name: contract.name,
backend: String::from(BACKEND_IDENTIFIER),
functions: preprocessed_contract_functions,
})
});
for contract in preprocessed_contracts? {
},
debug_infos,
))
});
for (contract, debug_infos) in preprocessed_contracts? {
save_contract_to_file(
&contract,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);

if args.output_debug {
let debug_artifact = DebugArtifact::new(debug_infos, &context);
save_debug_artifact_to_file(
&debug_artifact,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);
}
}
} else {
let (_, program) = compile_package(backend, package, &args.compile_options)?;
let (context, program) = compile_package(backend, package, &args.compile_options)?;

common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;

let (preprocessed_program, _) =
let (preprocessed_program, debug_info) =
preprocess_program(backend, args.include_keys, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
save_program_to_file(&preprocessed_program, &package.name, &circuit_dir);

if args.output_debug {
let debug_artifact = DebugArtifact::new(vec![debug_info], &context);
let circuit_name: String = (&package.name).into();
save_debug_artifact_to_file(&debug_artifact, &circuit_name, &circuit_dir);
}
}
}

Expand Down Expand Up @@ -167,9 +197,18 @@ pub(super) fn optimize_contract<B: Backend>(
backend: &B,
contract: CompiledContract,
) -> Result<CompiledContract, CliError<B>> {
let functions = try_vecmap(contract.functions, |func| {
let optimized_bytecode = optimize_circuit(backend, func.bytecode)?.0;
Ok::<_, CliError<B>>(ContractFunction { bytecode: optimized_bytecode, ..func })
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,
});

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

Ok(CompiledContract { functions, ..contract })
Expand Down
19 changes: 16 additions & 3 deletions crates/nargo_cli/src/cli/fs/program.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::path::{Path, PathBuf};

use nargo::artifacts::{contract::PreprocessedContract, program::PreprocessedProgram};
use nargo::artifacts::{
contract::PreprocessedContract, debug::DebugArtifact, program::PreprocessedProgram,
};
use noirc_frontend::graph::CrateName;

use crate::errors::FilesystemError;
Expand All @@ -15,20 +17,31 @@ pub(crate) fn save_program_to_file<P: AsRef<Path>>(
let circuit_name: String = crate_name.into();
save_build_artifact_to_file(compiled_program, &circuit_name, circuit_dir)
}

pub(crate) fn save_contract_to_file<P: AsRef<Path>>(
compiled_contract: &PreprocessedContract,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
save_build_artifact_to_file(compiled_contract, circuit_name, circuit_dir)
}

pub(crate) fn save_debug_artifact_to_file<P: AsRef<Path>>(
debug_artifact: &DebugArtifact,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
let artifact_name = format!("debug_{}", circuit_name);
save_build_artifact_to_file(debug_artifact, &artifact_name, circuit_dir)
}

fn save_build_artifact_to_file<P: AsRef<Path>, T: ?Sized + serde::Serialize>(
build_artifact: &T,
circuit_name: &str,
artifact_name: &str,
circuit_dir: P,
) -> PathBuf {
create_named_dir(circuit_dir.as_ref(), "target");
let circuit_path = circuit_dir.as_ref().join(circuit_name).with_extension("json");
let circuit_path = circuit_dir.as_ref().join(artifact_name).with_extension("json");

write_to_file(&serde_json::to_vec(build_artifact).unwrap(), &circuit_path);

Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::program::{deserialize_circuit, serialize_circuit};
use acvm::acir::circuit::Circuit;
use noirc_abi::Abi;
use noirc_errors::debug_info::DebugInfo;
use serde::{Deserialize, Serialize};

/// Describes the types of smart contract functions that are allowed.
Expand Down Expand Up @@ -47,6 +48,8 @@ pub struct ContractFunction {

#[serde(serialize_with = "serialize_circuit", deserialize_with = "deserialize_circuit")]
pub bytecode: Circuit,

pub debug: DebugInfo,
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
}

impl ContractFunctionType {
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
let compiled_program = compile_no_check(context, options, main)?;

if options.print_acir {
println!("Compiled ACIR for main (unoptimized):");

Check warning on line 167 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
println!("{}", compiled_program.circuit);
}

Expand Down Expand Up @@ -198,7 +198,7 @@
for compiled_contract in &compiled_contracts {
for contract_function in &compiled_contract.functions {
println!(
"Compiled ACIR for {}::{} (unoptimized):",

Check warning on line 201 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
compiled_contract.name, contract_function.name
);
println!("{}", contract_function.bytecode);
Expand Down Expand Up @@ -249,6 +249,7 @@
is_internal: func_meta.is_internal.unwrap_or(false),
abi: function.abi,
bytecode: function.circuit,
debug: function.debug,
});
}

Expand Down
8 changes: 4 additions & 4 deletions crates/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use std::collections::HashMap;
use std::collections::BTreeMap;

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

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
/// Map opcode index of an ACIR circuit into the source code location
pub locations: HashMap<usize, Location>,
pub locations: BTreeMap<usize, Location>,
}

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

Expand All @@ -24,7 +24,7 @@ impl DebugInfo {
/// 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 = HashMap::new();
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]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! `GeneratedAcir` is constructed as part of the `acir_gen` pass to accumulate all of the ACIR
//! program as it is being converted from SSA form.
use std::collections::HashMap;
use std::collections::BTreeMap;

use crate::{
brillig::brillig_gen::brillig_directive,
Expand Down Expand Up @@ -45,8 +45,8 @@
/// All witness indices which are inputs to the main function
pub(crate) input_witnesses: Vec<Witness>,

/// Correspondance between an opcode index (in opcodes) and the source code location which generated it

Check warning on line 48 in crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (Correspondance)
pub(crate) locations: HashMap<usize, Location>,
pub(crate) locations: BTreeMap<usize, Location>,

/// Source code location of the current instruction being processed
/// None if we do not know the location
Expand Down Expand Up @@ -344,8 +344,8 @@
}

/// Signed division lhs / rhs
/// We derive the signed division from the unsigned euclidian division.

Check warning on line 347 in crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (euclidian)
/// note that this is not euclidian division!

Check warning on line 348 in crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (euclidian)
// if x is a signed integer, then sign(x)x >= 0
// so if a and b are signed integers, we can do the unsigned division:
// sign(a)a = q1*sign(b)b + r1
Expand Down Expand Up @@ -753,7 +753,7 @@
let two_max_bits: FieldElement = two.pow(&FieldElement::from(max_bits as i128));
let comparison_evaluation = (a - b) + two_max_bits;

// Euclidian division by 2^{max_bits} : 2^{max_bits} + a - b = q * 2^{max_bits} + r

Check warning on line 756 in crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (Euclidian)
//
// 2^{max_bits} is of max_bits+1 bit size
// If a>b, then a-b is less than 2^{max_bits} - 1, so 2^{max_bits} + a - b is less than 2^{max_bits} + 2^{max_bits} - 1 = 2^{max_bits+1} - 1
Expand Down
Loading