Skip to content

Commit

Permalink
refactor: improve printing, optimize Localinterval values
Browse files Browse the repository at this point in the history
  • Loading branch information
Fumuran committed Oct 18, 2023
1 parent 51b3f39 commit 9551d5d
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 72 deletions.
32 changes: 16 additions & 16 deletions assembly/src/assembler/instruction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
};
use crate::utils::bound_into_included_u64;
use core::ops::RangeBounds;
use vm_core::{DebugOptions, Decorator, FieldElement, StarkField};
use vm_core::{Decorator, FieldElement, StarkField};

mod adv_ops;
mod crypto_ops;
Expand Down Expand Up @@ -339,7 +339,7 @@ impl Assembler {

Instruction::Debug(options) => {
if self.in_debug_mode() {
span.push_decorator(process_debug_options(ctx, options)?)
span.push_decorator(Decorator::Debug(*options))
}
Ok(None)
}
Expand Down Expand Up @@ -409,17 +409,17 @@ where
})
}

/// Returns a [Decorator::Debug] instance created with information about number of procedure
/// locals.
fn process_debug_options(
ctx: &AssemblyContext,
options: &DebugOptions,
) -> Result<Decorator, AssemblyError> {
let num_locals = ctx.num_proc_locals() as u32;
match options {
DebugOptions::LocalInterval(start, end, _) => {
Ok(Decorator::Debug(DebugOptions::LocalInterval(*start, *end, num_locals)))
}
_ => Ok(Decorator::Debug(*options)),
}
}
// / Returns a [Decorator::Debug] instance created with information about number of procedure
// / locals.
// fn process_debug_options(
// ctx: &AssemblyContext,
// options: &DebugOptions,
// ) -> Result<Decorator, AssemblyError> {
// let num_locals = ctx.num_proc_locals();
// match options {
// DebugOptions::LocalInterval(start, end, _) => {
// Ok(Decorator::Debug(DebugOptions::LocalInterval(*start, *end, num_locals)))
// }
// _ => Ok(Decorator::Debug(*options)),
// }
// }
2 changes: 2 additions & 0 deletions assembly/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl ProgramAst {
local_procs: LocalProcMap::default(),
reexported_procs: ReExportedProcMap::default(),
local_constants,
num_procs_local: 0,
};

context.parse_procedures(&mut tokens, false)?;
Expand Down Expand Up @@ -450,6 +451,7 @@ impl ModuleAst {
local_procs: LocalProcMap::default(),
reexported_procs: ReExportedProcMap::default(),
local_constants,
num_procs_local: 0,
};
context.parse_procedures(&mut tokens, true)?;

Expand Down
12 changes: 6 additions & 6 deletions assembly/src/ast/nodes/serde/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ pub fn write_options_into<W: ByteWriter>(target: &mut W, options: &DebugOptions)
}
DebugOptions::LocalInterval(start, end, num_locals) => {
target.write_u8(LOCAL_INTERVAL);
target.write_u32(*start);
target.write_u32(*end);
target.write_u32(*num_locals);
target.write_u16(*start);
target.write_u16(*end);
target.write_u16(*num_locals);
}
}
}
Expand All @@ -49,9 +49,9 @@ pub fn read_options_from<R: ByteReader>(
Ok(DebugOptions::MemInterval(n, m))
}
LOCAL_INTERVAL => {
let n = source.read_u32()?;
let m = source.read_u32()?;
let num_locals = source.read_u32()?;
let n = source.read_u16()?;
let m = source.read_u16()?;
let num_locals = source.read_u16()?;
Ok(DebugOptions::LocalInterval(n, m, num_locals))
}
val => Err(DeserializationError::InvalidValue(val.to_string())),
Expand Down
7 changes: 6 additions & 1 deletion assembly/src/ast/parsers/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub struct ParserContext<'a> {
pub local_procs: LocalProcMap,
pub reexported_procs: ReExportedProcMap,
pub local_constants: LocalConstMap,
pub num_procs_local: u16,
}

impl ParserContext<'_> {
Expand Down Expand Up @@ -290,9 +291,13 @@ impl ParserContext<'_> {
None
};

