[Cider 2] Fix extra cycle (#1973)

* checkpoint

* clean up

* remove the unnecessary bundle struct since a vec will do just fine

* add note about how the current termination trick is a little dubious

interp/src/flatten/structures/environment/assignments.rs

@@ -1,73 +1,31 @@
-use crate::flatten::{
-    flat_ir::prelude::{Assignment, AssignmentIdx, GlobalCellIdx},
-    structures::context::Context,
-};
+use crate::flatten::flat_ir::prelude::{GlobalCellIdx, LocalPortOffset};
 
 use super::env::AssignmentRange;
 
-/// A collection of assignments represented using a series of half-open ranges
-/// via [AssignmentRange]
 #[derive(Debug)]
-pub(crate) struct AssignmentBundle {
-    assigns: Vec<(GlobalCellIdx, AssignmentRange)>,
+pub struct GroupInterfacePorts {
+    pub go: LocalPortOffset,
+    pub done: LocalPortOffset,
 }
 
-// TODO griffin: remove the dead stuff later
-#[allow(dead_code)]
-impl AssignmentBundle {
-    pub fn new() -> Self {
-        Self {
-            assigns: Vec::new(),
-        }
-    }
-
-    pub fn with_capacity(size: usize) -> Self {
-        Self {
-            assigns: Vec::with_capacity(size),
-        }
-    }
-
-    #[inline]
-    pub fn push(&mut self, value: (GlobalCellIdx, AssignmentRange)) {
-        self.assigns.push(value)
-    }
-
-    pub fn iter(
-        &self,
-    ) -> impl Iterator<Item = &(GlobalCellIdx, AssignmentRange)> {
-        self.assigns.iter()
-    }
-
-    pub fn iter_over_indices(
-        &self,
-    ) -> impl Iterator<Item = (GlobalCellIdx, AssignmentIdx)> + '_ {
-        self.assigns
-            .iter()
-            .flat_map(|(c, x)| x.iter().map(|y| (*c, y)))
-    }
-
-    pub fn iter_over_assignments<'a>(
-        &'a self,
-        ctx: &'a Context,
-    ) -> impl Iterator<Item = (GlobalCellIdx, &'a Assignment)> {
-        self.iter_over_indices()
-            .map(|(c, idx)| (c, &ctx.primary[idx]))
-    }
-
-    /// The total number of assignments. Not the total number of index ranges!
-    pub fn len(&self) -> usize {
-        self.assigns
-            .iter()
-            .fold(0, |acc, (_, range)| acc + range.size())
-    }
+/// A group of assignments that is scheduled to be evaluated
+#[derive(Debug)]
+pub struct ScheduledAssignments {
+    pub active_cell: GlobalCellIdx,
+    pub assignments: AssignmentRange,
+    pub interface_ports: Option<GroupInterfacePorts>,
 }
 
-impl FromIterator<(GlobalCellIdx, AssignmentRange)> for AssignmentBundle {
-    fn from_iter<T: IntoIterator<Item = (GlobalCellIdx, AssignmentRange)>>(
-        iter: T,
+impl ScheduledAssignments {
+    pub fn new(
+        active_cell: GlobalCellIdx,
+        assignments: AssignmentRange,
+        interface_ports: Option<GroupInterfacePorts>,
     ) -> Self {
         Self {
-            assigns: iter.into_iter().collect(),
+            active_cell,
+            assignments,
+            interface_ports,
         }
     }
 }

interp/src/flatten/structures/environment/env.rs

@@ -1,6 +1,9 @@
 use itertools::Itertools;
 
-use super::{assignments::AssignmentBundle, program_counter::ProgramCounter};
+use super::{
+    assignments::{GroupInterfacePorts, ScheduledAssignments},
+    program_counter::ProgramCounter,
+};
 
 use super::super::{
     context::Context, index_trait::IndexRange, indexed_map::IndexedMap,
@@ -511,13 +514,22 @@ impl<'a> Simulator<'a> {
     fn get_assignments(
         &self,
         control_points: &[ControlPoint],
-    ) -> AssignmentBundle {
+    ) -> Vec<ScheduledAssignments> {
         control_points
             .iter()
             .map(|node| {
                 match &self.ctx().primary[node.control_node_idx] {
                     ControlNode::Enable(e) => {
-                        (node.comp, self.ctx().primary[e.group()].assignments)
+                        let group = &self.ctx().primary[e.group()];
+
+                        ScheduledAssignments::new(
+                            node.comp,
+                            group.assignments,
+                            Some(GroupInterfacePorts {
+                                go: group.go,
+                                done: group.done,
+                            }),
+                        )
                     }
 
                     ControlNode::Invoke(_) => {
@@ -551,7 +563,6 @@ impl<'a> Simulator<'a> {
         // buffers. Can pick anything from zero to the number of nodes in the
         // program counter as the size
         let mut leaf_nodes = vec![];
-        let mut done_groups = vec![];
 
         self.env.pc.vec_mut().retain_mut(|node| {
             // just considering a single node case for the moment
@@ -638,42 +649,19 @@ impl<'a> Simulator<'a> {
                         leaf_nodes.push(node.clone());
                         true
                     } else {
-                        done_groups.push((
-                            node.clone(),
-                            self.env.ports[done_idx].clone(),
-                        ));
-                        // remove from the list now
-                        false
+                        // This group has finished running and may be removed
+                        // this is somewhat dubious at the moment since it
+                        // relies on the fact that the group done port will
+                        // still be high since convergence hasn't propagated the
+                        // low done signal yet.
+                        node.mutate_into_next(self.env.ctx)
                     }
                 }
                 ControlNode::Invoke(_) => todo!("invokes not implemented yet"),
             }
         });
 
         self.undef_all_ports();
-        for (node, val) in &done_groups {
-            match &self.env.ctx.primary[node.control_node_idx] {
-                ControlNode::Enable(e) => {
-                    let go_local = self.env.ctx.primary[e.group()].go;
-                    let done_local = self.env.ctx.primary[e.group()].done;
-                    let index_bases = &self.env.cells[node.comp]
-                        .as_comp()
-                        .unwrap()
-                        .index_bases;
-                    let done_idx = index_bases + done_local;
-                    let go_idx = index_bases + go_local;
-
-                    // retain done condition from before
-                    self.env.ports[done_idx] = val.clone();
-                    self.env.ports[go_idx] =
-                        PortValue::new_implicit(Value::bit_high());
-                }
-                ControlNode::Invoke(_) => todo!(),
-                _ => {
-                    unreachable!("non-leaf node included in list of done nodes. This should never happen, please report it.")
-                }
-            }
-        }
 
         for node in &leaf_nodes {
             match &self.env.ctx.primary[node.control_node_idx] {
@@ -707,13 +695,6 @@ impl<'a> Simulator<'a> {
             }
         }
 
-        // need to cleanup the finished groups
-        for (node, _) in done_groups {
-            if let Some(next) = ControlPoint::get_next(&node, self.env.ctx) {
-                self.env.pc.insert_node(next)
-            }
-        }
-
         Ok(())
     }
 
@@ -729,9 +710,6 @@ impl<'a> Simulator<'a> {
     /// Evaluate the entire program
     pub fn run_program(&mut self) -> InterpreterResult<()> {
         while !self.is_done() {
-            dbg!("calling step");
-            // self.env.print_pc();
-            self.print_env();
             self.step()?
         }
         Ok(())
@@ -796,26 +774,78 @@ impl<'a> Simulator<'a> {
         let assigns_bundle = self.get_assignments(control_points);
         let mut has_changed = true;
 
+        // TODO griffin: rewrite this so that someone can actually read it
+        let done_ports: Vec<_> = assigns_bundle
+            .iter()
+            .filter_map(|x| {
+                x.interface_ports.as_ref().map(|y| {
+                    &self.env.cells[x.active_cell]
+                        .as_comp()
+                        .unwrap()
+                        .index_bases
+                        + y.done
+                })
+            })
+            .collect();
+
         while has_changed {
             has_changed = false;
 
             // evaluate all the assignments and make updates
-            for (cell, assigns) in assigns_bundle.iter() {
-                for assign_idx in assigns {
+            for ScheduledAssignments {
+                active_cell,
+                assignments,
+                interface_ports,
+            } in assigns_bundle.iter()
+            {
+                let ledger = self.env.cells[*active_cell].as_comp().unwrap();
+                let go = interface_ports
+                    .as_ref()
+                    .map(|x| &ledger.index_bases + x.go);
+                let done = interface_ports
+                    .as_ref()
+                    .map(|x| &ledger.index_bases + x.done);
+
+                for assign_idx in assignments {
                     let assign = &self.env.ctx.primary[assign_idx];
 
                     // TODO griffin: Come back to this unwrap default later
                     // since we may want to do something different if the guard
                     // does not have a defined value
-                    if self.evaluate_guard(assign.guard, *cell).unwrap_or(false)
+                    if self
+                        .evaluate_guard(assign.guard, *active_cell)
+                        .unwrap_or_default()
+                    // the go for the group is high
+                    && go
+                        .as_ref()
+                        .map(|g| self.env.ports[*g].as_bool().unwrap_or_default())
+                        // if there is no go signal, then we want to run the
+                        // assignment
+                        .unwrap_or(true)
                     {
-                        let val = self.get_value(&assign.src, *cell);
-                        let dest = self.get_global_idx(&assign.dst, *cell);
+                        let val = self.get_value(&assign.src, *active_cell);
+                        let dest =
+                            self.get_global_idx(&assign.dst, *active_cell);
+
+                        if let Some(done) = done {
+                            if dest != done {
+                                let done_val = &self.env.ports[done];
+
+                                if done_val.as_bool().unwrap_or(true) {
+                                    // skip this assignment when we are done or
+                                    // or the done signal is undefined
+                                    continue;
+                                }
+                            }
+                        }
+
                         if let Some(v) = val.as_option() {
-                            self.env.ports.insert_val(
+                            let changed = self.env.ports.insert_val(
                                 dest,
                                 AssignedValue::new(v.val().clone(), assign_idx),
                             )?;
+
+                            has_changed |= changed.as_bool();
                         } else if self.env.ports[dest].is_def() {
                             todo!("Raise an error here since this assignment is undefining things")
                         }
@@ -841,6 +871,19 @@ impl<'a> Simulator<'a> {
                 .as_bool();
 
             has_changed |= changed;
+
+            // check for undefined done ports. If any remain after we've
+            // converged then they should be set to zero and we should continue
+            // convergence
+            if !has_changed {
+                for &done_port in &done_ports {
+                    if self.env.ports[done_port].is_undef() {
+                        self.env.ports[done_port] =
+                            PortValue::new_implicit(Value::bit_low());
+                        has_changed = true;
+                    }
+                }
+            }
         }
 
         Ok(())
@@ -849,7 +892,6 @@ impl<'a> Simulator<'a> {
     pub fn _main_test(&mut self) {
         self.env.print_pc();
         let _ = self.run_program();
-        self.env.print_pc();
         self.print_env();
     }
 }

interp/src/flatten/structures/environment/program_counter.rs

@@ -368,10 +368,6 @@ impl ProgramCounter {
     pub(crate) fn vec_mut(&mut self) -> &mut Vec<ControlPoint> {
         &mut self.vec
     }
-
-    pub(crate) fn insert_node(&mut self, node: ControlPoint) {
-        self.vec.push(node)
-    }
 }
 
 impl<'a> IntoIterator for &'a ProgramCounter {