From ce58e29edde57989ca800ce51d43429e36b7e403 Mon Sep 17 00:00:00 2001 From: Griffin Berlstein Date: Mon, 16 Sep 2024 13:52:38 -0400 Subject: [PATCH] [Cider] Improve conflicting assignment error message (#2282) Small tweak for cider which modifies the way errors are processed and improves the error message for conflicting assignments to print both assignments and their origin. Also adds the data converter flags to the `fud2` flow. --- fud2/scripts/cider.rhai | 9 +- ...ests__test@calyx_through_cider_to_dat.snap | 8 +- .../tests__test@calyx_to_cider-debug.snap | 8 +- .../snapshots/tests__test@plan_cider.snap | 8 +- .../snapshots/tests__test@plan_debug.snap | 8 +- interp/src/errors.rs | 107 ++++++++++++++++-- interp/src/flatten/flat_ir/base.rs | 11 ++ interp/src/flatten/flat_ir/component.rs | 99 ++++++++++++++++ interp/src/flatten/primitives/builder.rs | 19 +++- .../flatten/primitives/stateful/memories.rs | 37 ++++-- interp/src/flatten/structures/context.rs | 32 +++++- .../src/flatten/structures/environment/env.rs | 41 ++----- interp/src/serialization/data_dump.rs | 6 +- interp/tests/runt.toml | 1 + 14 files changed, 323 insertions(+), 71 deletions(-) diff --git a/fud2/scripts/cider.rhai b/fud2/scripts/cider.rhai index 67bca34d6b..c705457864 100644 --- a/fud2/scripts/cider.rhai +++ b/fud2/scripts/cider.rhai @@ -22,13 +22,16 @@ fn cider_setup(e) { ); e.arg("pool", "console"); + e.config_var_or("converter-flags", "cider.converter-flags", ""); + e.config_var_or("cider-flags", "cider.flags", ""); + e.rule( "run-cider", - "$cider-exe -l $calyx-base --data data.dump $in > $out", + "$cider-exe -l $calyx-base --data data.dump $cider-flags $in > $out", ); - e.rule("dump-to-interp", "$cider-converter --to cider $in > $out"); - e.rule("interp-to-dump", "$cider-converter --to json $in > $out"); + e.rule("dump-to-interp", "$cider-converter --to cider $converter-flags $in > $out"); + e.rule("interp-to-dump", "$cider-converter --to json $converter-flags $in > $out"); e.build_cmd( ["data.dump"], "dump-to-interp", diff --git a/fud2/tests/snapshots/tests__test@calyx_through_cider_to_dat.snap b/fud2/tests/snapshots/tests__test@calyx_through_cider_to_dat.snap index 1f8b731cea..b8dceedaec 100644 --- a/fud2/tests/snapshots/tests__test@calyx_through_cider_to_dat.snap +++ b/fud2/tests/snapshots/tests__test@calyx_through_cider_to_dat.snap @@ -35,12 +35,14 @@ cider-converter = $calyx-base/target/debug/cider-data-converter rule run-cider-debug command = $cider-exe -l $calyx-base --data data.dump $in debug || true pool = console +converter-flags = +cider-flags = rule run-cider - command = $cider-exe -l $calyx-base --data data.dump $in > $out + command = $cider-exe -l $calyx-base --data data.dump $cider-flags $in > $out rule dump-to-interp - command = $cider-converter --to cider $in > $out + command = $cider-converter --to cider $converter-flags $in > $out rule interp-to-dump - command = $cider-converter --to json $in > $out + command = $cider-converter --to json $converter-flags $in > $out build data.dump: dump-to-interp $sim_data | $cider-converter build pseudo_cider: calyx-with-flags _from_stdin_calyx.futil diff --git a/fud2/tests/snapshots/tests__test@calyx_to_cider-debug.snap b/fud2/tests/snapshots/tests__test@calyx_to_cider-debug.snap index a88b6ace90..b2e030ec6f 100644 --- a/fud2/tests/snapshots/tests__test@calyx_to_cider-debug.snap +++ b/fud2/tests/snapshots/tests__test@calyx_to_cider-debug.snap @@ -37,12 +37,14 @@ cider-converter = $calyx-base/target/debug/cider-data-converter rule run-cider-debug command = $cider-exe -l $calyx-base --data data.dump $in debug || true pool = console +converter-flags = +cider-flags = rule run-cider - command = $cider-exe -l $calyx-base --data data.dump $in > $out + command = $cider-exe -l $calyx-base --data data.dump $cider-flags $in > $out rule dump-to-interp - command = $cider-converter --to cider $in > $out + command = $cider-converter --to cider $converter-flags $in > $out rule interp-to-dump - command = $cider-converter --to json $in > $out + command = $cider-converter --to json $converter-flags $in > $out build data.dump: dump-to-interp $sim_data | $cider-converter build pseudo_cider: calyx-with-flags _from_stdin_calyx.futil diff --git a/fud2/tests/snapshots/tests__test@plan_cider.snap b/fud2/tests/snapshots/tests__test@plan_cider.snap index 90ae62de2e..e819ea9e11 100644 --- a/fud2/tests/snapshots/tests__test@plan_cider.snap +++ b/fud2/tests/snapshots/tests__test@plan_cider.snap @@ -35,12 +35,14 @@ cider-converter = $calyx-base/target/debug/cider-data-converter rule run-cider-debug command = $cider-exe -l $calyx-base --data data.dump $in debug || true pool = console +converter-flags = +cider-flags = rule run-cider - command = $cider-exe -l $calyx-base --data data.dump $in > $out + command = $cider-exe -l $calyx-base --data data.dump $cider-flags $in > $out rule dump-to-interp - command = $cider-converter --to cider $in > $out + command = $cider-converter --to cider $converter-flags $in > $out rule interp-to-dump - command = $cider-converter --to json $in > $out + command = $cider-converter --to json $converter-flags $in > $out build data.dump: dump-to-interp $sim_data | $cider-converter build interp_out.dump: run-cider /input.ext | data.dump diff --git a/fud2/tests/snapshots/tests__test@plan_debug.snap b/fud2/tests/snapshots/tests__test@plan_debug.snap index 1949c09626..4258438c0a 100644 --- a/fud2/tests/snapshots/tests__test@plan_debug.snap +++ b/fud2/tests/snapshots/tests__test@plan_debug.snap @@ -37,12 +37,14 @@ cider-converter = $calyx-base/target/debug/cider-data-converter rule run-cider-debug command = $cider-exe -l $calyx-base --data data.dump $in debug || true pool = console +converter-flags = +cider-flags = rule run-cider - command = $cider-exe -l $calyx-base --data data.dump $in > $out + command = $cider-exe -l $calyx-base --data data.dump $cider-flags $in > $out rule dump-to-interp - command = $cider-converter --to cider $in > $out + command = $cider-converter --to cider $converter-flags $in > $out rule interp-to-dump - command = $cider-converter --to json $in > $out + command = $cider-converter --to json $converter-flags $in > $out build data.dump: dump-to-interp $sim_data | $cider-converter build /output.ext: run-cider-debug /input.ext | data.dump diff --git a/interp/src/errors.rs b/interp/src/errors.rs index 6c60a63e98..62e7ed55c0 100644 --- a/interp/src/errors.rs +++ b/interp/src/errors.rs @@ -1,4 +1,10 @@ -use crate::flatten::flat_ir::prelude::AssignedValue; +use crate::flatten::{ + flat_ir::{ + base::{AssignmentWinner, ComponentIdx, GlobalCellIdx, GlobalPortIdx}, + prelude::AssignedValue, + }, + structures::environment::Environment, +}; use crate::values::Value; use calyx_ir::Id; use calyx_utils::{Error as CalyxError, MultiError as CalyxMultiError}; @@ -17,6 +23,16 @@ impl BoxedInterpreterError { pub fn inner_mut(&mut self) -> &mut InterpreterError { &mut self.0 } + + pub fn prettify_message< + C: AsRef + Clone, + >( + mut self, + env: &Environment, + ) -> Self { + self.0 = Box::new(self.0.prettify_message(env)); + self + } } impl std::fmt::Display for BoxedInterpreterError { @@ -112,6 +128,7 @@ pub enum InterpreterError { " )] FlatConflictingAssignments { + target: GlobalPortIdx, a1: AssignedValue, a2: AssignedValue, }, @@ -157,28 +174,32 @@ pub enum InterpreterError { /// memory. Contains the name of the register or memory as a string //TODO Griffin: Make this more descriptive #[error( - "Attempted to write an undefined value to register or memory named \"{0}\"" + "Attempted to write an undefined value to register or memory named \"{0:?}\"" )] - UndefinedWrite(String), + UndefinedWrite(GlobalCellIdx), /// The error for attempting to write to an undefined memory address. This /// is distinct from writing to an out of bounds address. //TODO Griffin: Make this more descriptive #[error( - "Attempted to write an undefined memory address in memory named \"{0}\"" + "Attempted to write an undefined memory address in memory named \"{0:?}\"" )] - UndefinedWriteAddr(String), + UndefinedWriteAddr(GlobalCellIdx), /// The error for attempting to read from an undefined memory address. This /// is distinct from reading from an out of bounds address. #[error( - "Attempted to read an undefined memory address from memory named \"{0}\"" + "Attempted to read an undefined memory address from memory named \"{0:?}\"" )] - UndefinedReadAddr(String), + UndefinedReadAddr(GlobalCellIdx), /// A wrapper for serialization errors #[error(transparent)] SerializationError(#[from] crate::serialization::SerializationError), + + /// A nonspecific error, used for arbitrary messages + #[error("{0}")] + GenericError(String), } // this is silly but needed to make the program print something sensible when returning @@ -206,3 +227,75 @@ impl From for InterpreterError { CalyxError::invalid_file(err.to_string()).into() } } + +impl InterpreterError { + pub fn prettify_message< + C: AsRef + Clone, + >( + self, + env: &Environment, + ) -> Self { + fn assign_to_string + Clone>( + assign: &AssignedValue, + env: &Environment, + ) -> ( + String, + Option<(ComponentIdx, crate::flatten::flat_ir::component::AssignmentDefinitionLocation)>, + ){ + match assign.winner() { + AssignmentWinner::Cell => ("Cell".to_string(), None), + AssignmentWinner::Implicit => ("Implicit".to_string(), None), + AssignmentWinner::Assign(idx) => { + let (comp, loc) = + env.ctx().find_assignment_definition(*idx); + + let str = env.ctx().printer().print_assignment(comp, *idx); + (str, Some((comp, loc))) + } + } + } + + fn source_to_string< + C: AsRef + Clone, + >( + source: &crate::flatten::flat_ir::component::AssignmentDefinitionLocation, + comp: ComponentIdx, + env: &Environment, + ) -> String { + let comp_name = env.ctx().lookup_name(comp); + match source { + crate::flatten::flat_ir::component::AssignmentDefinitionLocation::CombGroup(g) => format!(" in comb group {comp_name}::{}", env.ctx().lookup_name(*g)), + crate::flatten::flat_ir::component::AssignmentDefinitionLocation::Group(g) => format!(" in group {comp_name}::{}", env.ctx().lookup_name(*g)), + crate::flatten::flat_ir::component::AssignmentDefinitionLocation::ContinuousAssignment => format!(" in {comp_name}'s continuous assignments"), + //TODO Griffin: Improve the identification of the invoke + crate::flatten::flat_ir::component::AssignmentDefinitionLocation::Invoke(_) => format!(" in an invoke in {comp_name}"), + } + } + + match self { + InterpreterError::FlatConflictingAssignments { target, a1, a2 } => { + let (a1_str, a1_source) = assign_to_string(&a1, env); + let (a2_str, a2_source) = assign_to_string(&a2, env); + + let a1_v = a1.val(); + let a2_v = a2.val(); + let a1_source = a1_source + .map(|(comp, s)| source_to_string(&s, comp, env)) + .unwrap_or_default(); + let a2_source = a2_source + .map(|(comp, s)| source_to_string(&s, comp, env)) + .unwrap_or_default(); + + let target = env.get_full_name(target); + + InterpreterError::GenericError( + format!("conflicting assignments to port \"{target}\":\n 1. assigned {a1_v} by {a1_str}{a1_source}\n 2. assigned {a2_v} by {a2_str}{a2_source}") + ) + } + InterpreterError::UndefinedWrite(c) => InterpreterError::GenericError(format!("Attempted to write an undefined value to register or memory named \"{}\"", env.get_full_name(c))), + InterpreterError::UndefinedWriteAddr(c) => InterpreterError::GenericError(format!("Attempted to write to an undefined memory address in memory named \"{}\"", env.get_full_name(c))), + InterpreterError::UndefinedReadAddr(c) => InterpreterError::GenericError(format!("Attempted to read from an undefined memory address from memory named \"{}\"", env.get_full_name(c))), + e => e, + } + } +} diff --git a/interp/src/flatten/flat_ir/base.rs b/interp/src/flatten/flat_ir/base.rs index eda7f66191..7b5fbed8ac 100644 --- a/interp/src/flatten/flat_ir/base.rs +++ b/interp/src/flatten/flat_ir/base.rs @@ -402,6 +402,17 @@ pub enum AssignmentWinner { Assign(AssignmentIdx), } +impl AssignmentWinner { + #[must_use] + pub fn as_assign(&self) -> Option<&AssignmentIdx> { + if let Self::Assign(v) = self { + Some(v) + } else { + None + } + } +} + impl From for AssignmentWinner { fn from(v: AssignmentIdx) -> Self { Self::Assign(v) diff --git a/interp/src/flatten/flat_ir/component.rs b/interp/src/flatten/flat_ir/component.rs index 8606dad029..363e300bda 100644 --- a/interp/src/flatten/flat_ir/component.rs +++ b/interp/src/flatten/flat_ir/component.rs @@ -1,3 +1,4 @@ +use super::super::structures::context::Context; use crate::flatten::structures::{ index_trait::{IndexRange, SignatureRange}, indexed_map::IndexedMap, @@ -71,6 +72,104 @@ pub struct ComponentCore { pub done: LocalPortOffset, } +pub enum AssignmentDefinitionLocation { + /// The assignment is contained in a comb group + CombGroup(CombGroupIdx), + /// The assignment is contained in a group + Group(GroupIdx), + /// The assignment is one of the continuous assignments for the component + ContinuousAssignment, + /// The assignment is implied by an invoke + Invoke(ControlIdx), +} + +impl ComponentCore { + /// Returns true if the given assignment is contained in this component. + /// + /// Note: This is not a very efficient implementation since it's doing a + /// DFS search over the control tree. + pub fn contains_assignment( + &self, + ctx: &Context, + assign: AssignmentIdx, + ) -> Option { + if self.continuous_assignments.contains(assign) { + return Some(AssignmentDefinitionLocation::ContinuousAssignment); + } else if let Some(root) = self.control { + let mut search_stack = vec![root]; + while let Some(node) = search_stack.pop() { + match &ctx.primary[node] { + ControlNode::Empty(_) => {} + ControlNode::Enable(e) => { + if ctx.primary[e.group()].assignments.contains(assign) { + return Some(AssignmentDefinitionLocation::Group( + e.group(), + )); + } + } + ControlNode::Seq(s) => { + for stmt in s.stms() { + search_stack.push(*stmt); + } + } + ControlNode::Par(p) => { + for stmt in p.stms() { + search_stack.push(*stmt); + } + } + ControlNode::If(i) => { + if let Some(comb) = i.cond_group() { + if ctx.primary[comb].assignments.contains(assign) { + return Some( + AssignmentDefinitionLocation::CombGroup( + comb, + ), + ); + } + } + + search_stack.push(i.tbranch()); + search_stack.push(i.fbranch()); + } + ControlNode::While(wh) => { + if let Some(comb) = wh.cond_group() { + if ctx.primary[comb].assignments.contains(assign) { + return Some( + AssignmentDefinitionLocation::CombGroup( + comb, + ), + ); + } + } + search_stack.push(wh.body()); + } + ControlNode::Repeat(r) => { + search_stack.push(r.body); + } + ControlNode::Invoke(i) => { + if let Some(comb) = i.comb_group { + if ctx.primary[comb].assignments.contains(assign) { + return Some( + AssignmentDefinitionLocation::CombGroup( + comb, + ), + ); + } + } + + if i.assignments.contains(assign) { + return Some(AssignmentDefinitionLocation::Invoke( + node, + )); + } + } + } + } + } + None + } +} + #[derive(Debug, Clone)] /// Other information about a component definition. This is not on the hot path /// and is instead needed primarily during setup and error reporting. diff --git a/interp/src/flatten/primitives/builder.rs b/interp/src/flatten/primitives/builder.rs index f12d239792..f82551fb81 100644 --- a/interp/src/flatten/primitives/builder.rs +++ b/interp/src/flatten/primitives/builder.rs @@ -4,6 +4,7 @@ use super::{combinational::*, stateful::*, Primitive}; use crate::{ flatten::{ flat_ir::{ + base::GlobalCellIdx, cell_prototype::{ CellPrototype, DoubleWidthType, FXType, MemType, SingleWidthType, TripleWidthType, @@ -19,6 +20,8 @@ use crate::{ pub fn build_primitive( prim: &CellInfo, base_port: GlobalPortIdx, + // the global idx of the instantiated primitive + cell_idx: GlobalCellIdx, // extras for memory initialization ctx: &Context, dump: &Option, @@ -38,7 +41,9 @@ pub fn build_primitive( "Build primitive erroneously called on a calyx component" ), CellPrototype::SingleWidth { op, width } => match op { - SingleWidthType::Reg => Box::new(StdReg::new(base_port, *width)), + SingleWidthType::Reg => { + Box::new(StdReg::new(base_port, cell_idx, *width)) + } SingleWidthType::Not => Box::new(StdNot::new(base_port)), SingleWidthType::And => Box::new(StdAnd::new(base_port)), SingleWidthType::Or => Box::new(StdOr::new(base_port)), @@ -171,16 +176,20 @@ pub fn build_primitive( MemType::Seq => Box::new(if let Some(data) = data { memories_initialized .insert(ctx.resolve_id(prim.name).clone()); - SeqMem::new_with_init(base_port, *width, false, dims, data) + SeqMem::new_with_init( + base_port, cell_idx, *width, false, dims, data, + ) } else { - SeqMemD1::new(base_port, *width, false, dims) + SeqMemD1::new(base_port, cell_idx, *width, false, dims) }), MemType::Std => Box::new(if let Some(data) = data { memories_initialized .insert(ctx.resolve_id(prim.name).clone()); - CombMem::new_with_init(base_port, *width, false, dims, data) + CombMem::new_with_init( + base_port, cell_idx, *width, false, dims, data, + ) } else { - CombMem::new(base_port, *width, false, dims) + CombMem::new(base_port, cell_idx, *width, false, dims) }), } } diff --git a/interp/src/flatten/primitives/stateful/memories.rs b/interp/src/flatten/primitives/stateful/memories.rs index a33a0245e2..ceb05badce 100644 --- a/interp/src/flatten/primitives/stateful/memories.rs +++ b/interp/src/flatten/primitives/stateful/memories.rs @@ -3,7 +3,10 @@ use itertools::Itertools; use crate::{ errors::InterpreterError, flatten::{ - flat_ir::prelude::{AssignedValue, GlobalPortIdx, PortValue}, + flat_ir::{ + base::GlobalCellIdx, + prelude::{AssignedValue, GlobalPortIdx, PortValue}, + }, primitives::{ declare_ports, make_getters, ports, prim_trait::{UpdateResult, UpdateStatus}, @@ -17,6 +20,7 @@ use crate::{ pub struct StdReg { base_port: GlobalPortIdx, + global_idx: GlobalCellIdx, internal_state: Value, done_is_high: bool, } @@ -24,10 +28,15 @@ pub struct StdReg { impl StdReg { declare_ports![IN: 0, WRITE_EN: 1, _CLK: 2, RESET: 3, OUT: 4, DONE: 5]; - pub fn new(base_port: GlobalPortIdx, width: u32) -> Self { + pub fn new( + base_port: GlobalPortIdx, + global_idx: GlobalCellIdx, + width: u32, + ) -> Self { let internal_state = Value::zeroes(width); Self { base_port, + global_idx, internal_state, done_is_high: false, } @@ -51,7 +60,7 @@ impl Primitive for StdReg { } else if port_map[write_en].as_bool().unwrap_or_default() { self.internal_state = port_map[input] .as_option() - .ok_or(InterpreterError::UndefinedWrite(String::new()))? + .ok_or(InterpreterError::UndefinedWrite(self.global_idx))? .val() .clone(); @@ -221,6 +230,7 @@ pub struct CombMem { _width: u32, addresser: MemDx, done_is_high: bool, + global_idx: GlobalCellIdx, } impl CombMem { declare_ports![ @@ -242,6 +252,7 @@ impl CombMem { pub fn new( base: GlobalPortIdx, + global_idx: GlobalCellIdx, width: u32, allow_invalid: bool, size: T, @@ -259,11 +270,13 @@ impl CombMem { _width: width, addresser: MemDx::new(shape), done_is_high: false, + global_idx, } } pub fn new_with_init( base_port: GlobalPortIdx, + global_idx: GlobalCellIdx, width: u32, allow_invalid: bool, size: T, @@ -293,6 +306,7 @@ impl CombMem { _width: width, addresser: MemDx::new(size), done_is_high: false, + global_idx, } } @@ -346,11 +360,11 @@ impl Primitive for CombMem { let done = if write_en && !reset { let addr = addr - .ok_or(InterpreterError::UndefinedWriteAddr(String::new()))?; + .ok_or(InterpreterError::UndefinedWriteAddr(self.global_idx))?; let write_data = port_map[self.write_data()] .as_option() - .ok_or(InterpreterError::UndefinedWrite(String::new()))?; + .ok_or(InterpreterError::UndefinedWrite(self.global_idx))?; self.internal_state[addr] = write_data.val().clone(); self.done_is_high = true; port_map.insert_val(done, AssignedValue::cell_b_high())? @@ -393,6 +407,7 @@ impl Primitive for CombMem { pub struct SeqMem { base_port: GlobalPortIdx, + global_idx: GlobalCellIdx, internal_state: Vec, // TODO griffin: This bool is unused in the actual struct and should either // be removed or @@ -406,6 +421,7 @@ pub struct SeqMem { impl SeqMem { pub fn new>( base: GlobalPortIdx, + global_idx: GlobalCellIdx, width: u32, allow_invalid: bool, size: T, @@ -421,11 +437,13 @@ impl SeqMem { addresser: MemDx::new(shape), done_is_high: false, read_out: PortValue::new_undef(), + global_idx, } } pub fn new_with_init( base_port: GlobalPortIdx, + global_idx: GlobalCellIdx, width: u32, allow_invalid: bool, size: T, @@ -456,6 +474,7 @@ impl SeqMem { addresser: MemDx::new(size), done_is_high: false, read_out: PortValue::new_undef(), + global_idx, } } @@ -539,15 +558,15 @@ impl Primitive for SeqMem { self.done_is_high = true; self.read_out = PortValue::new_undef(); let addr_actual = addr - .ok_or(InterpreterError::UndefinedWriteAddr(String::new()))?; + .ok_or(InterpreterError::UndefinedWriteAddr(self.global_idx))?; let write_data = port_map[self.write_data()] .as_option() - .ok_or(InterpreterError::UndefinedWrite(String::new()))?; + .ok_or(InterpreterError::UndefinedWrite(self.global_idx))?; self.internal_state[addr_actual] = write_data.val().clone(); } else if content_en { self.done_is_high = true; - let addr_actual = - addr.ok_or(InterpreterError::UndefinedReadAddr(String::new()))?; + let addr_actual = addr + .ok_or(InterpreterError::UndefinedReadAddr(self.global_idx))?; self.read_out = PortValue::new_cell(self.internal_state[addr_actual].clone()); } else { diff --git a/interp/src/flatten/structures/context.rs b/interp/src/flatten/structures/context.rs index 374ffd5edd..4592b23c73 100644 --- a/interp/src/flatten/structures/context.rs +++ b/interp/src/flatten/structures/context.rs @@ -2,7 +2,10 @@ use std::ops::Index; use crate::flatten::flat_ir::{ cell_prototype::CellPrototype, - component::{AuxillaryComponentInfo, ComponentCore, ComponentMap}, + component::{ + AssignmentDefinitionLocation, AuxillaryComponentInfo, ComponentCore, + ComponentMap, + }, identifier::IdMap, prelude::{ Assignment, AssignmentIdx, CellDefinitionIdx, CellInfo, CombGroup, @@ -414,6 +417,26 @@ impl Context { pub fn lookup_name(&self, id: T) -> &String { id.lookup_name(self) } + + /// Returns information about where an assignment is defined and the + /// component in which it is defined. + /// + /// # Panics + /// + /// This function will panic if the given assignment is not defined in any + /// component. + pub fn find_assignment_definition( + &self, + target: AssignmentIdx, + ) -> (ComponentIdx, AssignmentDefinitionLocation) { + for (idx, comp) in self.primary.components.iter() { + let found = comp.contains_assignment(self, target); + if let Some(found) = found { + return (idx, found); + } + } + unreachable!("Assignment does not belong to any component"); + } } impl AsRef for &Context { @@ -449,3 +472,10 @@ impl LookupName for Identifier { ctx.resolve_id(*self) } } + +impl LookupName for CombGroupIdx { + #[inline] + fn lookup_name<'ctx>(&self, ctx: &'ctx Context) -> &'ctx String { + ctx.resolve_id(ctx.primary[*self].name()) + } +} diff --git a/interp/src/flatten/structures/environment/env.rs b/interp/src/flatten/structures/environment/env.rs index a02470e9b6..3a2f4af820 100644 --- a/interp/src/flatten/structures/environment/env.rs +++ b/interp/src/flatten/structures/environment/env.rs @@ -100,6 +100,7 @@ impl PortMap { // TODO: Fix to make the error more helpful Some(t) if t.has_conflict_with(&val) => InterpreterResult::Err( InterpreterError::FlatConflictingAssignments { + target, a1: t.clone(), a2: val, } @@ -469,6 +470,7 @@ impl + Clone> Environment { let cell_dyn = primitives::build_primitive( info, port_base, + self.cells.peek_next_idx(), self.ctx.as_ref(), data_map, memories_initialized, @@ -986,25 +988,6 @@ impl + Clone> Environment { .and_then(|x| x.last().copied()) } - pub fn make_nice_error( - &self, - cell: GlobalCellIdx, - mut err: BoxedInterpreterError, - ) -> BoxedInterpreterError { - let mut_err = err.inner_mut(); - - match mut_err { - InterpreterError::UndefinedWrite(s) - | InterpreterError::UndefinedWriteAddr(s) - | InterpreterError::UndefinedReadAddr(s) => { - *s = self.get_full_name(cell); - } - _ => {} - } - - err - } - /// Traverses the given name, and returns the end of the traversal. For /// paths with ref cells this will resolve the concrete cell **currently** /// pointed to by the ref cell. @@ -1618,19 +1601,20 @@ impl + Clone> Simulator { let assigns_bundle = self.get_assignments(self.env.pc.node_slice()); self.simulate_combinational(&assigns_bundle) + .map_err(|e| e.prettify_message(&self.env)) } pub fn step(&mut self) -> InterpreterResult<()> { self.converge()?; - let out: Result<(), (GlobalCellIdx, BoxedInterpreterError)> = { + let out: Result<(), BoxedInterpreterError> = { let mut result = Ok(()); - for (idx, cell) in self.env.cells.iter_mut() { + for (_, cell) in self.env.cells.iter_mut() { match cell { CellLedger::Primitive { cell_dyn } => { let res = cell_dyn.exec_cycle(&mut self.env.ports); if res.is_err() { - result = Err((idx, res.unwrap_err())); + result = Err(res.unwrap_err()); break; } } @@ -1663,7 +1647,7 @@ impl + Clone> Simulator { // insert all the new nodes from the par into the program counter self.env.pc.vec_mut().extend(new_nodes); - out.map_err(|(idx, err)| self.env.make_nice_error(idx, err)) + out.map_err(|err| err.prettify_message(&self.env)) } fn evaluate_control_node( @@ -2040,18 +2024,13 @@ impl + Clone> Simulator { .range() .iter() .filter_map(|x| match &mut self.env.cells[x] { - CellLedger::Primitive { cell_dyn } => Some( - cell_dyn - .exec_comb(&mut self.env.ports) - .map_err(|e| (x, e)), - ), + CellLedger::Primitive { cell_dyn } => { + Some(cell_dyn.exec_comb(&mut self.env.ports)) + } CellLedger::Component(_) => None, }) .fold_ok(UpdateStatus::Unchanged, |has_changed, update| { has_changed | update - }) - .map_err(|(cell_idx, err)| { - self.env.make_nice_error(cell_idx, err) })? .as_bool(); diff --git a/interp/src/serialization/data_dump.rs b/interp/src/serialization/data_dump.rs index acc4577e54..5a71a746d8 100644 --- a/interp/src/serialization/data_dump.rs +++ b/interp/src/serialization/data_dump.rs @@ -466,7 +466,7 @@ mod tests { } use crate::flatten::{ - flat_ir::prelude::GlobalPortIdx, + flat_ir::{base::GlobalCellIdx, prelude::GlobalPortIdx}, primitives::stateful::{CombMemD1, SeqMemD1}, structures::index_trait::IndexRef, }; @@ -475,7 +475,7 @@ mod tests { #[test] fn comb_roundtrip(dump in arb_data_dump()) { for mem in &dump.header.memories { - let memory_prim = CombMemD1::new_with_init(GlobalPortIdx::new(0), mem.width(), false, mem.size(), dump.get_data(&mem.name).unwrap()); + let memory_prim = CombMemD1::new_with_init(GlobalPortIdx::new(0), GlobalCellIdx::new(0), mem.width(), false, mem.size(), dump.get_data(&mem.name).unwrap()); let data = memory_prim.dump_data(); prop_assert_eq!(dump.get_data(&mem.name).unwrap(), data); } @@ -484,7 +484,7 @@ mod tests { #[test] fn seq_roundtrip(dump in arb_data_dump()) { for mem in &dump.header.memories { - let memory_prim = SeqMemD1::new_with_init(GlobalPortIdx::new(0), mem.width(), false, mem.size(), dump.get_data(&mem.name).unwrap()); + let memory_prim = SeqMemD1::new_with_init(GlobalPortIdx::new(0), GlobalCellIdx::new(0), mem.width(), false, mem.size(), dump.get_data(&mem.name).unwrap()); let data = memory_prim.dump_data(); prop_assert_eq!(dump.get_data(&mem.name).unwrap(), data); } diff --git a/interp/tests/runt.toml b/interp/tests/runt.toml index d0924d6c0b..9e00d99b7f 100644 --- a/interp/tests/runt.toml +++ b/interp/tests/runt.toml @@ -143,6 +143,7 @@ fud2 --from calyx --to dat \ --through cider \ -s sim.data={}.data \ -s calyx.args="--log off" \ + -s cider.converter-flags="-r --legacy-quotes" \ {} | jq --sort-keys """