[Cider] Combinational Component support (#2372)

Small PR which adds support for combinational components and makes some
minor adjustments in a few other places.

The notable difference for the convergence algorithm is that we now sort
the program counter by containment, before iterating over it. This means
we will visit parents before children during convergence and is
necessary for race detection in the presence of invoke chains. While it
is unnecessary when not doing race detection, I've currently elected to
do it unconditionally for simplicity and consistency's sake, but if it
becomes necessary we can move it behind the race detection flag.

interp/src/flatten/flat_ir/base.rs

@@ -642,6 +642,18 @@ impl PortValue {
         self.0.as_ref()
     }
 
+    /// Returns a mutable reference to the underlying [`AssignedValue`] if it is
+    /// defined. Otherwise returns `None`.
+    pub fn as_option_mut(&mut self) -> Option<&mut AssignedValue> {
+        self.0.as_mut()
+    }
+
+    /// Returns the underlying [`AssignedValue`] if it is defined. Otherwise
+    /// returns `None`.
+    pub fn into_option(self) -> Option<AssignedValue> {
+        self.0
+    }
+
     pub fn with_thread(mut self, thread: ThreadIdx) -> Self {
         if let Some(val) = self.0.as_mut() {
             val.thread = Some(thread);
@@ -660,10 +672,6 @@ impl PortValue {
         self.0.as_ref().and_then(|x| x.transitive_clocks())
     }
 
-    pub fn as_option_mut(&mut self) -> Option<&mut AssignedValue> {
-        self.0.as_mut()
-    }
-
     /// If the value is defined, returns the value cast to a boolean. Otherwise
     /// returns `None`. It will panic if the given value is not one bit wide.
     pub fn as_bool(&self) -> Option<bool> {

interp/src/flatten/flat_ir/component.rs

@@ -79,21 +79,119 @@ impl Default for DefinitionRanges {
 }
 
 /// A structure which contains the basic information about a component
-/// definition needed during simulation.
+/// definition needed during simulation. This is for standard (non-combinational)
+/// components
 #[derive(Debug)]
 pub struct ComponentCore {
     /// The control program for this component.
     pub control: Option<ControlIdx>,
     /// The set of assignments that are always active.
     pub continuous_assignments: IndexRange<AssignmentIdx>,
-    /// True iff component is combinational
-    pub is_comb: bool,
     /// The go port for this component
     pub go: LocalPortOffset,
     /// The done port for this component
     pub done: LocalPortOffset,
 }
 
+#[derive(Debug)]
+pub struct CombComponentCore {
+    /// The set of assignments that are always active.
+    pub continuous_assignments: IndexRange<AssignmentIdx>,
+}
+
+impl CombComponentCore {
+    pub fn contains_assignment(
+        &self,
+        assign: AssignmentIdx,
+    ) -> Option<AssignmentDefinitionLocation> {
+        self.continuous_assignments
+            .contains(assign)
+            .then_some(AssignmentDefinitionLocation::ContinuousAssignment)
+    }
+}
+
+#[derive(Debug)]
+pub enum PrimaryComponentInfo {
+    Comb(CombComponentCore),
+    Standard(ComponentCore),
+}
+
+impl PrimaryComponentInfo {
+    pub fn contains_assignment(
+        &self,
+        ctx: &Context,
+        assign: AssignmentIdx,
+    ) -> Option<AssignmentDefinitionLocation> {
+        match self {
+            PrimaryComponentInfo::Comb(info) => {
+                info.contains_assignment(assign)
+            }
+            PrimaryComponentInfo::Standard(info) => {
+                info.contains_assignment(ctx, assign)
+            }
+        }
+    }
+
+    #[must_use]
+    pub fn as_standard(&self) -> Option<&ComponentCore> {
+        if let Self::Standard(v) = self {
+            Some(v)
+        } else {
+            None
+        }
+    }
+
+    pub fn unwrap_standard(&self) -> &ComponentCore {
+        match self {
+            PrimaryComponentInfo::Standard(v) => v,
+            _ => panic!("Expected a standard component"),
+        }
+    }
+
+    #[must_use]
+    pub fn as_comb(&self) -> Option<&CombComponentCore> {
+        if let Self::Comb(v) = self {
+            Some(v)
+        } else {
+            None
+        }
+    }
+
+    /// Returns `true` if the primary component info is [`Comb`].
+    ///
+    /// [`Comb`]: PrimaryComponentInfo::Comb
+    #[must_use]
+    pub fn is_comb(&self) -> bool {
+        matches!(self, Self::Comb(..))
+    }
+
+    pub fn continuous_assignments(&self) -> IndexRange<AssignmentIdx> {
+        match self {
+            PrimaryComponentInfo::Comb(info) => info.continuous_assignments,
+            PrimaryComponentInfo::Standard(info) => info.continuous_assignments,
+        }
+    }
+
+    pub fn control(&self) -> Option<ControlIdx> {
+        match self {
+            PrimaryComponentInfo::Comb(_) => None,
+            PrimaryComponentInfo::Standard(info) => info.control,
+        }
+    }
+}
+
+impl From<ComponentCore> for PrimaryComponentInfo {
+    fn from(v: ComponentCore) -> Self {
+        Self::Standard(v)
+    }
+}
+
+impl From<CombComponentCore> for PrimaryComponentInfo {
+    fn from(v: CombComponentCore) -> Self {
+        Self::Comb(v)
+    }
+}
+
 pub enum AssignmentDefinitionLocation {
     /// The assignment is contained in a comb group
     CombGroup(CombGroupIdx),
@@ -389,4 +487,4 @@ impl IdxSkipSizes {
     }
 }
 
-pub type ComponentMap = IndexedMap<ComponentIdx, ComponentCore>;
+pub type ComponentMap = IndexedMap<ComponentIdx, PrimaryComponentInfo>;

interp/src/flatten/flat_ir/control/translator.rs

@@ -9,7 +9,10 @@ use crate::{
     flatten::{
         flat_ir::{
             cell_prototype::{CellPrototype, ConstantType},
-            component::{AuxiliaryComponentInfo, ComponentCore},
+            component::{
+                AuxiliaryComponentInfo, CombComponentCore, ComponentCore,
+                PrimaryComponentInfo,
+            },
             flatten_trait::{flatten_tree, FlattenTree, SingleHandle},
             prelude::{
                 Assignment, AssignmentIdx, CellRef, CombGroup, CombGroupIdx,
@@ -289,27 +292,38 @@ fn translate_component(
         .find_all_with_attr(NumAttr::Done)
         .collect_vec();
 
-    // Will need to rethink this at some point
-    if go_ports.len() > 1 || done_ports.len() > 1 {
-        todo!(
-            "handle multiple go and done ports. On component: {}",
+    let comp_info: PrimaryComponentInfo = if comp.is_comb {
+        assert!(
+            go_ports.is_empty() && done_ports.is_empty(),
+            "malformed comb component: {}",
             comp.name
         );
-    } else if comp.is_comb {
-        todo!("handle comb components. On component: {}", comp.name);
-    }
-    let go_port = &go_ports[0];
-    let done_port = &done_ports[0];
-
-    let comp_core = ComponentCore {
-        control,
-        continuous_assignments,
-        is_comb: comp.is_comb,
-        go: *layout.port_map[&go_port.as_raw()].unwrap_local(),
-        done: *layout.port_map[&done_port.as_raw()].unwrap_local(),
+
+        PrimaryComponentInfo::Comb(CombComponentCore {
+            continuous_assignments,
+        })
+    } else {
+        let go_port = &go_ports[0];
+        let done_port = &done_ports[0];
+
+        // Will need to rethink this at some point
+        if go_ports.len() > 1 || done_ports.len() > 1 {
+            todo!(
+                "handle multiple go and done ports. On component: {}",
+                comp.name
+            );
+        }
+
+        let comp_core = ComponentCore {
+            control,
+            continuous_assignments,
+            go: *layout.port_map[&go_port.as_raw()].unwrap_local(),
+            done: *layout.port_map[&done_port.as_raw()].unwrap_local(),
+        };
+        comp_core.into()
     };
 
-    let ctrl_ref = ctx.primary.components.push(comp_core);
+    let ctrl_ref = ctx.primary.components.push(comp_info);
     ctx.secondary
         .comp_aux_info
         .insert(ctrl_ref, auxiliary_component_info);

interp/src/flatten/primitives/stateful/memories.rs

@@ -719,6 +719,37 @@ impl RaceDetectionPrimitive for CombMem {
     }
 }
 
+#[derive(Copy, Clone, Debug)]
+enum MemOut {
+    /// Points to a valid address in the memory
+    Valid(usize),
+    /// Output is zero, but not a memory address
+    Zero,
+    /// Output is undefined
+    Undef,
+}
+
+impl MemOut {
+    fn is_def(&self) -> bool {
+        match self {
+            MemOut::Valid(_) | MemOut::Zero => true,
+            MemOut::Undef => false,
+        }
+    }
+
+    fn get_value(&self, data: &[ValueWithClock]) -> PortValue {
+        match self {
+            MemOut::Valid(addr) => {
+                PortValue::new_cell(data[*addr].value.clone())
+            }
+            MemOut::Zero => {
+                PortValue::new_cell(BitVecValue::zero(data[0].value.width()))
+            }
+            MemOut::Undef => PortValue::new_undef(),
+        }
+    }
+}
+
 #[derive(Clone)]
 pub struct SeqMem {
     base_port: GlobalPortIdx,
@@ -727,10 +758,10 @@ pub struct SeqMem {
     // TODO griffin: This bool is unused in the actual struct and should either
     // be removed or
     _allow_invalid_access: bool,
-    width: u32,
     addresser: MemDx<true>,
     done_is_high: bool,
-    read_out: PortValue,
+    // memory index which is currently latched
+    read_out: MemOut,
 }
 
 impl SeqMem {
@@ -755,10 +786,9 @@ impl SeqMem {
             base_port: base,
             internal_state,
             _allow_invalid_access: allow_invalid,
-            width,
             addresser: MemDx::new(shape),
             done_is_high: false,
-            read_out: PortValue::new_undef(),
+            read_out: MemOut::Undef,
             global_idx,
         }
     }
@@ -804,10 +834,9 @@ impl SeqMem {
             base_port,
             internal_state,
             _allow_invalid_access: allow_invalid,
-            width,
             addresser: MemDx::new(size),
             done_is_high: false,
-            read_out: PortValue::new_undef(),
+            read_out: MemOut::Undef,
             global_idx,
         }
     }
@@ -884,7 +913,10 @@ impl Primitive for SeqMem {
         {
             port_map.insert_val_general(
                 self.read_data(),
-                self.read_out.as_option().unwrap().clone(),
+                self.read_out
+                    .get_value(&self.internal_state)
+                    .into_option()
+                    .unwrap(),
             )?
         } else {
             UpdateStatus::Unchanged
@@ -908,10 +940,10 @@ impl Primitive for SeqMem {
 
         if reset {
             self.done_is_high = false;
-            self.read_out = PortValue::new_cell(BitVecValue::zero(self.width));
+            self.read_out = MemOut::Zero;
         } else if content_en && write_en {
             self.done_is_high = true;
-            self.read_out = PortValue::new_undef();
+            self.read_out = MemOut::Undef;
             let addr_actual =
                 addr.ok_or(RuntimeError::UndefinedWriteAddr(self.global_idx))?;
             let write_data = port_map[self.write_data()]
@@ -922,9 +954,7 @@ impl Primitive for SeqMem {
             self.done_is_high = true;
             let addr_actual =
                 addr.ok_or(RuntimeError::UndefinedReadAddr(self.global_idx))?;
-            self.read_out = PortValue::new_cell(
-                self.internal_state[addr_actual].value.clone(),
-            );
+            self.read_out = MemOut::Valid(addr_actual);
         } else {
             self.done_is_high = false;
         }
@@ -937,7 +967,10 @@ impl Primitive for SeqMem {
                 BitVecValue::fals()
             }),
         )?;
-        port_map.write_exact_unchecked(self.read_data(), self.read_out.clone());
+        port_map.write_exact_unchecked(
+            self.read_data(),
+            self.read_out.get_value(&self.internal_state),
+        );
         Ok(())
     }
 

interp/src/flatten/structures/context.rs

@@ -5,8 +5,8 @@ use calyx_ir::Direction;
 use crate::flatten::flat_ir::{
     cell_prototype::CellPrototype,
     component::{
-        AssignmentDefinitionLocation, AuxiliaryComponentInfo, ComponentCore,
-        ComponentMap,
+        AssignmentDefinitionLocation, AuxiliaryComponentInfo, ComponentMap,
+        PrimaryComponentInfo,
     },
     identifier::IdMap,
     prelude::{
@@ -53,7 +53,7 @@ pub struct InterpretationContext {
 }
 
 impl Index<ComponentIdx> for InterpretationContext {
-    type Output = ComponentCore;
+    type Output = PrimaryComponentInfo;
 
     fn index(&self, index: ComponentIdx) -> &Self::Output {
         &self.components[index]