[Cider] Clock propagation (#2368)

This somewhat chunky PR includes the logic needed to propagate clocks
through combinational logic and amends the read checks to defer until
the value is used in a non-combinational context. The continuous
assignments now run under a single thread which is never synchronized,
so attempts to write a register in continuous logic will always result
in a race when used outside continuous logic.

There's some additional refactoring and minor changes to the primitive
interface. Additionally, a single step can now advance past multiple
control nodes, which should reduce some of the repetition when using the
debugger

interp/Cargo.toml

@@ -16,7 +16,6 @@ path = "src/main.rs"
 [dependencies]
 smallvec = { workspace = true, features = ["union", "const_generics"] }
 serde = { workspace = true, features = ["derive", "rc"] }
-lazy_static.workspace = true
 itertools.workspace = true
 pest.workspace = true
 pest_derive.workspace = true

interp/src/as_raw.rs

@@ -17,12 +17,12 @@ impl<T> AsRaw<T> for *const T {
     }
 }
 
-impl<'a, T> AsRaw<T> for &Ref<'a, T> {
+impl<T> AsRaw<T> for &Ref<'_, T> {
     fn as_raw(&self) -> *const T {
         self as &T as *const T
     }
 }
-impl<'a, T> AsRaw<T> for Ref<'a, T> {
+impl<T> AsRaw<T> for Ref<'_, T> {
     fn as_raw(&self) -> *const T {
         self as &T as *const T
     }

interp/src/debugger/commands/core.rs

@@ -1,7 +1,6 @@
 //! This module contains the core data structures and commands used by the debugger
 
 use itertools::{self, Itertools};
-use lazy_static::lazy_static;
 use owo_colors::OwoColorize;
 use std::{
     fmt::{Display, Write},
@@ -403,7 +402,7 @@ impl Command {
             invocation: names,
             description: message,
             ..
-        } in COMMAND_INFO.iter()
+        } in get_command_info().iter()
         {
             writeln!(out, "    {: <30}{}", names.join(", "), message.green())
                 .unwrap();
@@ -419,7 +418,9 @@ impl Command {
             invocation,
             description,
             usage_example,
-        } in COMMAND_INFO.iter().filter(|x| !x.usage_example.is_empty())
+        } in get_command_info()
+            .iter()
+            .filter(|x| !x.usage_example.is_empty())
         {
             writeln!(out).unwrap();
             writeln!(out, "{}", invocation.join(", ")).unwrap();
@@ -438,11 +439,16 @@ impl Command {
 
 // I wouldn't recommend looking at this
 
-lazy_static! {
-    /// A (lazy) static list of [CommandInfo] objects used for the help and
-    /// explain messages
-    static ref COMMAND_INFO: Vec<CommandInfo> = {
-        vec![
+use std::sync::OnceLock;
+/// A (lazy) static list of [CommandInfo] objects used for the help and
+/// explain messages. Access via [get_command_info]
+static COMMAND_INFO: OnceLock<Box<[CommandInfo]>> = OnceLock::new();
+
+/// Returns the list of [CommandInfo] objects used for the help and explain
+/// messages
+fn get_command_info() -> &'static [CommandInfo] {
+    COMMAND_INFO.get_or_init(|| {
+         [
             // step
             CIBuilder::new().invocation("step")
                 .invocation("s")
@@ -522,7 +528,6 @@ lazy_static! {
                 .description("Disable target watchpoint")
                 .usage("> disable-watch 4")
                 .usage("> disable-watch do_mult").build(),
-
             // explain
             CIBuilder::new().invocation("explain")
                 .description("Show examples of commands which take arguments").build(),
@@ -532,15 +537,15 @@ lazy_static! {
             CIBuilder::new().invocation("exit")
                 .invocation("quit")
                 .description("Exit the debugger").build(),
-        ]
-    };
+        ].into()
+    })
 }
 
 #[derive(Clone, Debug)]
 struct CommandInfo {
-    invocation: Vec<CommandName>,
+    invocation: Box<[CommandName]>,
     description: Description,
-    usage_example: Vec<UsageExample>,
+    usage_example: Box<[UsageExample]>,
 }
 
 // type shenanigans
@@ -627,9 +632,9 @@ where
 impl CommandInfoBuilder<Present, Present> {
     fn build(self) -> CommandInfo {
         CommandInfo {
-            invocation: self.invocation,
+            invocation: self.invocation.into(),
             description: self.description.unwrap(),
-            usage_example: self.usage_example,
+            usage_example: self.usage_example.into(),
         }
     }
 }

interp/src/debugger/debugging_context/context.rs

@@ -648,10 +648,8 @@ impl DebuggingContext {
                 let watchpoint_indicies =
                     self.watchpoints.get_by_group(x).unwrap();
                 match watchpoint_indicies {
-                    WatchPointIndices::Before(x) => return x.iter(),
-                    WatchPointIndices::Both { before, .. } => {
-                        return before.iter()
-                    }
+                    WatchPointIndices::Before(x) => x.iter(),
+                    WatchPointIndices::Both { before, .. } => before.iter(),
                     // this is stupid but works
                     _ => [].iter(),
                 }
@@ -665,10 +663,8 @@ impl DebuggingContext {
                 let watchpoint_indicies =
                     self.watchpoints.get_by_group(x).unwrap();
                 match watchpoint_indicies {
-                    WatchPointIndices::After(x) => return x.iter(),
-                    WatchPointIndices::Both { after, .. } => {
-                        return after.iter()
-                    }
+                    WatchPointIndices::After(x) => x.iter(),
+                    WatchPointIndices::Both { after, .. } => after.iter(),
                     // this is stupid but works
                     _ => [].iter(),
                 }

interp/src/errors.rs

@@ -225,7 +225,7 @@ pub enum RuntimeError {
     OverflowError,
 
     #[error(transparent)]
-    ConflictingAssignments(ConflictingAssignments),
+    ConflictingAssignments(Box<ConflictingAssignments>),
 }
 
 // this is silly but needed to make the program print something sensible when returning
@@ -299,7 +299,8 @@ impl RuntimeError {
         }
 
         match self {
-            RuntimeError::ConflictingAssignments(ConflictingAssignments { target, a1, a2 }) => {
+            RuntimeError::ConflictingAssignments(boxed_err) => {
+                let ConflictingAssignments { target, a1, a2 } = *boxed_err;
                 let (a1_str, a1_source) = assign_to_string(&a1, env);
                 let (a2_str, a2_source) = assign_to_string(&a2, env);
 
@@ -323,8 +324,20 @@ impl RuntimeError {
             RuntimeError::UndefinedReadAddr(c) => CiderError::GenericError(format!("Attempted to read from an undefined memory address from memory named \"{}\"", env.get_full_name(c))),
             RuntimeError::ClockError(clk) => {
                 match clk {
-                    ClockError::ReadWrite(c) => CiderError::GenericError(format!("Concurrent read & write to the same register/memory {}", env.get_full_name(c).underline())),
-                    ClockError::WriteWrite(c) => CiderError::GenericError(format!("Concurrent writes to the same register/memory {}", env.get_full_name(c).underline())),
+                    ClockError::ReadWrite(c, num) => {
+                        if let Some(entry_number) = num {
+                            CiderError::GenericError(format!("Concurrent read & write to the same memory {} in slot {}", env.get_full_name(c).underline(), entry_number))
+                        } else {
+                            CiderError::GenericError(format!("Concurrent read & write to the same register {}", env.get_full_name(c).underline()))
+                        }
+                },
+                    ClockError::WriteWrite(c, num) => {
+                        if let Some(entry_number) = num {
+                            CiderError::GenericError(format!("Concurrent writes to the same memory {} in slot {}", env.get_full_name(c).underline(), entry_number))
+                        } else {
+                            CiderError::GenericError(format!("Concurrent writes to the same register {}", env.get_full_name(c).underline()))
+                        }
+                    },
                     c => CiderError::GenericError(format!("Unexpected clock error: {c:?}")),
                 }
             }

interp/src/flatten/flat_ir/base.rs

@@ -13,6 +13,7 @@ use crate::{
     serialization::PrintCode,
 };
 use baa::{BitVecOps, BitVecValue};
+use std::collections::HashSet;
 
 // making these all u32 for now, can give the macro an optional type as the
 // second arg to contract or expand as needed
@@ -428,12 +429,24 @@ impl From<(GlobalCellIdx, AssignmentIdx)> for AssignmentWinner {
 
 /// A struct representing a value that has been assigned to a port. It wraps a
 /// concrete value and the "winner" which assigned it.
-#[derive(Clone, PartialEq)]
+#[derive(Clone)]
 pub struct AssignedValue {
     val: BitVecValue,
     winner: AssignmentWinner,
     thread: Option<ThreadIdx>,
     clocks: Option<ClockPair>,
+    propagate_clocks: bool,
+    transitive_clocks: Option<HashSet<ClockPair>>,
+}
+
+impl AssignedValue {
+    pub fn eq_no_transitive_clocks(&self, other: &Self) -> bool {
+        self.val == other.val
+            && self.winner == other.winner
+            && self.thread == other.thread
+            && self.clocks == other.clocks
+            && self.propagate_clocks == other.propagate_clocks
+    }
 }
 
 impl std::fmt::Debug for AssignedValue {
@@ -442,6 +455,8 @@ impl std::fmt::Debug for AssignedValue {
             .field("val", &self.val.to_bit_str())
             .field("winner", &self.winner)
             .field("thread", &self.thread)
+            .field("clocks", &self.clocks)
+            .field("propagate_clocks", &self.propagate_clocks)
             .finish()
     }
 }
@@ -461,9 +476,37 @@ impl AssignedValue {
             winner: winner.into(),
             thread: None,
             clocks: None,
+            propagate_clocks: false,
+            transitive_clocks: None,
         }
     }
 
+    /// Adds a clock to the set of transitive reads associated with this value
+    pub fn add_transitive_clock(&mut self, clock_pair: ClockPair) {
+        self.transitive_clocks
+            .get_or_insert_with(Default::default)
+            .insert(clock_pair);
+    }
+
+    pub fn add_transitive_clocks<I: IntoIterator<Item = ClockPair>>(
+        &mut self,
+        clocks: I,
+    ) {
+        self.transitive_clocks
+            .get_or_insert_with(Default::default)
+            .extend(clocks);
+    }
+
+    pub fn iter_transitive_clocks(
+        &self,
+    ) -> impl Iterator<Item = ClockPair> + '_ {
+        self.transitive_clocks
+            .as_ref()
+            .map(|set| set.iter().copied())
+            .into_iter()
+            .flatten()
+    }
+
     pub fn with_thread(mut self, thread: ThreadIdx) -> Self {
         self.thread = Some(thread);
         self
@@ -479,6 +522,31 @@ impl AssignedValue {
         self
     }
 
+    pub fn with_clocks_optional(
+        mut self,
+        clock_pair: Option<ClockPair>,
+    ) -> Self {
+        self.clocks = clock_pair;
+        self
+    }
+
+    pub fn with_transitive_clocks_opt(
+        mut self,
+        clocks: Option<HashSet<ClockPair>>,
+    ) -> Self {
+        self.transitive_clocks = clocks;
+        self
+    }
+
+    pub fn with_propagate_clocks(mut self) -> Self {
+        self.propagate_clocks = true;
+        self
+    }
+
+    pub fn set_propagate_clocks(&mut self, propagate_clocks: bool) {
+        self.propagate_clocks = propagate_clocks;
+    }
+
     /// Returns true if the two AssignedValues do not have the same winner
     pub fn has_conflict_with(&self, other: &Self) -> bool {
         self.winner != other.winner
@@ -497,36 +565,21 @@ impl AssignedValue {
     /// A utility constructor which returns a new implicitly assigned value with
     /// a one bit high value
     pub fn implicit_bit_high() -> Self {
-        Self {
-            val: BitVecValue::tru(),
-            winner: AssignmentWinner::Implicit,
-            thread: None,
-            clocks: None,
-        }
+        Self::new(BitVecValue::tru(), AssignmentWinner::Implicit)
     }
 
     /// A utility constructor which returns an [`AssignedValue`] with the given
     /// value and a [`AssignmentWinner::Cell`] as the winner
     #[inline]
     pub fn cell_value(val: BitVecValue) -> Self {
-        Self {
-            val,
-            winner: AssignmentWinner::Cell,
-            thread: None,
-            clocks: None,
-        }
+        Self::new(val, AssignmentWinner::Cell)
     }
 
     /// A utility constructor which returns an [`AssignedValue`] with the given
     /// value and a [`AssignmentWinner::Implicit`] as the winner
     #[inline]
     pub fn implicit_value(val: BitVecValue) -> Self {
-        Self {
-            val,
-            winner: AssignmentWinner::Implicit,
-            thread: None,
-            clocks: None,
-        }
+        Self::new(val, AssignmentWinner::Implicit)
     }
 
     /// A utility constructor which returns an [`AssignedValue`] with a one bit
@@ -549,6 +602,14 @@ impl AssignedValue {
     pub fn clocks(&self) -> Option<&ClockPair> {
         self.clocks.as_ref()
     }
+
+    pub fn transitive_clocks(&self) -> Option<&HashSet<ClockPair>> {
+        self.transitive_clocks.as_ref()
+    }
+
+    pub fn propagate_clocks(&self) -> bool {
+        self.propagate_clocks
+    }
 }
 
 #[derive(Debug, Clone, Default)]
@@ -595,6 +656,14 @@ impl PortValue {
         self
     }
 
+    pub fn transitive_clocks(&self) -> Option<&HashSet<ClockPair>> {
+        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/control/translator.rs

@@ -290,8 +290,13 @@ fn translate_component(
         .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");
+    if go_ports.len() > 1 || done_ports.len() > 1 {
+        todo!(
+            "handle multiple go and done ports. On 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];

interp/src/flatten/flat_ir/flatten_trait.rs

@@ -74,7 +74,7 @@ where
     handle: &'a mut VecHandle<'outer, In, Idx, Out>,
 }
 
-impl<'a, 'outer, In, Idx, Out> SingleHandle<'a, 'outer, In, Idx, Out>
+impl<'outer, In, Idx, Out> SingleHandle<'_, 'outer, In, Idx, Out>
 where
     Idx: IndexRef,
 {