Skip to content

Commit

Permalink
feat(nargo): Multiple circuits info for binary programs (#4719)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves  #4697 

## Summary\*

The `nargo info` command previously assumed that we only had a single
circuit. In order to display accurate circuits sizes we should display
the circuit size of each individual circuit entry part which is part of
an entire `Program`.

For improved debugging I also had to exposed names in the
`GeneratedAcir`. This prompted also refactoring the context that we pass
around in `ssa.rs` when creating a circuit and a program. Two new
structs,`SsaProgramArtifact` and `SsaCircuitArtifact`, have been created
to clean this up.

Example output from `nargo info` on `fold_basic_nested_call`:
<img width="847" alt="Screenshot 2024-04-04 at 3 54 46 PM"
src="https://github.com/noir-lang/noir/assets/43554004/95a827c4-766c-4d7d-b03e-fb9428cfdb2a">


## Additional Context

It isn't necessarily obvious how we should display foldable circuits for
contracts which already have multiple entry points. I have left this for
follow-up work: #4720

## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [X] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
vezenovm authored Apr 8, 2024
1 parent dbb86ac commit 50d2735
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 59 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/gates_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ jobs:
- name: Compare gates reports
id: gates_diff
uses: TomAFrench/noir-gates-diff@e7cf131b7e7f044c01615f93f0b855f65ddc02d4
uses: TomAFrench/noir-gates-diff@df05f34e2ab275ddc4f2cac065df1c88f8a05e5d
with:
report: gates_report.json
summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,7 @@ pub struct ContractFunction {
pub bytecode: Program,

pub debug: DebugInfo,

/// Names of the functions in the program. These are used for more informative debugging and benchmarking.
pub names: Vec<String>,
}
21 changes: 18 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use noirc_abi::{AbiParameter, AbiType, ContractEvent};
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
use noirc_evaluator::ssa::SsaProgramArtifact;
use noirc_frontend::debug::build_debug_crate_file;
use noirc_frontend::graph::{CrateId, CrateName};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
Expand Down Expand Up @@ -423,6 +424,7 @@ fn compile_contract_inner(
bytecode: function.program,
debug: function.debug,
is_unconstrained: modifiers.is_unconstrained,
names: function.names,
});
}

Expand Down Expand Up @@ -485,16 +487,28 @@ pub fn compile_no_check(
}
let visibility = program.return_visibility;

let (program, debug, warnings, input_witnesses, return_witnesses) = create_program(
let SsaProgramArtifact {
program,
debug,
warnings,
main_input_witnesses,
main_return_witnesses,
names,
} = create_program(
program,
options.show_ssa,
options.show_brillig,
options.force_brillig,
options.benchmark_codegen,
)?;

let abi =
abi_gen::gen_abi(context, &main_function, input_witnesses, return_witnesses, visibility);
let abi = abi_gen::gen_abi(
context,
&main_function,
main_input_witnesses,
main_return_witnesses,
visibility,
);
let file_map = filter_relevant_files(&debug, &context.file_manager);

Ok(CompiledProgram {
Expand All @@ -510,5 +524,6 @@ pub fn compile_no_check(
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
warnings,
names,
})
}
2 changes: 2 additions & 0 deletions compiler/noirc_driver/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ pub struct CompiledProgram {
pub debug: DebugInfo,
pub file_map: BTreeMap<FileId, DebugFile>,
pub warnings: Vec<SsaReport>,
/// Names of the functions in the program. These are used for more informative debugging and benchmarking.
pub names: Vec<String>,
}
82 changes: 54 additions & 28 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,29 @@ fn time<T>(name: &str, print_timings: bool, f: impl FnOnce() -> T) -> T {
result
}

#[derive(Default)]
pub struct SsaProgramArtifact {
pub program: AcirProgram,
pub debug: Vec<DebugInfo>,
pub warnings: Vec<SsaReport>,
pub main_input_witnesses: Vec<Witness>,
pub main_return_witnesses: Vec<Witness>,
pub names: Vec<String>,
}

impl SsaProgramArtifact {
fn add_circuit(&mut self, mut circuit_artifact: SsaCircuitArtifact, is_main: bool) {
self.program.functions.push(circuit_artifact.circuit);
self.debug.push(circuit_artifact.debug_info);
self.warnings.append(&mut circuit_artifact.warnings);
if is_main {
self.main_input_witnesses = circuit_artifact.input_witnesses;
self.main_return_witnesses = circuit_artifact.return_witnesses;
}
self.names.push(circuit_artifact.name);
}
}

/// Compiles the [`Program`] into [`ACIR``][acvm::acir::circuit::Program].
///
/// The output ACIR is is backend-agnostic and so must go through a transformation pass before usage in proof generation.
Expand All @@ -99,8 +122,7 @@ pub fn create_program(
enable_brillig_logging: bool,
force_brillig_output: bool,
print_codegen_timings: bool,
) -> Result<(AcirProgram, Vec<DebugInfo>, Vec<SsaReport>, Vec<Witness>, Vec<Witness>), RuntimeError>
{
) -> Result<SsaProgramArtifact, RuntimeError> {
let debug_variables = program.debug_variables.clone();
let debug_types = program.debug_types.clone();
let debug_functions = program.debug_functions.clone();
Expand All @@ -121,37 +143,33 @@ pub fn create_program(
"The generated ACIRs should match the supplied function signatures"
);

let mut functions = vec![];
let mut debug_infos = vec![];
let mut warning_infos = vec![];
let mut main_input_witnesses = Vec::new();
let mut main_return_witnesses = Vec::new();
let mut program_artifact = SsaProgramArtifact::default();
// For setting up the ABI we need separately specify main's input and return witnesses
let mut is_main = true;
for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) {
let (circuit, debug_info, warnings, input_witnesses, return_witnesses) =
convert_generated_acir_into_circuit(
acir,
func_sig,
recursive,
// TODO: get rid of these clones
debug_variables.clone(),
debug_functions.clone(),
debug_types.clone(),
);
functions.push(circuit);
debug_infos.push(debug_info);
warning_infos.extend(warnings);
if is_main {
main_input_witnesses = input_witnesses;
main_return_witnesses = return_witnesses;
}
let circuit_artifact = convert_generated_acir_into_circuit(
acir,
func_sig,
recursive,
// TODO: get rid of these clones
debug_variables.clone(),
debug_functions.clone(),
debug_types.clone(),
);
program_artifact.add_circuit(circuit_artifact, is_main);
is_main = false;
}

let program = AcirProgram { functions };
Ok(program_artifact)
}

