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(profiler): Add Brillig procedure info to debug artifact for more informative profiling #6385

Merged
merged 11 commits into from
Oct 30, 2024
66 changes: 66 additions & 0 deletions acvm-repo/acir/src/circuit/brillig.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::str::FromStr;

use super::opcodes::BlockId;
use crate::native_types::{Expression, Witness};
use brillig::Opcode as BrilligOpcode;
use serde::{Deserialize, Serialize};
use thiserror::Error;

/// Inputs for the Brillig VM. These are the initial inputs
/// that the Brillig VM will use to start.
Expand Down Expand Up @@ -46,3 +49,66 @@ impl std::fmt::Display for BrilligFunctionId {
write!(f, "{}", self.0)
}
}

/// 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, PartialOrd, Ord, Deserialize, Serialize)]
pub enum ProcedureId {
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
ArrayCopy,
ArrayReverse,
VectorCopy,
MemCopy,
PrepareVectorPush(bool),
VectorPopFront,
VectorPopBack,
PrepareVectorInsert,
VectorRemove,
CheckMaxStackDepth,
}

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"),
}
}
}
#[derive(Error, Debug)]
pub enum ProcedureIdFromStrError {
#[error("Invalid procedure id string: {0}")]
InvalidProcedureIdString(String),
}

/// The implementation of display and FromStr allows serializing and deserializing a ProcedureId to a string.
/// This is useful when used as key in a map that has to be serialized to JSON/TOML, for example when mapping an opcode to its metadata.
impl FromStr for ProcedureId {
type Err = ProcedureIdFromStrError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let res = match s {
"ArrayCopy" => ProcedureId::ArrayCopy,
"ArrayReverse" => ProcedureId::ArrayReverse,
"VectorCopy" => ProcedureId::VectorCopy,
"MemCopy" => ProcedureId::MemCopy,
"PrepareVectorPush(true)" => ProcedureId::PrepareVectorPush(true),
"PrepareVectorPush(false)" => ProcedureId::PrepareVectorPush(false),
"VectorPopFront" => ProcedureId::VectorPopFront,
"VectorPopBack" => ProcedureId::VectorPopBack,
"PrepareVectorInsert" => ProcedureId::PrepareVectorInsert,
"VectorRemove" => ProcedureId::VectorRemove,
"CheckMaxStackDepth" => ProcedureId::CheckMaxStackDepth,
_ => return Err(ProcedureIdFromStrError::InvalidProcedureIdString(s.to_string())),
};
Ok(res)
}
}
7 changes: 6 additions & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use acvm::acir::circuit::brillig::BrilligFunctionId;
use acvm::acir::circuit::brillig::ProcedureId;
use acvm::acir::circuit::BrilligOpcodeLocation;
use acvm::acir::circuit::OpcodeLocation;
use acvm::compiler::AcirTransformationMap;
Expand Down Expand Up @@ -104,6 +105,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.
#[serde_as(as = "BTreeMap<_, BTreeMap<DisplayFromStr, _>>")]
pub brillig_procedure_locs: BTreeMap<BrilligFunctionId, BTreeMap<ProcedureId, (usize, usize)>>,
}

/// Holds OpCodes Counts for Acir and Brillig Opcodes
Expand All @@ -124,8 +128,9 @@ impl DebugInfo {
variables: DebugVariables,
functions: DebugFunctions,
types: DebugTypes,
brillig_procedure_locs: BTreeMap<BrilligFunctionId, BTreeMap<ProcedureId, (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()
}
}
13 changes: 11 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use acvm::acir::brillig::Opcode as BrilligOpcode;
use acvm::acir::{brillig::Opcode as BrilligOpcode, circuit::brillig::ProcedureId};
use std::collections::{BTreeMap, HashMap};

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

/// Represents a parameter or a return value of an entry point function.
Expand All @@ -24,6 +23,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 +53,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 +157,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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use acvm::{
BinaryFieldOp, BinaryIntOp, BitSize, BlackBoxOp, HeapArray, HeapValueType,
MemoryAddress, Opcode as BrilligOpcode, ValueOrArray,
},
circuit::brillig::ProcedureId,
AcirField,
},
FieldElement,
Expand All @@ -15,7 +16,6 @@ use super::{
artifact::{Label, UnresolvedJumpLocation},
brillig_variable::SingleAddrVariable,
debug_show::DebugToString,
procedures::ProcedureId,
registers::RegisterAllocator,
BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod vector_pop_back;
mod vector_pop_front;
mod vector_remove;

use acvm::acir::circuit::brillig::ProcedureId;
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;
Expand All @@ -28,23 +29,6 @@ use super::{
BrilligContext,
};

/// 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 {
ArrayCopy,
ArrayReverse,
VectorCopy,
MemCopy,
PrepareVectorPush(bool),
VectorPopFront,
VectorPopBack,
PrepareVectorInsert,
VectorRemove,
CheckMaxStackDepth,
}

pub(crate) fn compile_procedure<F: AcirField + DebugToString>(
procedure_id: ProcedureId,
) -> BrilligArtifact<F> {
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_evaluator/src/brillig/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ impl Brillig {
self.ssa_function_to_brillig.get(&function_id).map(Cow::Borrowed)
}
// Procedures are compiled as needed
LabelType::Procedure(procedure_id) => Some(Cow::Owned(compile_procedure(procedure_id))),
LabelType::Procedure(procedure_id) => {
let mut artifact = compile_procedure(procedure_id);
artifact.procedure = Some(procedure_id);
Some(Cow::Owned(artifact))
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}
_ => unreachable!("ICE: Expected a function or procedure label"),
}
}
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 @@ -9,7 +9,7 @@ use crate::{
};
use acvm::acir::{
circuit::{
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs, ProcedureId},
opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode},
AssertionPayload, BrilligOpcodeLocation, OpcodeLocation,
},
Expand All @@ -20,6 +20,7 @@ use acvm::{
acir::AcirField,
acir::{circuit::directives::Directive, native_types::Expression},
};

use iter_extended::vecmap;
use num_bigint::BigUint;

Expand Down Expand Up @@ -72,13 +73,24 @@ 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>,

/// Flag that determines whether we should gather extra information for profiling
/// such as opcode range of Brillig procedures.
pub(crate) profiling_active: bool,
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved

/// 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<ProcedureId, (usize, usize)>;

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
pub(crate) enum BrilligStdlibFunc {
Inverse,
Expand Down Expand Up @@ -591,6 +603,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, (*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
Loading
Loading