Introduce ability to pass in subtypes as ref cells in invoke statem…

…ents (#2085)

* WIP: attempt at implementing subtyping. TBD if this works

* typo in structure

* change error for wellformedness subtyping runt test

* compile_invoke now compiles, tbd if actually works

* some clippy fixes

* wip compile invoke changes

* added debugging stuff to invoke, still a WIP

* WIP: Need to do clean up but also this might allow subtyping???? Hope so

* add comments to well-formed

* maybe works?

* seems to be working!

* update runt tests

* make clippy happy

* formatting

* update comments

* more runt tests

* replace hashed map with linkedhashmap for determinism

* fix bug where iterating over too many ports in comp_ports in compile_invoke

* more runt tests for subtyping

* derive partialeq and eq for InLineAttributes

* extract `get_concrete_port` as helper function

* compile_invoke formatting

* WIP: extract require_subtype. TODO: allow it to take in StaticInvoke as well

* factor out subtyping check

* elaborate on some comments in well_formed

* improve comments

* add some info about subtyping to the language reference

calyx-frontend/src/attribute.rs

@@ -195,7 +195,7 @@ impl FromStr for Attribute {
     }
 }
 
-#[derive(Default, Debug, Clone)]
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
 /// Inline storage for boolean attributes.
 pub(super) struct InlineAttributes {
     /// Boolean attributes stored in a 16-bit number.

calyx-frontend/src/attributes.rs

@@ -169,6 +169,20 @@ impl Attributes {
     }
 }
 
