Skip to content

Commit

Permalink
feat(profiler): Add Brillig procedure info to debug artifact for more…
Browse files Browse the repository at this point in the history
… informative profiling (#6385)

Co-authored-by: Ary Borenszweig <asterite@gmail.com>
  • Loading branch information
vezenovm and asterite authored Oct 30, 2024
1 parent 4344682 commit f5f65dc
Show file tree
Hide file tree
Showing 15 changed files with 153 additions and 15 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

12 changes: 11 additions & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
pub struct ProcedureDebugId(pub u32);

#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct ProgramDebugInfo {
pub debug_infos: Vec<DebugInfo>,
Expand Down Expand Up @@ -103,6 +106,9 @@ pub struct DebugInfo {
pub variables: DebugVariables,
pub functions: DebugFunctions,
pub types: DebugTypes,
/// This a map per brillig function representing the range of opcodes where a procedure is activated.
pub brillig_procedure_locs:
BTreeMap<BrilligFunctionId, BTreeMap<ProcedureDebugId, (usize, usize)>>,
}

impl DebugInfo {
Expand All @@ -115,8 +121,12 @@ impl DebugInfo {
variables: DebugVariables,
functions: DebugFunctions,
types: DebugTypes,
brillig_procedure_locs: BTreeMap<
BrilligFunctionId,
BTreeMap<ProcedureDebugId, (usize, usize)>,
>,
) -> Self {
Self { locations, brillig_locations, variables, functions, types }
Self { locations, brillig_locations, variables, functions, types, brillig_procedure_locs }
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ pub(crate) fn directive_invert<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 1 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_invert".to_string(),
..Default::default()
}
}

Expand Down Expand Up @@ -132,8 +131,7 @@ pub(crate) fn directive_quotient<F: AcirField>() -> GeneratedBrillig<F> {
},
BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 2 },
],
assert_messages: Default::default(),
locations: Default::default(),
name: "directive_integer_quotient".to_string(),
..Default::default()
}
}
11 changes: 9 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ use acvm::{
};
use debug_show::DebugShow;

use super::ProcedureId;

