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

Seq #2346

Merged
merged 12 commits into from
Nov 12, 2024
17 changes: 13 additions & 4 deletions interp/src/debugger/commands/command_parser.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use super::core::{
Command, ParsedBreakPointID, ParsedGroupName, PrintMode, WatchPosition,
use super::{
core::{
Command, ParsedBreakPointID, ParsedGroupName, PrintMode, WatchPosition,
},
PrintCommand,
};
use pest_consume::{match_nodes, Error, Parser};

Expand All @@ -20,10 +23,15 @@ impl CommandParser {
fn EOI(_input: Node) -> ParseResult<()> {
Ok(())
}

fn code_calyx(_input: Node) -> ParseResult<()> {
Ok(())
}

fn code_nodes(_input: Node) -> ParseResult<()> {
Ok(())
}

// ----------------------

fn help(_input: Node) -> ParseResult<Command> {
Expand Down Expand Up @@ -59,8 +67,9 @@ impl CommandParser {

fn comm_where(input: Node) -> ParseResult<Command> {
Ok(match_nodes!(input.into_children();
[code_calyx(_)] => Command::PrintPC(true),
[] => Command::PrintPC(false),
[code_calyx(_)] => Command::PrintPC(PrintCommand::PrintCalyx),
[code_nodes(_)] => Command::PrintPC(PrintCommand::PrintNodes),
[] => Command::PrintPC(PrintCommand::Normal),
))
}

Expand Down
3 changes: 2 additions & 1 deletion interp/src/debugger/commands/commands.pest
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pc_s = { ^"s" }
pc_ufx = { ^"u." ~ num }
pc_sfx = { ^"s." ~ num }
code_calyx = { ^"calyx" }
code_nodes = {^"nodes"}

print_code = {
"\\" ~ (pc_ufx | pc_sfx | pc_s | pc_un)
Expand Down Expand Up @@ -67,7 +68,7 @@ disable_watch = { (^"disable-watch " | ^"disw ") ~ brk_id+ }

exit = { ^"exit" | ^"quit" }

comm_where = { (^"where" | "pc") ~ (code_calyx)? }
comm_where = { (^"where" | "pc") ~ (code_calyx | code_nodes)? }

explain = { ^"explain" }

Expand Down
9 changes: 8 additions & 1 deletion interp/src/debugger/commands/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,13 @@ impl From<(Vec<Path>, Option<PrintCode>, PrintMode)> for PrintTuple {
}
}

// Different types of printing commands
pub enum PrintCommand {
Normal,
PrintCalyx,
PrintNodes,
}

/// A command that can be sent to the debugger.
pub enum Command {
/// Advance the execution by a given number of steps (cycles).
Expand Down Expand Up @@ -345,7 +352,7 @@ pub enum Command {
PrintMode,
),
/// Print the current program counter
PrintPC(bool),
PrintPC(PrintCommand),
/// Show command examples
Explain,
/// Restart the debugger from the beginning of the execution. Command history, breakpoints, watchpoints, etc. are preserved.
Expand Down
15 changes: 11 additions & 4 deletions interp/src/debugger/debugger_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use super::{
source::structures::NewSourceMap,
};
use crate::{
debugger::{source::SourceMap, unwrap_error_message},
debugger::{
commands::PrintCommand, source::SourceMap, unwrap_error_message,
},
errors::{InterpreterError, InterpreterResult},
flatten::{
flat_ir::prelude::GroupIdx,
Expand Down Expand Up @@ -360,10 +362,15 @@ impl<C: AsRef<Context> + Clone> Debugger<C> {
Command::InfoWatch => self
.debugging_context
.print_watchpoints(self.interpreter.env()),
Command::PrintPC(_override_flag) => {
self.interpreter.print_pc();
}

Command::PrintPC(print_mode) => match print_mode {
PrintCommand::Normal | PrintCommand::PrintCalyx => {
self.interpreter.print_pc();
}
PrintCommand::PrintNodes => {
self.interpreter.print_pc_string();
}
},
Command::Explain => {
print!("{}", Command::get_explain_string())
}
Expand Down
22 changes: 22 additions & 0 deletions interp/src/flatten/structures/environment/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use owo_colors::OwoColorize;
use slog::warn;
use std::fmt::Debug;
use std::fmt::Write;
use std::slice::Iter;

pub type PortMap = IndexedMap<GlobalPortIdx, PortValue>;

Expand Down Expand Up @@ -284,6 +285,11 @@ impl<C: AsRef<Context> + Clone> Environment<C> {
pub fn ctx(&self) -> &Context {
self.ctx.as_ref()
}

pub fn pc_iter(&self) -> Iter<'_, ControlPoint> {
self.pc.iter()
}

/// Returns the full name and port list of each cell in the context
pub fn iter_cells(
&self,
Expand Down Expand Up @@ -721,6 +727,18 @@ impl<C: AsRef<Context> + Clone> Environment<C> {
}
}

pub fn print_pc_string(&self) {
let current_nodes = self.pc.iter();
let ctx = &self.ctx.as_ref();
for node in current_nodes {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This & is unnecessary

Suggested change
for node in current_nodes {
let ctx = self.ctx.as_ref();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why this ended up on the wrong line

println!(
"{}: {}",
self.get_full_name(node.comp),
node.string_path(ctx)
);
}
}

fn get_name_from_cell_and_parent(
&self,
parent: GlobalCellIdx,
Expand Down Expand Up @@ -1268,6 +1286,10 @@ impl<C: AsRef<Context> + Clone> Simulator<C> {
self.env.print_pc()
}

pub fn print_pc_string(&self) {
self.env.print_pc_string()
}

/// Pins the port with the given name to the given value. This may only be
/// used for input ports on the entrypoint component (excluding the go port)
/// and will panic if used otherwise. Intended for external use.
Expand Down
61 changes: 61 additions & 0 deletions interp/src/flatten/structures/environment/program_counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,67 @@ impl ControlPoint {
false
}
}

/// Returns a string showing the path from the root node to input node.
/// How to get context?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove /// How to get context? also specify that the path is displayed in the
minimal metadata path syntax

pub fn string_path(&self, ctx: &Context) -> String {
// Does it matter if it takes ownership?
let path = SearchPath::find_path_from_root(self.control_node_idx, ctx);
let control_map = &ctx.primary.control;
let mut string_path = String::from("");
let mut count = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer new over from with an empty &str

Suggested change
let mut count = -1;
let mut string_path = String::new();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess my suggestion thing is just cursed, this also belongs on the line above

let mut body = false;
let mut if_branches: HashMap<ControlIdx, String> = HashMap::new();
Comment on lines +74 to +76
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach is fine and there's no need to change it here, but wanted
to flag the fact that if you zip the path with itself staggered by one
element, you can look at both the current node and the parent without needing
auxiliary structures. This can either be accomplished by processing the first
node specially or by appending a None value and mapping the parent field to an
option

for search_node in path.path {
// The control_idx should exist in the map, so we shouldn't worry about it
// exploding. First SearchNode is root, hence "."
let control_idx = search_node.node;
let control_node = control_map.get(control_idx).unwrap();
match control_node {
// These are terminal nodes
// ControlNode::Empty(_) => "empty",
// ControlNode::Invoke(_) => "invoke",
// ControlNode::Enable(_) => "enable",

// These have unbounded children
// ControlNode::Seq(_) => "seq",
// ControlNode::Par(_) => "par",

// Special cases
ControlNode::If(if_node) => {
if_branches.insert(if_node.tbranch(), String::from("t"));
if_branches.insert(if_node.tbranch(), String::from("f"));
}
ControlNode::While(_) => {
body = true;
}
ControlNode::Repeat(_) => {
body = true;
}
_ => {}
};
// At root, at end to process logic above.
if string_path.is_empty() {
string_path += ".";
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoist this out of the for loop and use in initialization of the string_path

let control_type = if body {
body = false;
count = -1;
String::from("b")
} else if if_branches.contains_key(&control_idx) {
let (_, branch) =
if_branches.get_key_value(&control_idx).unwrap();
branch.clone()
} else {
count += 1;
count.to_string()
};

string_path = string_path + "-" + &control_type;
}
string_path
}
}

#[derive(Debug, Clone)]
Expand Down
Loading