self.num_procs_local = num_locals;

// parse procedure body
let body = self.parse_body(tokens, false)?;

self.num_procs_local = 0;

// consume the 'end' token
match tokens.read() {
None => {
Expand Down Expand Up @@ -623,7 +628,7 @@ impl ParserContext<'_> {

// ----- debug decorators -------------------------------------------------------------
"breakpoint" => simple_instruction(op, Breakpoint),
"debug" => debug::parse_debug(op),
"debug" => debug::parse_debug(op, self.num_procs_local),

// ----- catch all --------------------------------------------------------------------
_ => Err(ParsingError::invalid_op(op)),
Expand Down
14 changes: 7 additions & 7 deletions assembly/src/ast/parsers/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use vm_core::DebugOptions;
/// # Errors
/// Returns an error if the instruction token contains a wrong number of parameters, or if
/// the provided parameters are not valid.
pub fn parse_debug(op: &Token) -> Result<Node, ParsingError> {
pub fn parse_debug(op: &Token, num_procs_local: u16) -> Result<Node, ParsingError> {
debug_assert_eq!(op.parts()[0], "debug");
if op.num_parts() < 2 {
return Err(ParsingError::missing_param(op, "debug.stack.<debug_params?>"));
Expand Down Expand Up @@ -46,18 +46,18 @@ pub fn parse_debug(op: &Token) -> Result<Node, ParsingError> {
_ => return Err(ParsingError::extra_param(op)),
},
"local" => match op.num_parts() {
2 => DebugOptions::LocalInterval(0, 2u32.pow(16), 0),
2 => DebugOptions::LocalInterval(0, u16::MAX, num_procs_local),
3 => {
let n: u32 = parse_checked_param(op, 2, 0..=u16::MAX as u32)?;
DebugOptions::LocalInterval(n, n, 0)
let n: u16 = parse_checked_param(op, 2, 0..=u16::MAX)?;
DebugOptions::LocalInterval(n, n, num_procs_local)
}
4 => {
let n: u32 = parse_checked_param(op, 2, 0..=u16::MAX as u32)?;
let m: u32 = parse_checked_param(op, 3, 0..=u16::MAX as u32)?;
let n: u16 = parse_checked_param(op, 2, 0..=u16::MAX)?;
let m: u16 = parse_checked_param(op, 3, 0..=u16::MAX)?;
if m < n {
return Err(ParsingError::invalid_param_with_reason(op, 3, "the index of the end of the interval must be greater than the index of its beginning"));
}
DebugOptions::LocalInterval(n, m, 0)
DebugOptions::LocalInterval(n, m, num_procs_local)
}
_ => return Err(ParsingError::extra_param(op)),
},
Expand Down
2 changes: 1 addition & 1 deletion core/src/operations/decorators/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub enum DebugOptions {
///
/// First parameter specifies the starting address, second -- the ending address, and the third
/// specifies the overall number of locals.
LocalInterval(u32, u32, u32),
LocalInterval(u16, u16, u16),
}

impl fmt::Display for DebugOptions {
Expand Down
74 changes: 33 additions & 41 deletions processor/src/host/debug.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::ProcessState;
use crate::Vec;
use vm_core::{utils::string::ToString, DebugOptions, StarkField, Word};
use vm_core::{DebugOptions, StarkField, Word};

// DEBUG HANDLER
// ================================================================================================
Expand All @@ -22,7 +22,7 @@ pub fn print_debug_info<S: ProcessState>(process: &S, options: &DebugOptions) {
printer.print_mem_interval(process, *n, *m);
}
DebugOptions::LocalInterval(n, m, num_locals) => {
printer.print_local_interval(process, (*n, *m), *num_locals);
printer.print_local_interval(process, (*n as u32, *m as u32), *num_locals as u32);
}
}
}
Expand Down Expand Up @@ -71,7 +71,8 @@ impl Printer {
/// Prints the whole memory state at the cycle `clk` in context `ctx`.
fn print_mem_all<S: ProcessState>(&self, process: &S) {
let mem = process.get_mem_state(self.ctx);
let padding = mem.iter().fold(0, |max, value| word_elem_max_len(Some(value.1)).max(max));
let padding =
mem.iter().fold(0, |max, value| word_elem_max_len(Some(value.1)).max(max)) as usize;

println!("Memory state before step {} for the context {}:", self.clk, self.ctx);

Expand Down Expand Up @@ -119,7 +120,7 @@ impl Printer {
let local_memory_offset = self.fmp - num_locals + 1;

// in case start index is 0 and end index is 2^16, we should print all available locals.
let (start, end) = if interval.0 == 0 && interval.1 == 2u32.pow(16) {
let (start, end) = if interval.0 == 0 && interval.1 == u16::MAX as u32 {
(0, num_locals - 1)
} else {
interval
Expand All @@ -129,7 +130,7 @@ impl Printer {
.push((index, process.get_mem_value(self.ctx, index + local_memory_offset)))
}

if interval.0 == 0 && interval.1 == 2u32.pow(16) {
if interval.0 == 0 && interval.1 == u16::MAX as u32 {
println!("Local state before step {} for the context {}:", self.clk, self.ctx)
} else if interval.0 == interval.1 {
println!(
Expand All @@ -155,7 +156,8 @@ impl Printer {
/// If `is_local` is true, the output addresses are formatted as decimal values, otherwise as hex
/// strings.
fn print_interval(mem_interval: Vec<(u32, Option<Word>)>, is_local: bool) {
let padding = mem_interval.iter().fold(0, |max, value| word_elem_max_len(value.1).max(max));
let padding =
mem_interval.iter().fold(0, |max, value| word_elem_max_len(value.1).max(max)) as usize;

// print the main part of the memory (wihtout the last value)
for (addr, value) in mem_interval.iter().take(mem_interval.len() - 1) {
Expand All @@ -182,42 +184,19 @@ fn print_mem_address(
if let Some(value) = value {
if is_last {
if is_local {
println!(
"└── {addr:>5}: [{:>width$}, {:>width$}, {:>width$}, {:>width$}]\n",
value[0].as_int(),
value[1].as_int(),
value[2].as_int(),
value[3].as_int(),
width = padding
)
print!("└── {addr:>5}: ");
} else {
println!(
"└── {addr:#010x}: [{:>width$}, {:>width$}, {:>width$}, {:>width$}]\n",
value[0].as_int(),
value[1].as_int(),
value[2].as_int(),
value[3].as_int(),
width = padding
)
print!("└── {addr:#010x}: ");
}
} else if is_local {
println!(
"├── {addr:>5}: [{:>width$}, {:>width$}, {:>width$}, {:>width$}]",
value[0].as_int(),
value[1].as_int(),
value[2].as_int(),
value[3].as_int(),
width = padding
)
print_word(value, padding);
println!();
} else {
println!(
"├── {addr:#010x}: [{:>width$}, {:>width$}, {:>width$}, {:>width$}]",
value[0].as_int(),
value[1].as_int(),
value[2].as_int(),
value[3].as_int(),
width = padding
)
if is_local {
print!("├── {addr:>5}: ");
} else {
print!("├── {addr:#010x}: ");
}
print_word(value, padding);
}
} else if is_last {
if is_local {
Expand All @@ -232,10 +211,23 @@ fn print_mem_address(
}
}

/// Prints the provided Word with specified padding.
fn print_word(value: Word, padding: usize) {
println!(
"[{:>width$}, {:>width$}, {:>width$}, {:>width$}]",
value[0].as_int(),
value[1].as_int(),
value[2].as_int(),
value[3].as_int(),
width = padding
)
}

/// Returns the maximum length among the word elements.
fn word_elem_max_len(word: Option<Word>) -> usize {
fn word_elem_max_len(word: Option<Word>) -> u32 {
if let Some(word) = word {
word.iter().fold(0, |max, value| value.as_int().to_string().len().max(max))
word.iter()
.fold(0, |max, value| (value.as_int().checked_ilog10().unwrap_or(1) + 1).max(max))
} else {
0
}
Expand Down

0 comments on commit 9551d5d

Please sign in to comment.