Merge branch 'master' into builder-infer-width

CHANGELOG.md

@@ -1,4 +1,11 @@
 ## Unreleased
+- 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`.
+
+## 0.5.0
 - Don't require `@clk` and `@reset` ports in `comb` components
 - `inline` pass supports inlining `ref` cells
 - `comb-prop`: disable rewrite from `wire.in = port` when the output of a wire is read.

Cargo.toml

@@ -56,7 +56,7 @@ features = ["matrix_graph"]
 [package]
 name = "calyx"
 default-run = "calyx"
-version.workspace = true
+version = "0.5.1"
 edition.workspace = true
 description.workspace = true
 authors.workspace = true

calyx-ir/src/common.rs

@@ -1,4 +1,6 @@
-use calyx_utils::{GetName, Id};
+use calyx_utils::GetName;
+#[cfg(debug_assertions)]
+use calyx_utils::Id;
 use std::cell::RefCell;
 use std::rc::{Rc, Weak};
 
@@ -15,6 +17,7 @@ where
     T: GetName,
 {
     pub(super) internal: Weak<RefCell<T>>,
+    #[cfg(debug_assertions)]
     debug_name: Id,
 }
 
@@ -23,7 +26,10 @@ impl<T: GetName> WRC<T> {
     /// pointer.
     pub fn upgrade(&self) -> RRC<T> {
         let Some(r) = self.internal.upgrade() else {
-             unreachable!("weak reference points to a dropped. Original object's name: `{}'", self.debug_name)
+            #[cfg(debug_assertions)]
+            unreachable!("weak reference points to a dropped. Original object's name: `{}'", self.debug_name);
+            #[cfg(not(debug_assertions))]
+            unreachable!("weak reference points to a dropped.");
         };
         r
     }
@@ -32,10 +38,10 @@ impl<T: GetName> WRC<T> {
 /// From implementation with the same signature as `Rc::downgrade`.
 impl<T: GetName> From<&RRC<T>> for WRC<T> {
     fn from(internal: &RRC<T>) -> Self {
-        let debug_name = internal.borrow().name();
         Self {
             internal: Rc::downgrade(internal),
-            debug_name,
+            #[cfg(debug_assertions)]
+            debug_name: internal.borrow().name(),
         }
     }
 }
@@ -45,6 +51,7 @@ impl<T: GetName> Clone for WRC<T> {
     fn clone(&self) -> Self {
         Self {
             internal: Weak::clone(&self.internal),
+            #[cfg(debug_assertions)]
             debug_name: self.debug_name,
         }
     }

calyx-ir/src/rewriter.rs

@@ -11,22 +11,24 @@ pub type RewriteMap<T> = HashMap<ir::Id, RRC<T>>;
 /// [ir::Port::canonical]) to the new [ir::Port] instance.
 pub type PortRewriteMap = HashMap<ir::Canonical, RRC<ir::Port>>;
 
+#[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<ir::Cell>,
-    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<ir::Cell>,
+    /// Mapping from names of groups to group instance.
+    pub group_map: RewriteMap<ir::Group>,
+    /// Mapping from names of combinational groups to combinational group instance.
+    pub comb_group_map: RewriteMap<ir::CombGroup>,
+    /// Mapping from names of static groups to static group instance.
+    pub static_group_map: RewriteMap<ir::StaticGroup>,
 }
 
-impl<'a> Rewriter<'a> {
-    pub fn new(
-        cell_map: &'a RewriteMap<ir::Cell>,
-        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<RRC<ir::Cell>> {
         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<ir::Cell>] and a [RewriteMap<ir::CombGroup>]
-    pub fn rewrite_invoke(
-        &self,
-        inv: &mut ir::Invoke,
-        comb_group_map: &RewriteMap<ir::CombGroup>,
-    ) {
+    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<ir::StaticGroup>,
-    ) {
+    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<ir::Group>,
-        comb_group_map: &RewriteMap<ir::CombGroup>,
-        static_group_map: &RewriteMap<ir::StaticGroup>,
-    ) {
+    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),
         }
     }
 }

calyx-ir/src/structure.rs

@@ -7,7 +7,7 @@ use super::{
     Attributes, Direction, GetAttributes, Guard, Id, PortDef, RRC, WRC,
 };
 use calyx_frontend::Attribute;