+impl PartialEq for Attributes {
+    fn eq(&self, other: &Self) -> bool {
+        self.inl == other.inl
+            && self.hinfo.attrs.len() == other.hinfo.attrs.len()
+            && self
+                .hinfo
+                .attrs
+                .iter()
+                .all(|(k, v)| other.hinfo.attrs.get(k) == Some(v))
+    }
+}
+
+impl Eq for Attributes {}
+
 #[cfg(feature = "serialize")]
 impl serde::Serialize for HeapAttrInfo {
     fn serialize<S>(&self, ser: S) -> Result<S::Ok, S::Error>

calyx-ir/src/rewriter.rs

@@ -1,3 +1,5 @@
+use linked_hash_map::LinkedHashMap;
+
 use crate::control::StaticInvoke;
 use crate::{self as ir, RRC};
 use std::borrow::BorrowMut;
@@ -10,7 +12,7 @@ pub type RewriteMap<T> = HashMap<ir::Id, RRC<T>>;
 
 /// Map to rewrite port uses. Maps the canonical name of an old port (generated using
 /// [ir::Port::canonical]) to the new [ir::Port] instance.
-pub type PortRewriteMap = HashMap<ir::Canonical, RRC<ir::Port>>;
+pub type PortRewriteMap = LinkedHashMap<ir::Canonical, RRC<ir::Port>>;
 
 #[derive(Default)]
 /// A structure to track rewrite maps for ports. Stores both cell rewrites and direct port

calyx-ir/src/structure.rs

@@ -126,6 +126,15 @@ impl Port {
     {
         self.get_attributes().has(attr)
     }
+
+    /// Returns true if the widths, name, direction, and attributes of 2 ports match.
+    /// This is different than `==` equivalence, which only looks at parent and name.
+    pub fn type_equivalent(&self, other: &Port) -> bool {
+        self.width == other.width
+            && self.direction == other.direction
+            && self.name == other.name
+            && self.attributes == other.attributes
+    }
 }
 
 impl GetAttributes for Port {
@@ -215,7 +224,7 @@ pub enum CellType {
 }
 
 impl CellType {
-    /// Return the name associated with this CellType is present
+    /// Return the name associated with this CellType if present
     pub fn get_name(&self) -> Option<Id> {
         match self {
             CellType::Primitive { name, .. } | CellType::Component { name } => {
@@ -244,7 +253,7 @@ impl CellType {
 }
 
 /// Represents an instantiated cell.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 #[cfg_attr(feature = "serialize", derive(serde::Serialize))]
 pub struct Cell {
     /// Name of this cell.

calyx-opt/src/passes/compile_invoke.rs

@@ -6,6 +6,7 @@ use calyx_ir::{self as ir, Attributes, LibrarySignatures};
 use calyx_utils::{CalyxResult, Error};
 use ir::{Assignment, RRC, WRC};
 use itertools::Itertools;
+use linked_hash_map::LinkedHashMap;
 use std::collections::HashMap;
 use std::rc::Rc;
 
@@ -54,21 +55,21 @@ fn build_assignments<T>(
 /// Map for storing added ports for each ref cell
 /// level of Hashmap represents:
 /// HashMap<-component name-, Hashmap<(-ref cell name-,-port name-), port>>;
-struct RefPortMap(HashMap<ir::Id, HashMap<ir::Canonical, RRC<ir::Port>>>);
+struct RefPortMap(HashMap<ir::Id, LinkedHashMap<ir::Canonical, RRC<ir::Port>>>);
 
 impl RefPortMap {
     fn insert(
         &mut self,
         comp_name: ir::Id,
-        ports: HashMap<ir::Canonical, RRC<ir::Port>>,
+        ports: LinkedHashMap<ir::Canonical, RRC<ir::Port>>,
     ) {
         self.0.insert(comp_name, ports);
     }
 
     fn get(
         &self,
         comp_name: &ir::Id,
-    ) -> Option<&HashMap<ir::Canonical, RRC<ir::Port>>> {
+    ) -> Option<&LinkedHashMap<ir::Canonical, RRC<ir::Port>>> {
         self.0.get(comp_name)
     }
 
@@ -91,7 +92,7 @@ pub struct CompileInvoke {
     port_names: RefPortMap,
     /// Mapping from the ports of cells that were removed to the new port on the
     /// component signature.
-    removed: HashMap<ir::Canonical, ir::RRC<ir::Port>>,
+    removed: LinkedHashMap<ir::Canonical, ir::RRC<ir::Port>>,
     /// Ref cells in the component. We hold onto these so that our references don't get invalidated
     ref_cells: Vec<ir::RRC<ir::Cell>>,
 }
@@ -103,7 +104,7 @@ impl ConstructVisitor for CompileInvoke {
     {
         Ok(CompileInvoke {
             port_names: RefPortMap::default(),
-            removed: HashMap::new(),
+            removed: LinkedHashMap::new(),
             ref_cells: Vec::new(),
         })
     }
@@ -131,38 +132,45 @@ impl CompileInvoke {
     ///
     /// Since this pass eliminates all ref cells in post order, we expect that
     /// invoked component already had all of its ref cells removed.
+
     fn ref_cells_to_ports<T>(
         &mut self,
         inv_cell: RRC<ir::Cell>,
         ref_cells: impl Iterator<Item = (ir::Id, ir::RRC<ir::Cell>)>,
     ) -> Vec<ir::Assignment<T>> {
         let inv_comp = inv_cell.borrow().type_name().unwrap();
         let mut assigns = Vec::new();
-        for (ref_cell_name, cell) in ref_cells {
+        for (ref_cell_name, concrete_cell) in ref_cells {
             log::debug!(
                 "Removing ref cell `{}` with {} ports",
                 ref_cell_name,
-                cell.borrow().ports.len()
+                concrete_cell.borrow().ports.len()
             );
 
             // Mapping from canonical names of the ports of the ref cell to the
-            // new port defined on the signature of the component
+            // new port defined on the signature of the component. This has name of ref cell, not arg cell
             let Some(comp_ports) = self.port_names.get(&inv_comp) else {
                 unreachable!("component `{}` invoked but not already visited by the pass", inv_comp)
             };
 
-            // The type of the cell is the same as the ref cell so we can
-            // iterate over its ports and generate bindings for the ref cell.
-            for pr in &cell.borrow().ports {
-                let port = pr.borrow();
-                if port.has_attribute(ir::BoolAttr::Clk)
-                    || port.has_attribute(ir::BoolAttr::Reset)
+            // We expect each canonical port in `comp_ports` to exactly match with a port in
+            //`concrete_cell` based on well-formedness subtype checks.
+            for canon in comp_ports.keys() {
+                //only interested in ports attached to the ref cell
+                if canon.cell != ref_cell_name {
+                    continue;
+                }
+                // The given port of the actual, concrete cell passed in
+                let concrete_port =
+                    Self::get_concrete_port(concrete_cell.clone(), &canon.port);
+
+                if concrete_port.borrow().has_attribute(ir::BoolAttr::Clk)
+                    || concrete_port.borrow().has_attribute(ir::BoolAttr::Reset)
                 {
                     continue;
                 }
 
-                let canon = ir::Canonical::new(ref_cell_name, port.name);
-                let Some(comp_port) = comp_ports.get(&canon) else {
+                let Some(comp_port) = comp_ports.get(canon) else {
                     unreachable!("port `{}` not found in the signature of {}. Known ports are: {}",
                         canon,
                         inv_comp,
@@ -173,7 +181,7 @@ impl CompileInvoke {
                 let ref_port = inv_cell.borrow().get(comp_port.borrow().name);
                 log::debug!("Port `{}` -> `{}`", canon, ref_port.borrow().name);
 
-                let old_port = pr.borrow().canonical();
+                let old_port = concrete_port.borrow().canonical();
                 // If the port has been removed already, get the new port from the component's signature
                 let arg_port = if let Some(sig_pr) = self.removed.get(&old_port)
                 {
@@ -184,10 +192,10 @@ impl CompileInvoke {
                     );
                     Rc::clone(sig_pr)
                 } else {
-                    Rc::clone(pr)
+                    Rc::clone(&concrete_port)
                 };
 
-                match port.direction {
+                match concrete_port.borrow().direction {
                     ir::Direction::Output => {
                         log::debug!(
                             "constructing: {} = {}",
@@ -213,11 +221,34 @@ impl CompileInvoke {
                     _ => {
                         unreachable!("Cell should have inout ports");
                     }
-                }
+                };
             }
         }
         assigns
     }
+
+    /// Takes in a concrete cell (aka an in_cell/what is passed in to a ref cell at invocation)
+    /// and returns the concrete port based on just the port of a canonical id.
+    fn get_concrete_port(
+        concrete_cell: RRC<ir::Cell>,
+        canonical_port: &ir::Id,
+    ) -> RRC<ir::Port> {
+        let concrete_cell = concrete_cell.borrow();
+        concrete_cell
+            .ports
+            .iter()
+            .find(|&concrete_cell_port| {
+                concrete_cell_port.borrow().name == canonical_port
+            })
+            .unwrap_or_else(|| {
+                unreachable!(
+                    "port `{}` not found in the cell `{}`",
+                    canonical_port,
+                    concrete_cell.name()
+                )
+            })
+            .clone()
+    }
 }
 
 impl Visitor for CompileInvoke {
@@ -287,6 +318,7 @@ impl Visitor for CompileInvoke {
         // Assigns representing the ref cell connections
         invoke_group.borrow_mut().assignments.extend(
             self.ref_cells_to_ports(Rc::clone(&s.comp), s.ref_cells.drain(..)),
+            // ), //the clone here is questionable? but lets things type check? Maybe change ref_cells_to_ports to expect a reference?
         );
 
         // comp.go = 1'd1;
@@ -361,7 +393,6 @@ impl Visitor for CompileInvoke {
         _comps: &[ir::Component],
     ) -> VisResult {
         let mut builder = ir::Builder::new(comp, ctx);
-
         let invoke_group = builder.add_static_group("static_invoke", s.latency);
 
         invoke_group.borrow_mut().assignments.extend(

calyx-opt/src/passes/component_iniliner.rs

@@ -7,6 +7,7 @@ use calyx_ir::{self as ir, rewriter, GetAttributes, LibrarySignatures, RRC};
 use calyx_utils::Error;
 use ir::Nothing;
 use itertools::Itertools;
+use linked_hash_map::LinkedHashMap;
 use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 
@@ -71,7 +72,7 @@ impl ComponentInliner {
             always_inline,
             new_fsms,
             control_map: HashMap::default(),
-            interface_rewrites: HashMap::default(),
+            interface_rewrites: LinkedHashMap::default(),
             inlined_cells: Vec::default(),
         }
     }
@@ -442,7 +443,8 @@ impl Visitor for ComponentInliner {
             .collect::<HashMap<_, _>>();
 
         // Rewrites for the interface ports of inlined cells.
-        let mut interface_rewrites: rewriter::PortRewriteMap = HashMap::new();
+        let mut interface_rewrites: rewriter::PortRewriteMap =
+            LinkedHashMap::new();
         // Track names of cells that were inlined.
         let mut inlined_cells = HashSet::new();
         let mut builder = ir::Builder::new(comp, sigs);

calyx-opt/src/passes/discover_external.rs

@@ -4,7 +4,7 @@ use calyx_utils::CalyxResult;
 use ir::RRC;
 use itertools::Itertools;
 use linked_hash_map::LinkedHashMap;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 
 /// A pass to detect cells that have been inlined into the top-level component
 /// and turn them into real cells marked with [ir::BoolAttr::External].
@@ -241,7 +241,7 @@ impl Visitor for DiscoverExternal {
         }
 
         // Rewrite the ports mentioned in the component signature and remove them
-        let mut rewrites: ir::rewriter::PortRewriteMap = HashMap::new();
+        let mut rewrites: ir::rewriter::PortRewriteMap = LinkedHashMap::new();
         for (pre, ports) in port_map {
             // let prim = sigs.get_primitive(pre_to_prim[&pre]);
             let cr = pre_to_cells[&pre].clone();