/// The Brillig VM does not apply a limit to the memory address space,
/// As a convention, we take use 32 bits. This means that we assume that
/// memory has 2^32 memory slots.
Expand Down Expand Up @@ -111,9 +113,14 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {

/// Special brillig context to codegen compiler intrinsic shared procedures
impl<F: AcirField + DebugToString> BrilligContext<F, ScratchSpace> {
pub(crate) fn new_for_procedure(enable_debug_trace: bool) -> BrilligContext<F, ScratchSpace> {
pub(crate) fn new_for_procedure(
enable_debug_trace: bool,
procedure_id: ProcedureId,
) -> BrilligContext<F, ScratchSpace> {
let mut obj = BrilligArtifact::default();
obj.procedure = Some(procedure_id);
BrilligContext {
obj: BrilligArtifact::default(),
obj,
registers: ScratchSpace::new(),
context_label: Label::entrypoint(),
current_section: 0,
Expand Down
13 changes: 12 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use acvm::acir::brillig::Opcode as BrilligOpcode;
use std::collections::{BTreeMap, HashMap};

use crate::brillig::brillig_ir::procedures::ProcedureId;
use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId};

use super::procedures::ProcedureId;

/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
pub(crate) enum BrilligParameter {
Expand All @@ -24,6 +25,7 @@ pub(crate) struct GeneratedBrillig<F> {
pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,
pub(crate) name: String,
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

#[derive(Default, Debug, Clone)]
Expand Down Expand Up @@ -53,6 +55,14 @@ pub(crate) struct BrilligArtifact<F> {
call_stack: CallStack,
/// Name of the function, only used for debugging purposes.
pub(crate) name: String,

/// This field contains the given procedure id if this artifact originates from as procedure
pub(crate) procedure: Option<ProcedureId>,
/// Procedure ID mapped to the range of its opcode locations
/// This is created as artifacts are linked together and allows us to determine
/// which opcodes originate from reusable procedures.s
/// The range is inclusive for both start and end opcode locations.
pub(crate) procedure_locations: HashMap<ProcedureId, (OpcodeLocation, OpcodeLocation)>,
}

/// A pointer to a location in the opcode.
Expand Down Expand Up @@ -149,6 +159,7 @@ impl<F: Clone + std::fmt::Debug> BrilligArtifact<F> {
locations: self.locations,
assert_messages: self.assert_messages,
name: self.name,
procedure_locations: self.procedure_locations,
}
}

Expand Down
61 changes: 58 additions & 3 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ use array_copy::compile_array_copy_procedure;
use array_reverse::compile_array_reverse_procedure;
use check_max_stack_depth::compile_check_max_stack_depth_procedure;
use mem_copy::compile_mem_copy_procedure;
use noirc_errors::debug_info::ProcedureDebugId;
use prepare_vector_insert::compile_prepare_vector_insert_procedure;
use prepare_vector_push::compile_prepare_vector_push_procedure;
use serde::{Deserialize, Serialize};
use vector_copy::compile_vector_copy_procedure;
use vector_pop_back::compile_vector_pop_back_procedure;
use vector_pop_front::compile_vector_pop_front_procedure;
Expand All @@ -31,8 +33,8 @@ use super::{
/// Procedures are a set of complex operations that are common in the noir language.
/// Extracting them to reusable procedures allows us to reduce the size of the generated Brillig.
/// Procedures receive their arguments on scratch space to avoid stack dumping&restoring.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
pub(crate) enum ProcedureId {
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
pub enum ProcedureId {
ArrayCopy,
ArrayReverse,
VectorCopy,
Expand All @@ -45,10 +47,63 @@ pub(crate) enum ProcedureId {
CheckMaxStackDepth,
}

impl ProcedureId {
pub(crate) fn to_debug_id(self) -> ProcedureDebugId {
ProcedureDebugId(match self {
ProcedureId::ArrayCopy => 0,
ProcedureId::ArrayReverse => 1,
ProcedureId::VectorCopy => 2,
ProcedureId::MemCopy => 3,
ProcedureId::PrepareVectorPush(true) => 4,
ProcedureId::PrepareVectorPush(false) => 5,
ProcedureId::VectorPopFront => 6,
ProcedureId::VectorPopBack => 7,
ProcedureId::PrepareVectorInsert => 8,
ProcedureId::VectorRemove => 9,
ProcedureId::CheckMaxStackDepth => 10,
})
}

pub fn from_debug_id(debug_id: ProcedureDebugId) -> Self {
let inner = debug_id.0;
match inner {
0 => ProcedureId::ArrayCopy,
1 => ProcedureId::ArrayReverse,
2 => ProcedureId::VectorCopy,
3 => ProcedureId::MemCopy,
4 => ProcedureId::PrepareVectorPush(true),
5 => ProcedureId::PrepareVectorPush(false),
6 => ProcedureId::VectorPopFront,
7 => ProcedureId::VectorPopBack,
8 => ProcedureId::PrepareVectorInsert,
9 => ProcedureId::VectorRemove,
10 => ProcedureId::CheckMaxStackDepth,
_ => panic!("Unsupported procedure debug ID of {inner} was supplied"),
}
}
}

impl std::fmt::Display for ProcedureId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ProcedureId::ArrayCopy => write!(f, "ArrayCopy"),
ProcedureId::ArrayReverse => write!(f, "ArrayReverse"),
ProcedureId::VectorCopy => write!(f, "VectorCopy"),
ProcedureId::MemCopy => write!(f, "MemCopy"),
ProcedureId::PrepareVectorPush(flag) => write!(f, "PrepareVectorPush({flag})"),
ProcedureId::VectorPopFront => write!(f, "VectorPopFront"),
ProcedureId::VectorPopBack => write!(f, "VectorPopBack"),
ProcedureId::PrepareVectorInsert => write!(f, "PrepareVectorInsert"),
ProcedureId::VectorRemove => write!(f, "VectorRemove"),
ProcedureId::CheckMaxStackDepth => write!(f, "CheckMaxStackDepth"),
}
}
}

pub(crate) fn compile_procedure<F: AcirField + DebugToString>(
procedure_id: ProcedureId,
) -> BrilligArtifact<F> {
let mut brillig_context = BrilligContext::new_for_procedure(false);
let mut brillig_context = BrilligContext::new_for_procedure(false, procedure_id);
brillig_context.enter_context(Label::procedure(procedure_id));

match procedure_id {
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_evaluator/src/brillig/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use crate::ssa::{
use fxhash::FxHashMap as HashMap;
use std::{borrow::Cow, collections::BTreeSet};

pub use self::brillig_ir::procedures::ProcedureId;

/// Context structure for the brillig pass.
/// It stores brillig-related data required for brillig generation.
#[derive(Default)]
Expand Down
11 changes: 9 additions & 2 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ fn convert_generated_acir_into_circuit(
assertion_payloads: assert_messages,
warnings,
name,
brillig_procedure_locs,
..
} = generated_acir;

Expand Down Expand Up @@ -328,8 +329,14 @@ fn convert_generated_acir_into_circuit(
})
.collect();

let mut debug_info =
DebugInfo::new(locations, brillig_locations, debug_variables, debug_functions, debug_types);
let mut debug_info = DebugInfo::new(
locations,
brillig_locations,
debug_variables,
debug_functions,
debug_types,
brillig_procedure_locs,
);

// Perform any ACIR-level optimizations
let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use acvm::{
acir::AcirField,
acir::{circuit::directives::Directive, native_types::Expression},
};

use iter_extended::vecmap;
use noirc_errors::debug_info::ProcedureDebugId;
use num_bigint::BigUint;

/// Brillig calls such as for the Brillig std lib are resolved only after code generation is finished.
Expand Down Expand Up @@ -72,13 +74,20 @@ pub(crate) struct GeneratedAcir<F: AcirField> {
/// As to avoid passing the ACIR gen shared context into each individual ACIR
/// we can instead keep this map and resolve the Brillig calls at the end of code generation.
pub(crate) brillig_stdlib_func_locations: BTreeMap<OpcodeLocation, BrilligStdlibFunc>,

/// Brillig function id -> Brillig procedure locations map
/// This maps allows a profiler to determine which Brillig opcodes
/// originated from a reusable procedure.
pub(crate) brillig_procedure_locs: BTreeMap<BrilligFunctionId, BrilligProcedureRangeMap>,
}

/// Correspondence between an opcode index (in opcodes) and the source code call stack which generated it
pub(crate) type OpcodeToLocationsMap = BTreeMap<OpcodeLocation, CallStack>;

pub(crate) type BrilligOpcodeToLocationsMap = BTreeMap<BrilligOpcodeLocation, CallStack>;

pub(crate) type BrilligProcedureRangeMap = BTreeMap<ProcedureDebugId, (usize, usize)>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
pub(crate) enum BrilligStdlibFunc {
Inverse,
Expand Down Expand Up @@ -591,6 +600,14 @@ impl<F: AcirField> GeneratedAcir<F> {
return;
}

for (procedure_id, (start_index, end_index)) in generated_brillig.procedure_locations.iter()
{
self.brillig_procedure_locs
.entry(brillig_function_index)
.or_default()
.insert(procedure_id.to_debug_id(), (*start_index, *end_index));
}

for (brillig_index, call_stack) in generated_brillig.locations.iter() {
self.brillig_locations
.entry(brillig_function_index)
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,15 @@ impl<'a> Context<'a> {
}
};
entry_point.link_with(artifact);
// Insert the range of opcode locations occupied by a procedure
if let Some(procedure_id) = artifact.procedure {
let num_opcodes = entry_point.byte_code.len();
let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
// We subtract one as to keep the range inclusive on both ends
entry_point
.procedure_locations
.insert(procedure_id, (previous_num_opcodes, num_opcodes - 1));
}
}
// Generate the final bytecode
Ok(entry_point.finish())
Expand Down
1 change: 1 addition & 0 deletions tooling/debugger/src/source_code_printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ mod tests {
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
)];
let debug_artifact = DebugArtifact::new(debug_symbols, &fm);

Expand Down
1 change: 0 additions & 1 deletion tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ fn compile_programs(
let target_width =
get_target_width(package.expression_width, compile_options.expression_width);
let program = nargo::ops::transform_program(program, target_width);

save_program_to_file(&program.into(), &package.name, workspace.target_directory_path());

Ok(((), warnings))
Expand Down
1 change: 1 addition & 0 deletions tooling/noirc_artifacts/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ mod tests {
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
BTreeMap::default(),
)];
let debug_artifact = DebugArtifact::new(debug_symbols, &fm);

Expand Down
1 change: 1 addition & 0 deletions tooling/profiler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ acir.workspace = true
nargo.workspace = true
noirc_errors.workspace = true
noirc_abi.workspace = true
noirc_evaluator.workspace = true

# Logs
tracing-subscriber.workspace = true
Expand Down
Loading

0 comments on commit f5f65dc

Please sign in to comment.