-use calyx_utils::GetName;
+use calyx_utils::{CalyxResult, Error, GetName};
 use itertools::Itertools;
 use smallvec::{smallvec, SmallVec};
 use std::hash::Hash;
@@ -292,31 +292,45 @@ impl Cell {
             .map(Rc::clone)
     }
 
-    /// Get a reference to the first port that has the attribute `attr`.
-    pub fn find_with_attr<A>(&self, attr: A) -> Option<RRC<Port>>
+    /// Return all ports that have the attribute `attr`.
+    pub fn find_all_with_attr<A>(
+        &self,
+        attr: A,
+    ) -> impl Iterator<Item = RRC<Port>> + '_
     where
         A: Into<Attribute>,
     {
         let attr = attr.into();
         self.ports
             .iter()
-            .find(|&g| g.borrow().attributes.has(attr))
+            .filter(move |&p| p.borrow().attributes.has(attr))
             .map(Rc::clone)
     }
 
-    /// Return all ports that have the attribute `attr`.
-    pub fn find_all_with_attr<A>(
+    /// Return the unique port with the given attribute.
+    /// If multiple ports have the same attribute, then we panic.
+    /// If there are not ports with the give attribute, then we return None.
+    pub fn find_unique_with_attr<A>(
         &self,
         attr: A,
-    ) -> impl Iterator<Item = RRC<Port>> + '_
+    ) -> CalyxResult<Option<RRC<Port>>>
     where
         A: Into<Attribute>,
     {
         let attr = attr.into();
-        self.ports
-            .iter()
-            .filter(move |&p| p.borrow().attributes.has(attr))
-            .map(Rc::clone)
+        let mut ports = self.find_all_with_attr(attr);
+        if let Some(port) = ports.next() {
+            if ports.next().is_some() {
+                Err(Error::malformed_structure(format!(
+                    "Multiple ports with attribute `{}` found on cell `{}`",
+                    attr, self.name
+                )))
+            } else {
+                Ok(Some(port))
+            }
+        } else {
+            Ok(None)
+        }
     }
 
     /// Get a reference to the named port and throw an error if it doesn't
@@ -362,18 +376,18 @@ impl Cell {
         }
     }
 
-    /// Get a reference to the first port with the attribute `attr` and throw an error if none
-    /// exist.
-    pub fn get_with_attr<A>(&self, attr: A) -> RRC<Port>
+    /// Get the unique port with the given attribute.
+    /// Panic if no port with the attribute is found and returns an error if multiple ports with the attribute are found.
+    pub fn get_unique_with_attr<A>(&self, attr: A) -> CalyxResult<RRC<Port>>
     where
         A: Into<Attribute> + std::fmt::Display + Copy,
     {
-        self.find_with_attr(attr).unwrap_or_else(|| {
+        Ok(self.find_unique_with_attr(attr)?.unwrap_or_else(|| {
             panic!(
                 "Port with attribute `{attr}' not found on cell `{}'",
                 self.name,
             )
-        })
+        }))
     }
 
     /// Returns the name of the component that is this cells type.

calyx-opt/src/analysis/variable_detection.rs

@@ -37,7 +37,8 @@ impl VariableDetection {
         // if guard is empty, because if it isn't this would show up as
         // a write
         let graph = GraphAnalysis::from(&*group);
-        let go_port = cell.find_with_attr(ir::NumAttr::Go)?;
+        let go_port =
+            cell.find_unique_with_attr(ir::NumAttr::Go).ok().flatten()?;
         let activation = graph
             .writes_to(&go_port.borrow())
             .map(|src| src.borrow().is_constant(1, 1))

calyx-opt/src/default_passes.rs

@@ -89,9 +89,9 @@ impl PassManager {
                 InferShare,
                 ComponentInliner,
                 CombProp,
+                CellShare, // LiveRangeAnalaysis should handle comb groups
+                SimplifyWithControl, // Must run before compile-invoke
                 CompileRef, //Must run before cell-share, and before component-inliner
-                CellShare,  // LiveRangeAnalaysis should handle comb groups
-                SimplifyWithControl, // Must run before infer-static-timing
                 CompileInvoke, // creates dead comb groups
                 AttributePromotion,
                 StaticPromotion,