diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d7a8f6d7e..0bfe040c31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Unreleased -- Deprecate `Cell::find_with_attr` in favor of `Cell::find_with_unique_attr`. The former is error-prone because pass logic might implicitly assume that there is only one port with a particular attribute. +- BREAKING: Deprecate `Cell::find_with_attr` in favor of `Cell::find_with_unique_attr`. The former is error-prone because pass logic might implicitly assume that there is only one port with a particular attribute. +- BREAKING: Redesign the `ir::Rewriter` interface to take all the rewrite maps when constructing the `ir::Rewriter` struct. ## 0.5.1 - Change the `calyx` build script to use the `CALYX_PRIMITIVES_DIR` env variable to install primitive libraries. If unset, use `$HOME/.calyx`. diff --git a/calyx-ir/src/rewriter.rs b/calyx-ir/src/rewriter.rs index 297d4ebfac..0d6c65ff57 100644 --- a/calyx-ir/src/rewriter.rs +++ b/calyx-ir/src/rewriter.rs @@ -11,22 +11,24 @@ pub type RewriteMap = HashMap>; /// [ir::Port::canonical]) to the new [ir::Port] instance. pub type PortRewriteMap = HashMap>; +#[derive(Default)] /// A structure to track rewrite maps for ports. Stores both cell rewrites and direct port /// rewrites. Attempts to apply port rewrites first before trying the cell /// rewrite. -pub struct Rewriter<'a> { - cell_map: &'a RewriteMap, - port_map: &'a PortRewriteMap, +pub struct Rewriter { + /// Mapping from canonical names of ports to port instances + pub port_map: PortRewriteMap, + /// Mapping from names of cells to cell instance. + pub cell_map: RewriteMap, + /// Mapping from names of groups to group instance. + pub group_map: RewriteMap, + /// Mapping from names of combinational groups to combinational group instance. + pub comb_group_map: RewriteMap, + /// Mapping from names of static groups to static group instance. + pub static_group_map: RewriteMap, } -impl<'a> Rewriter<'a> { - pub fn new( - cell_map: &'a RewriteMap, - port_map: &'a PortRewriteMap, - ) -> Self { - Self { cell_map, port_map } - } - +impl Rewriter { /// Return the rewrite for a cell pub fn get_cell_rewrite(&self, cell: &ir::Id) -> Option> { self.cell_map.get(cell).map(Rc::clone) @@ -123,11 +125,7 @@ impl<'a> Rewriter<'a> { // =========== Control Rewriting Methods ============= /// Rewrite a `invoke` node using a [RewriteMap] and a [RewriteMap] - pub fn rewrite_invoke( - &self, - inv: &mut ir::Invoke, - comb_group_map: &RewriteMap, - ) { + pub fn rewrite_invoke(&self, inv: &mut ir::Invoke) { // Rewrite the name of the cell let name = inv.comp.borrow().name(); if let Some(new_cell) = &self.get_cell_rewrite(&name) { @@ -137,7 +135,7 @@ impl<'a> Rewriter<'a> { // Rewrite the combinational group if let Some(cg_ref) = &inv.comb_group { let cg = cg_ref.borrow().name(); - if let Some(new_cg) = &comb_group_map.get(&cg) { + if let Some(new_cg) = &self.comb_group_map.get(&cg) { inv.comb_group = Some(Rc::clone(new_cg)); } } @@ -174,34 +172,30 @@ impl<'a> Rewriter<'a> { /// Given a control program, rewrite all uses of cells, groups, and comb groups using the given /// rewrite maps. - pub fn rewrite_static_control( - &self, - sc: &mut ir::StaticControl, - static_group_map: &RewriteMap, - ) { + pub fn rewrite_static_control(&self, sc: &mut ir::StaticControl) { match sc { ir::StaticControl::Empty(_) => (), ir::StaticControl::Enable(sen) => { let g = &sen.group.borrow().name(); - if let Some(new_group) = static_group_map.get(g) { + if let Some(new_group) = self.static_group_map.get(g) { sen.group = Rc::clone(new_group); } } ir::StaticControl::Repeat(rep) => { - self.rewrite_static_control(&mut rep.body, static_group_map) + self.rewrite_static_control(&mut rep.body) } ir::StaticControl::Seq(ir::StaticSeq { stmts, .. }) | ir::StaticControl::Par(ir::StaticPar { stmts, .. }) => stmts .iter_mut() - .for_each(|c| self.rewrite_static_control(c, static_group_map)), + .for_each(|c| self.rewrite_static_control(c)), ir::StaticControl::If(sif) => { // Rewrite port use if let Some(new_port) = self.get(&sif.port) { sif.port = new_port; } // rewrite branches - self.rewrite_static_control(&mut sif.tbranch, static_group_map); - self.rewrite_static_control(&mut sif.fbranch, static_group_map); + self.rewrite_static_control(&mut sif.tbranch); + self.rewrite_static_control(&mut sif.fbranch); } ir::StaticControl::Invoke(sin) => { self.rewrite_static_invoke(sin); @@ -211,31 +205,18 @@ impl<'a> Rewriter<'a> { /// Given a control program, rewrite all uses of cells, groups, and comb groups using the given /// rewrite maps. - pub fn rewrite_control( - &self, - c: &mut ir::Control, - group_map: &RewriteMap, - comb_group_map: &RewriteMap, - static_group_map: &RewriteMap, - ) { + pub fn rewrite_control(&self, c: &mut ir::Control) { match c { ir::Control::Empty(_) => (), ir::Control::Enable(en) => { let g = &en.group.borrow().name(); - if let Some(new_group) = group_map.get(g) { + if let Some(new_group) = self.group_map.get(g) { en.group = Rc::clone(new_group); } } ir::Control::Seq(ir::Seq { stmts, .. }) | ir::Control::Par(ir::Par { stmts, .. }) => { - stmts.iter_mut().for_each(|c| { - self.rewrite_control( - c, - group_map, - comb_group_map, - static_group_map, - ) - }) + stmts.iter_mut().for_each(|c| self.rewrite_control(c)) } ir::Control::If(ife) => { // Rewrite port use @@ -245,23 +226,13 @@ impl<'a> Rewriter<'a> { // Rewrite conditional comb group if defined if let Some(cg_ref) = &ife.cond { let cg = cg_ref.borrow().name(); - if let Some(new_cg) = &comb_group_map.get(&cg) { + if let Some(new_cg) = &self.comb_group_map.get(&cg) { ife.cond = Some(Rc::clone(new_cg)); } } // rewrite branches - self.rewrite_control( - &mut ife.tbranch, - group_map, - comb_group_map, - static_group_map, - ); - self.rewrite_control( - &mut ife.fbranch, - group_map, - comb_group_map, - static_group_map, - ); + self.rewrite_control(&mut ife.tbranch); + self.rewrite_control(&mut ife.fbranch); } ir::Control::While(wh) => { // Rewrite port use @@ -271,33 +242,19 @@ impl<'a> Rewriter<'a> { // Rewrite conditional comb group if defined if let Some(cg_ref) = &wh.cond { let cg = cg_ref.borrow().name(); - if let Some(new_cg) = &comb_group_map.get(&cg) { + if let Some(new_cg) = &self.comb_group_map.get(&cg) { wh.cond = Some(Rc::clone(new_cg)); } } // rewrite body - self.rewrite_control( - &mut wh.body, - group_map, - comb_group_map, - static_group_map, - ); + self.rewrite_control(&mut wh.body); } ir::Control::Repeat(rep) => { // rewrite body - self.rewrite_control( - &mut rep.body, - group_map, - comb_group_map, - static_group_map, - ); - } - ir::Control::Invoke(inv) => { - self.rewrite_invoke(inv, comb_group_map) - } - ir::Control::Static(s) => { - self.rewrite_static_control(s, static_group_map) + self.rewrite_control(&mut rep.body); } + ir::Control::Invoke(inv) => self.rewrite_invoke(inv), + ir::Control::Static(s) => self.rewrite_static_control(s), } } } diff --git a/calyx-opt/src/passes/cell_share.rs b/calyx-opt/src/passes/cell_share.rs index 46ac420f41..def1c69b14 100644 --- a/calyx-opt/src/passes/cell_share.rs +++ b/calyx-opt/src/passes/cell_share.rs @@ -518,8 +518,10 @@ impl Visitor for CellShare { } // Rewrite assignments using the coloring generated. - let empty_map: ir::rewriter::PortRewriteMap = HashMap::new(); - let rewriter = ir::Rewriter::new(&coloring, &empty_map); + let rewriter = ir::Rewriter { + cell_map: coloring, + ..Default::default() + }; comp.for_each_assignment(|assign| { assign.for_each_port(|port| rewriter.get(port)); }); @@ -528,12 +530,7 @@ impl Visitor for CellShare { }); // Rewrite control uses of ports - rewriter.rewrite_control( - &mut comp.control.borrow_mut(), - &HashMap::new(), - &HashMap::new(), - &HashMap::new(), - ); + rewriter.rewrite_control(&mut comp.control.borrow_mut()); Ok(Action::Stop) } diff --git a/calyx-opt/src/passes/comb_prop.rs b/calyx-opt/src/passes/comb_prop.rs index 9ea00ee09b..e4455abb81 100644 --- a/calyx-opt/src/passes/comb_prop.rs +++ b/calyx-opt/src/passes/comb_prop.rs @@ -1,7 +1,6 @@ use crate::traversal::{Action, ConstructVisitor, Named, VisResult, Visitor}; use calyx_ir::{self as ir, RRC}; use itertools::Itertools; -use std::collections::HashMap; use std::rc::Rc; /// A data structure to track rewrites of ports with added functionality to declare @@ -369,14 +368,11 @@ impl Visitor for CombProp { } }); - let cell_rewrites = HashMap::new(); - let rewriter = ir::Rewriter::new(&cell_rewrites, &rewrites); - rewriter.rewrite_control( - &mut comp.control.borrow_mut(), - &HashMap::new(), - &HashMap::new(), - &HashMap::new(), - ); + let rewriter = ir::Rewriter { + port_map: rewrites, + ..Default::default() + }; + rewriter.rewrite_control(&mut comp.control.borrow_mut()); Ok(Action::Stop) } diff --git a/calyx-opt/src/passes/compile_ref.rs b/calyx-opt/src/passes/compile_ref.rs index 6f4293e66b..5eb35c5ffb 100644 --- a/calyx-opt/src/passes/compile_ref.rs +++ b/calyx-opt/src/passes/compile_ref.rs @@ -164,13 +164,11 @@ impl Visitor for CompileRef { _comps: &[ir::Component], ) -> VisResult { log::debug!("compile-ref: {}", comp.name); - self.ref_cells = dump_ports::dump_ports_to_signature( - comp, - is_external_cell, - true, - &mut self.port_names, - &mut self.removed, - ); + let dump_ports::DumpResults { cells, rewrites } = + dump_ports::dump_ports_to_signature(comp, is_external_cell, true); + self.removed.extend(rewrites.clone()); + self.port_names.insert(comp.name, rewrites); + self.ref_cells = cells; // For all subcomponents that had a `ref` cell in them, we need to // update their cell to have the new ports added from inlining the @@ -237,21 +235,19 @@ impl Visitor for CompileRef { _sigs: &LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { + let port_map = std::mem::take(&mut self.removed); // Rewrite all of the ref cell ports - let empty = HashMap::new(); - let rw = ir::Rewriter::new(&empty, &self.removed); + let rw = ir::Rewriter { + port_map, + ..Default::default() + }; comp.for_each_assignment(|assign| { rw.rewrite_assign(assign); }); comp.for_each_static_assignment(|assign| { rw.rewrite_assign(assign); }); - rw.rewrite_control( - &mut comp.control.borrow_mut(), - &HashMap::new(), - &HashMap::new(), - &HashMap::new(), - ); + rw.rewrite_control(&mut comp.control.borrow_mut()); Ok(Action::Continue) } } diff --git a/calyx-opt/src/passes/component_iniliner.rs b/calyx-opt/src/passes/component_iniliner.rs index f2736ae9be..f7dd93d420 100644 --- a/calyx-opt/src/passes/component_iniliner.rs +++ b/calyx-opt/src/passes/component_iniliner.rs @@ -290,21 +290,25 @@ impl ComponentInliner { // Rewrites to inline the interface. let interface_map = Self::inline_interface(builder, comp, name); - let rewrite = ir::Rewriter::new(&cell_map, &interface_map); + let mut rewrite = ir::Rewriter { + cell_map, + port_map: interface_map, + ..Default::default() + }; // For each group, create a new group and rewrite all assignments within // it using the `rewrite_map`. - let group_map: rewriter::RewriteMap = comp + rewrite.group_map = comp .get_groups() .iter() .map(|gr| Self::inline_group(builder, &rewrite, gr)) .collect(); - let static_group_map: rewriter::RewriteMap = comp + rewrite.static_group_map = comp .get_static_groups() .iter() .map(|gr| Self::inline_static_group(builder, &rewrite, gr)) .collect(); - let comb_group_map: rewriter::RewriteMap = comp + rewrite.comb_group_map = comp .comb_groups .iter() .map(|gr| Self::inline_comb_group(builder, &rewrite, gr)) @@ -320,17 +324,12 @@ impl ComponentInliner { // Generate a control program associated with this instance let mut con = ir::Cloner::control(&comp.control.borrow()); - rewrite.rewrite_control( - &mut con, - &group_map, - &comb_group_map, - &static_group_map, - ); + rewrite.rewrite_control(&mut con); // Generate interface map for use in the parent cell. // Return as an iterator because it's immediately merged into the global rewrite map. let rev_interface_map = - interface_map.into_iter().map(move |(cp, pr)| { + rewrite.port_map.into_iter().map(move |(cp, pr)| { let ir::Canonical(_, p) = cp; let port = pr.borrow(); let np = match port.name.id.as_str() { diff --git a/calyx-opt/src/passes/dump_ports.rs b/calyx-opt/src/passes/dump_ports.rs index d9f0fa49bc..532e1ab5d7 100644 --- a/calyx-opt/src/passes/dump_ports.rs +++ b/calyx-opt/src/passes/dump_ports.rs @@ -1,7 +1,17 @@ -use crate::passes::compile_ref::RefPortMap; use calyx_ir::{self as ir, RRC, WRC}; +use ir::rewriter; use itertools::Itertools; -use std::{cell::RefCell, collections::HashMap, rc::Rc}; +use std::{cell::RefCell, rc::Rc}; + +#[derive(Default)] +/// Results generated from the process of dumping out ports. +pub struct DumpResults { + /// The cells that were removed from the component. + pub cells: Vec>, + /// Rewrites from (cell, port) to the new port. + /// Usually consumed by an [`ir::rewriter::Rewriter`]. + pub rewrites: rewriter::PortRewriteMap, +} /// Formats name of a port given the id of the cell and the port pub(super) fn format_port_name(canon: &ir::Canonical) -> ir::Id { @@ -12,15 +22,17 @@ pub(super) fn format_port_name(canon: &ir::Canonical) -> ir::Id { /// the component and inline all the ports of the removed cells to the component /// signature. /// -/// If remove_signals is true, does not inline ports marked with @clk and @reset. -pub(super) fn dump_ports_to_signature( +/// If `remove_clk_and_reset` is true, does not inline ports marked with @clk and @reset. +pub(super) fn dump_ports_to_signature( component: &mut ir::Component, - cell_filter: fn(&RRC) -> bool, - remove_signals: bool, - port_names: &mut RefPortMap, - removed: &mut HashMap>, -) -> Vec> { - let comp_name = component.name; + cell_filter: F, + remove_clk_and_reset: bool, +) -> DumpResults +where + F: Fn(&RRC) -> bool, +{ + let mut removed = rewriter::PortRewriteMap::default(); + let (ext_cells, cells): (Vec<_>, Vec<_>) = component.cells.drain().partition(cell_filter); component.cells.append(cells.into_iter()); @@ -36,8 +48,8 @@ pub(super) fn dump_ports_to_signature( .ports .iter() .filter(|pr| { - let p = pr.borrow(); - if remove_signals { + if remove_clk_and_reset { + let p = pr.borrow(); !p.attributes.has(ir::BoolAttr::Clk) && !p.attributes.has(ir::BoolAttr::Reset) } else { @@ -65,13 +77,10 @@ pub(super) fn dump_ports_to_signature( // Record the port as removed removed.insert(canon.clone(), Rc::clone(&new_port)); - - // Record the port to add to cells - port_names - .entry(comp_name) - .or_default() - .insert(canon, Rc::clone(&new_port)); } } - ext_cells + DumpResults { + cells: ext_cells, + rewrites: removed, + } } diff --git a/calyx-opt/src/passes/externalize.rs b/calyx-opt/src/passes/externalize.rs index 0a8d7002cf..1c743345d1 100644 --- a/calyx-opt/src/passes/externalize.rs +++ b/calyx-opt/src/passes/externalize.rs @@ -2,7 +2,6 @@ use super::dump_ports; use crate::traversal::{Action, ConstructVisitor, Named, VisResult, Visitor}; use calyx_ir::{self as ir, LibrarySignatures, RRC}; use calyx_utils::CalyxResult; -use std::collections::HashMap; /// Externalize input/output ports for cells marked with the `@external(1)` attribute. /// The ports of these cells are exposed through the ports of the parent @@ -75,30 +74,24 @@ impl Visitor for Externalize { _ctx: &LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { - let mut port_names = HashMap::new(); - let mut renamed = HashMap::new(); - let cells = dump_ports::dump_ports_to_signature( - comp, - has_external_attribute, - false, - &mut port_names, - &mut renamed, - ); + let dump_ports::DumpResults { cells, rewrites } = + dump_ports::dump_ports_to_signature( + comp, + has_external_attribute, + false, + ); - let cell_map = HashMap::default(); - let rw = ir::Rewriter::new(&cell_map, &renamed); + let rw = ir::Rewriter { + port_map: rewrites, + ..Default::default() + }; comp.for_each_assignment(|assign| { rw.rewrite_assign(assign); }); comp.for_each_static_assignment(|assign| { rw.rewrite_assign(assign); }); - rw.rewrite_control( - &mut comp.control.borrow_mut(), - &HashMap::new(), - &HashMap::new(), - &HashMap::new(), - ); + rw.rewrite_control(&mut comp.control.borrow_mut()); // Don't allow cells to be dropped before this because otherwise rewriting will fail drop(cells); diff --git a/calyx-opt/src/passes/register_unsharing.rs b/calyx-opt/src/passes/register_unsharing.rs index 772b750e58..c4469da71c 100644 --- a/calyx-opt/src/passes/register_unsharing.rs +++ b/calyx-opt/src/passes/register_unsharing.rs @@ -163,8 +163,10 @@ impl Bookkeeper { for (grp, rename_cells) in grp_map { let group_ref = comp.find_group(grp).unwrap(); let mut group = group_ref.borrow_mut(); - let empty_map = HashMap::new(); - let rewriter = ir::Rewriter::new(&rename_cells, &empty_map); + let rewriter = ir::Rewriter { + cell_map: rename_cells, + ..Default::default() + }; group .assignments .iter_mut() @@ -198,14 +200,18 @@ impl Visitor for RegisterUnsharing { _sigs: &LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { - let book = &self.bookkeeper; + let book = &mut self.bookkeeper; if let Some(name) = book.analysis.meta.fetch_label(invoke) { // only do rewrites if there is actually rewriting to do - if let Some(rename_vec) = book.invoke_map.get(name) { - let empty_map = HashMap::new(); - let rewriter = ir::Rewriter::new(rename_vec, &empty_map); - rewriter.rewrite_invoke(invoke, &HashMap::new()); + if let Some(rename_vec) = book.invoke_map.get_mut(name) { + let cell_map = std::mem::take(rename_vec); + let rewriter = ir::Rewriter { + cell_map, + ..Default::default() + }; + rewriter.rewrite_invoke(invoke); + *rename_vec = rewriter.cell_map; } } @@ -219,14 +225,18 @@ impl Visitor for RegisterUnsharing { _sigs: &LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { - let book = &self.bookkeeper; + let book = &mut self.bookkeeper; if let Some(name) = book.analysis.meta.fetch_label_static(invoke) { // only do rewrites if there is actually rewriting to do - if let Some(rename_vec) = book.invoke_map.get(name) { - let empty_map = HashMap::new(); - let rewriter = ir::Rewriter::new(rename_vec, &empty_map); + if let Some(rename_vec) = book.invoke_map.get_mut(name) { + let cell_map = std::mem::take(rename_vec); + let rewriter = ir::Rewriter { + cell_map, + ..Default::default() + }; rewriter.rewrite_static_invoke(invoke); + *rename_vec = rewriter.cell_map; } }