Ok((program, debug_infos, warning_infos, main_input_witnesses, main_return_witnesses))
pub struct SsaCircuitArtifact {
name: String,
circuit: Circuit,
debug_info: DebugInfo,
warnings: Vec<SsaReport>,
input_witnesses: Vec<Witness>,
return_witnesses: Vec<Witness>,
}

fn convert_generated_acir_into_circuit(
Expand All @@ -161,7 +179,7 @@ fn convert_generated_acir_into_circuit(
debug_variables: DebugVariables,
debug_functions: DebugFunctions,
debug_types: DebugTypes,
) -> (Circuit, DebugInfo, Vec<SsaReport>, Vec<Witness>, Vec<Witness>) {
) -> SsaCircuitArtifact {
let opcodes = generated_acir.take_opcodes();
let current_witness_index = generated_acir.current_witness_index().0;
let GeneratedAcir {
Expand All @@ -170,6 +188,7 @@ fn convert_generated_acir_into_circuit(
input_witnesses,
assert_messages,
warnings,
name,
..
} = generated_acir;

Expand Down Expand Up @@ -202,7 +221,14 @@ fn convert_generated_acir_into_circuit(
let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit);
debug_info.update_acir(transformation_map);

(optimized_circuit, debug_info, warnings, input_witnesses, return_witnesses)
SsaCircuitArtifact {
name,
circuit: optimized_circuit,
debug_info,
warnings,
input_witnesses,
return_witnesses,
}
}

// Takes each function argument and partitions the circuit's inputs witnesses according to its visibility.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use iter_extended::vecmap;
use num_bigint::BigUint;

#[derive(Debug, Default)]
/// The output of the Acir-gen pass
/// The output of the Acir-gen pass, which should only be produced for entry point Acir functions
pub(crate) struct GeneratedAcir {
/// The next witness index that may be declared.
/// If witness index is `None` then we have not yet created a witness
Expand Down Expand Up @@ -58,6 +58,10 @@ pub(crate) struct GeneratedAcir {
pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,

pub(crate) warnings: Vec<SsaReport>,

/// Name for the corresponding entry point represented by this Acir-gen output.
/// Only used for debugging and benchmarking purposes
pub(crate) name: String,
}

impl GeneratedAcir {
Expand Down
7 changes: 5 additions & 2 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ impl Ssa {
// TODO: can we parallelise this?
for function in self.functions.values() {
let context = Context::new();
if let Some(generated_acir) = context.convert_ssa_function(&self, function, brillig)? {
if let Some(mut generated_acir) =
context.convert_ssa_function(&self, function, brillig)?
{
generated_acir.name = function.name().to_owned();
acirs.push(generated_acir);
}
}
Expand Down Expand Up @@ -253,7 +256,7 @@ impl Context {
}
}
}
// We only want to convert entry point functions. This being `main` and those marked with `#[fold]`
// We only want to convert entry point functions. This being `main` and those marked with `InlineType::Fold`
Ok(Some(self.convert_acir_main(function, ssa, brillig)?))
}
RuntimeType::Brillig => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "fold_non_contract_method"
type = "contract"
authors = [""]
compiler_version = ">=0.26.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
contract Foo {
use crate::times_10;

fn double(x: Field) -> pub Field {
x * 2
}
fn triple(x: Field) -> pub Field {
x * 3
}
fn times_40(x: Field) -> pub Field {
times_10(x) * 4
}
}

#[fold]
fn times_10(x: Field) -> Field {
x * 10
}
4 changes: 4 additions & 0 deletions tooling/nargo/src/artifacts/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ pub struct ProgramArtifact {

/// Map of file Id to the source code so locations in debug info can be mapped to source code they point to.
pub file_map: BTreeMap<FileId, DebugFile>,

pub names: Vec<String>,
}

impl From<CompiledProgram> for ProgramArtifact {
Expand All @@ -45,6 +47,7 @@ impl From<CompiledProgram> for ProgramArtifact {
bytecode: compiled_program.program,
debug_symbols: compiled_program.debug,
file_map: compiled_program.file_map,
names: compiled_program.names,
}
}
}
Expand All @@ -59,6 +62,7 @@ impl From<ProgramArtifact> for CompiledProgram {
debug: program.debug_symbols,
file_map: program.file_map,
warnings: vec![],
names: program.names,
}
}
}
2 changes: 1 addition & 1 deletion tooling/nargo_cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ fn compile_success_empty_{test_name}() {{
let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap_or_else(|e| {{
panic!("JSON was not well-formatted {{:?}}\n\n{{:?}}", e, std::str::from_utf8(&output.stdout))
}});
let num_opcodes = &json["programs"][0]["acir_opcodes"];
let num_opcodes = &json["programs"][0]["functions"][0]["acir_opcodes"];
assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0);
}}
"#,
Expand Down
Loading

0 comments on commit 50d2735

Please sign in to comment.