Skip to content

Commit

Permalink
feat: Put debug info for functions separate from variables
Browse files Browse the repository at this point in the history
  • Loading branch information
ggiraldez committed Feb 12, 2024
1 parent 3b39a39 commit 3fae7dc
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 149 deletions.
14 changes: 13 additions & 1 deletion compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ use serde::{
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, PartialOrd, Ord, Deserialize, Serialize)]
pub struct DebugVarId(pub u32);

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

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

Expand All @@ -33,7 +36,14 @@ pub struct DebugVariable {
pub debug_type_id: DebugTypeId,
}

#[derive(Debug, Clone, Hash, Deserialize, Serialize)]
pub struct DebugFunction {
pub name: String,
pub arg_names: Vec<String>,
}

pub type DebugVariables = BTreeMap<DebugVarId, DebugVariable>;
pub type DebugFunctions = BTreeMap<DebugFnId, DebugFunction>;
pub type DebugTypes = BTreeMap<DebugTypeId, PrintableType>;

#[serde_as]
Expand All @@ -45,6 +55,7 @@ pub struct DebugInfo {
#[serde_as(as = "BTreeMap<DisplayFromStr, _>")]
pub locations: BTreeMap<OpcodeLocation, Vec<Location>>,
pub variables: DebugVariables,
pub functions: DebugFunctions,
pub types: DebugTypes,
}

Expand All @@ -60,9 +71,10 @@ impl DebugInfo {
pub fn new(
locations: BTreeMap<OpcodeLocation, Vec<Location>>,
variables: DebugVariables,
functions: DebugFunctions,
types: DebugTypes,
) -> Self {
Self { locations, variables, types }
Self { locations, variables, functions, types }
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub fn create_circuit(
) -> Result<(Circuit, DebugInfo, Vec<Witness>, Vec<Witness>, Vec<SsaReport>), RuntimeError> {
let debug_variables = program.debug_variables.clone();
let debug_types = program.debug_types.clone();
let debug_functions = program.debug_functions.clone();
let func_sig = program.main_function_signature.clone();
let recursive = program.recursive;
let mut generated_acir = optimize_into_acir(
Expand Down Expand Up @@ -135,7 +136,7 @@ pub fn create_circuit(
.map(|(index, locations)| (index, locations.into_iter().collect()))
.collect();

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

// Perform any ACIR-level optimizations
let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit);
Expand Down
50 changes: 45 additions & 5 deletions compiler/noirc_frontend/src/debug/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
ast::{Path, PathKind},
parser::{Item, ItemKind},
};
use noirc_errors::debug_info::{DebugFnId, DebugFunction};
use noirc_errors::{Span, Spanned};
use std::collections::HashMap;
use std::collections::VecDeque;
Expand All @@ -26,8 +27,12 @@ pub struct DebugInstrumenter {
// all field names referenced when assigning to a member of a variable
pub field_names: HashMap<SourceFieldId, String>,

// all collected function metadata (name + argument names)
pub functions: HashMap<DebugFnId, DebugFunction>,

next_var_id: u32,
next_field_name_id: u32,
next_fn_id: u32,

// last seen variable names and their IDs grouped by scope
scope: Vec<HashMap<String, SourceVarId>>,
Expand All @@ -38,9 +43,11 @@ impl Default for DebugInstrumenter {
Self {
variables: HashMap::default(),
field_names: HashMap::default(),
functions: HashMap::default(),
scope: vec![],
next_var_id: 0,
next_field_name_id: 1,
next_fn_id: 0,
}
}
}
Expand Down Expand Up @@ -76,11 +83,20 @@ impl DebugInstrumenter {
field_name_id
}

fn insert_function(&mut self, fn_name: String, arguments: Vec<String>) -> DebugFnId {
let fn_id = DebugFnId(self.next_fn_id);
self.next_fn_id += 1;
self.functions.insert(fn_id, DebugFunction { name: fn_name, arg_names: arguments });
fn_id
}

fn walk_fn(&mut self, func: &mut ast::FunctionDefinition) {
let func_name = &func.name.0.contents;
self.scope.push(HashMap::default());
let fn_id = self.insert_var(func_name);
let func_name = func.name.0.contents.clone();
let func_args =
func.parameters.iter().map(|param| pattern_to_string(&param.pattern)).collect();
let fn_id = self.insert_function(func_name, func_args);
let enter_fn = build_debug_call_stmt("enter", fn_id, func.span);
self.scope.push(HashMap::default());

let set_fn_params = func
.parameters
Expand Down Expand Up @@ -108,7 +124,7 @@ impl DebugInstrumenter {
&mut self,
statements: &mut Vec<ast::Statement>,
span: Span,
opt_fn_id: Option<SourceVarId>,
opt_fn_id: Option<DebugFnId>,
) {
statements.iter_mut().for_each(|stmt| self.walk_statement(stmt));

Expand Down Expand Up @@ -586,7 +602,7 @@ fn build_assign_member_stmt(
ast::Statement { kind: ast::StatementKind::Semi(ast::Expression { kind, span }), span }
}

fn build_debug_call_stmt(fname: &str, fn_id: SourceVarId, span: Span) -> ast::Statement {
fn build_debug_call_stmt(fname: &str, fn_id: DebugFnId, span: Span) -> ast::Statement {
let kind = ast::ExpressionKind::Call(Box::new(ast::CallExpression {
func: Box::new(ast::Expression {
kind: ast::ExpressionKind::Variable(ast::Path {
Expand Down Expand Up @@ -625,6 +641,30 @@ fn pattern_vars(pattern: &ast::Pattern) -> Vec<(ast::Ident, bool)> {
vars
}

fn pattern_to_string(pattern: &ast::Pattern) -> String {
match pattern {
ast::Pattern::Identifier(id) => id.0.contents.clone(),
ast::Pattern::Mutable(mpat, _) => format!("mut {}", pattern_to_string(mpat.as_ref())),
ast::Pattern::Tuple(elements, _) => format!(
"({})",
elements.iter().map(pattern_to_string).collect::<Vec<String>>().join(", ")
),
ast::Pattern::Struct(name, fields, _) => {
format!(
"{} {{ {} }}",
name,
fields
.iter()
.map(|(field_ident, field_pattern)| {
format!("{}: {}", &field_ident.0.contents, pattern_to_string(field_pattern))
})
.collect::<Vec<_>>()
.join(", "),
)
}
}
}

fn ident(s: &str, span: Span) -> ast::Ident {
ast::Ident(Spanned::from(span, s.to_string()))
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use acvm::FieldElement;
use iter_extended::vecmap;
use noirc_errors::{
debug_info::{DebugTypes, DebugVariables},
debug_info::{DebugFunctions, DebugTypes, DebugVariables},
Location,
};

Expand Down Expand Up @@ -252,6 +252,7 @@ pub struct Program {
/// Indicates to a backend whether a SNARK-friendly prover should be used.
pub recursive: bool,
pub debug_variables: DebugVariables,
pub debug_functions: DebugFunctions,
pub debug_types: DebugTypes,
}

Expand All @@ -265,6 +266,7 @@ impl Program {
return_visibility: Visibility,
recursive: bool,
debug_variables: DebugVariables,
debug_functions: DebugFunctions,
debug_types: DebugTypes,
) -> Program {
Program {
Expand All @@ -275,6 +277,7 @@ impl Program {
return_visibility,
recursive,
debug_variables,
debug_functions,
debug_types,
}
}
Expand Down
74 changes: 3 additions & 71 deletions compiler/noirc_frontend/src/monomorphization/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use noirc_printable_type::PrintableType;

use crate::debug::{SourceFieldId, SourceVarId};
use crate::hir_def::expr::*;
use crate::hir_def::stmt::HirPattern;
use crate::node_interner::ExprId;
use crate::Type;

use super::ast::{Expression, Ident};
use super::Monomorphizer;
Expand Down Expand Up @@ -48,8 +46,6 @@ impl<'interner> Monomorphizer<'interner> {
self.patch_debug_var_assign(call, arguments);
} else if name == "__debug_var_drop" {
self.patch_debug_var_drop(call, arguments);
} else if name == "__debug_fn_enter" {
self.patch_debug_fn_enter(call, arguments);
} else if let Some(arity) = name.strip_prefix(DEBUG_MEMBER_ASSIGN_PREFIX) {
let arity = arity.parse::<usize>().expect("failed to parse member assign arity");
self.patch_debug_member_assign(call, arguments, arity);
Expand All @@ -75,7 +71,7 @@ impl<'interner> Monomorphizer<'interner> {
let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]);
let source_var_id = source_var_id.to_u128().into();
// then update the ID used for tracking at runtime
let var_id = self.debug_type_tracker.insert_var(source_var_id, var_type);
let var_id = self.debug_type_tracker.insert_var(source_var_id, &var_type);
let interned_var_id = self.intern_var_id(var_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
}
Expand Down Expand Up @@ -172,77 +168,13 @@ impl<'interner> Monomorphizer<'interner> {
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
}

/// Update instrumentation code to track function enter and exit.
fn patch_debug_fn_enter(&mut self, call: &HirCallExpression, arguments: &mut [Expression]) {
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let var_id_arg = hir_arguments.get(DEBUG_VAR_ID_ARG_SLOT);
let Some(HirExpression::Literal(HirLiteral::Integer(source_var_id, _))) = var_id_arg else {
unreachable!("Missing source_var_id in __debug_fn_enter call");
};
let source_var_id = source_var_id.to_u128().into();
let func_id = self.current_function_id.expect("current function not set");
let fn_meta = self.interner.function_meta(&func_id);
let fn_name = self.interner.definition(fn_meta.name.id).name.clone();
let ptype = PrintableType::Function {
name: fn_name.clone(),
arguments: fn_meta
.parameters
.iter()
.map(|(arg_pattern, arg_type, _)| {
let arg_display = self.pattern_to_string(arg_pattern);
(arg_display, arg_type.follow_bindings().into())
})
.collect(),
env: Box::new(PrintableType::Tuple { types: vec![] }),
};
let fn_id = self.debug_type_tracker.insert_var_printable(source_var_id, ptype);
let interned_var_id = self.intern_var_id(fn_id, &call.location);
arguments[DEBUG_VAR_ID_ARG_SLOT] = self.expr(interned_var_id);
}

fn intern_var_id(&mut self, var_id: DebugVarId, location: &Location) -> ExprId {
let var_id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false);
let expr_id = self.interner.push_expr(HirExpression::Literal(var_id_literal));
let id_literal = HirLiteral::Integer((var_id.0 as u128).into(), false);
let expr_id = self.interner.push_expr(HirExpression::Literal(id_literal));
self.interner.push_expr_type(&expr_id, crate::Type::FieldElement);
self.interner.push_expr_location(expr_id, location.span, location.file);
expr_id
}

fn pattern_to_string(&self, pat: &HirPattern) -> String {
match pat {
HirPattern::Identifier(hir_id) => self.interner.definition(hir_id.id).name.clone(),
HirPattern::Mutable(mpat, _) => format!("mut {}", self.pattern_to_string(mpat)),
HirPattern::Tuple(elements, _) => format!(
"({})",
elements
.iter()
.map(|element| self.pattern_to_string(element))
.collect::<Vec<String>>()
.join(", ")
),
HirPattern::Struct(Type::Struct(struct_type, _), fields, _) => {
let struct_type = struct_type.borrow();
format!(
"{} {{ {} }}",
&struct_type.name.0.contents,
fields
.iter()
.map(|(field_ident, field_pattern)| {
format!(
"{}: {}",
&field_ident.0.contents,
self.pattern_to_string(field_pattern)
)
})
.collect::<Vec<String>>()
.join(", "),
)
}
HirPattern::Struct(typ, _, _) => {
panic!("unexpected type of struct: {typ:?}");
}
}
}
}

fn element_type_at_index(ptype: &PrintableType, i: usize) -> &PrintableType {
Expand Down
Loading

0 comments on commit 3fae7dc

Please sign in to comment.