Breaking: fix frame dependency calculation (#76)

* Fix: frame dependency calculation

* Breaking: Program.get_frames_for_instruction returns HashSet

src/program/frame.rs

@@ -129,7 +129,7 @@ pub(crate) enum FrameMatchCondition<'a> {
  /// Match all frames which contain exactly these qubits
  ExactQubits(&'a [Qubit]),
 
- /// Return these specific frames, if present in the set
+ /// Return this specific frame, if present in the set
  Specific(&'a FrameIdentifier),
 
  /// Return all frames which match all of these conditions

src/program/graph.rs

@@ -200,6 +200,81 @@ pub struct InstructionBlock {
  pub terminator: BlockTerminator,
 }
 
+/// PreviousNodes is a structure which helps maintain ordering among instructions which operate on a given frame.
+/// It works similarly to a multiple-reader-single-writer queue, where an instruction which "uses" a frame is like
+/// a writer and an instruction which blocks that frame is like a reader. Multiple instructions may concurrently
+/// block a frame, but an instruction may not use a frame while it is concurrently used or blocked.
+///
+/// ## Examples
+///
+/// Note that "depends on" is equivalent to "must execute after completion of".
+///
+/// ```text
+/// user --> user # a second user takes a dependency on the first
+///
+/// user --> blocker # multiple blockers take a dependency on the most recent user
+/// \-> blocker
+/// \-> blocker
+///
+/// blocker --> user --> blocker # users and blockers take dependencies on one another,
+/// # but blockers do not depend on other blocking instructions
+/// ```
+struct PreviousNodes {
+ using: Option<ScheduledGraphNode>,
+ blocking: HashSet<ScheduledGraphNode>,
+}
+
+impl Default for PreviousNodes {
+ /// The default value for [PreviousNodes] is useful in that, if no previous nodes have been recorded
+ /// as using a frame, we should consider that the start of the instruction block "blocks" use of that frame
+ /// (in other words, this instruction cannot be scheduled prior to the start of the instruction block).
+ fn default() -> Self {
+ Self {
+ using: None,
+ blocking: vec![ScheduledGraphNode::BlockStart].into_iter().collect(),
+ }
+ }
+}
+
+impl PreviousNodes {
+ /// Register a node as using a frame, and return the instructions on which it should depend/wait for scheduling (if any).
+ ///
+ /// A node which uses a frame will block on any previous user or blocker of the frame, much like a writer in a read-write lock.
+ fn get_dependencies_for_next_user(
+ &mut self,
+ node: ScheduledGraphNode,
+ ) -> HashSet<ScheduledGraphNode> {
+ let mut result = std::mem::take(&mut self.blocking);
+ if let Some(previous_user) = self.using.replace(node) {
+ result.insert(previous_user);
+ }
+
+ result
+ }
+
+ /// Register a node as blocking a frame, and return the instructions on which it should depend/wait for scheduling (if any).
+ ///
+ /// A node which blocks a frame will block on any previous user of the frame, but not concurrent blockers.
+ ///
+ /// If the frame is currently blocked by other nodes, it will add itself to the list of blockers,
+ /// much like a reader in a read-write lock.
+ fn get_dependency_for_next_blocker(
+ &mut self,
+ node: ScheduledGraphNode,
+ ) -> Option<ScheduledGraphNode> {
+ self.blocking.insert(node);
+ self.using
+ }
+
+ /// Consume the [PreviousNodes] and return all nodes within.
+ pub fn drain(mut self) -> HashSet<ScheduledGraphNode> {
+ if let Some(using) = self.using {
+ self.blocking.insert(using);
+ }
+ self.blocking
+ }
+}
+
 impl InstructionBlock {
  pub fn build(
  instructions: Vec<Instruction>,
@@ -213,8 +288,7 @@ impl InstructionBlock {
  let mut last_classical_instruction = ScheduledGraphNode::BlockStart;
 
  // Store the instruction index of the last instruction to block that frame
- let mut last_instruction_by_frame: HashMap<FrameIdentifier, ScheduledGraphNode> =
- HashMap::new();
+ let mut last_instruction_by_frame: HashMap<FrameIdentifier, PreviousNodes> = HashMap::new();
 
  // Store memory access reads and writes. Key is memory region name.
  // NOTE: this may be refined to serialize by memory region offset rather than by entire region.
@@ -236,21 +310,32 @@ impl InstructionBlock {
  let used_frames = program
  .get_frames_for_instruction(instruction, false)
  .unwrap_or_default();
+
  let blocked_frames = program
  .get_frames_for_instruction(instruction, true)
  .unwrap_or_default();
 
- // Take a dependency on any previous instructions to _block_ a frame which this instruction _uses_.
- for frame in used_frames {
- let previous_node_id = last_instruction_by_frame
- .get(frame)
- .unwrap_or(&ScheduledGraphNode::BlockStart);
- add_dependency!(graph, *previous_node_id => node, ExecutionDependency::ReferenceFrame);
+ let blocked_but_not_used_frames = blocked_frames.difference(&used_frames);
+
+ for frame in &used_frames {
+ let previous_node_ids = last_instruction_by_frame
+ .entry((*frame).clone())
+ .or_default()
+ .get_dependencies_for_next_user(node);
+
+ for previous_node_id in previous_node_ids {
+ add_dependency!(graph, previous_node_id => node, ExecutionDependency::ReferenceFrame);
+ }
  }
 
- // We mark all "blocked" frames as such for later instructions to take a dependency on
- for frame in blocked_frames {
- last_instruction_by_frame.insert(frame.clone(), node);
+ for frame in blocked_but_not_used_frames {
+ if let Some(previous_node_id) = last_instruction_by_frame
+ .entry((*frame).clone())
+ .or_default()
+ .get_dependency_for_next_blocker(node)
+ {
+ add_dependency!(graph, previous_node_id => node, ExecutionDependency::ReferenceFrame);
+ }
  }
 
  Ok(())
@@ -295,8 +380,10 @@ impl InstructionBlock {
  // does not terminate until these are complete
  add_dependency!(graph, last_classical_instruction => ScheduledGraphNode::BlockEnd, ExecutionDependency::StableOrdering);
 
- for (_, last_instruction) in last_instruction_by_frame {
- add_dependency!(graph, last_instruction => ScheduledGraphNode::BlockEnd, ExecutionDependency::ReferenceFrame);
+ for previous_nodes in last_instruction_by_frame.into_values() {
+ for node in previous_nodes.drain() {
+ add_dependency!(graph, node => ScheduledGraphNode::BlockEnd, ExecutionDependency::ReferenceFrame);
+ }
  }
 
  // Examine all "pending" memory operations for all regions

src/program/graphviz_dot.rs

@@ -300,6 +300,35 @@ NONBLOCKING PULSE 2 \"rf\" test(duration: 1e6)
 "
  );
 
+ build_dot_format_snapshot_test_case!(
+ blocking_pulses_wrap_nonblocking,
+ "
+PULSE 0 \"rf\" test(duration: 1e6)
+NONBLOCKING PULSE 0 \"ro_tx\" test(duration: 1e6)
+PULSE 0 \"rf\" test(duration: 1e6)
+FENCE 0
+FENCE 0
+"
+ );
+
+ build_dot_format_snapshot_test_case!(
+ blocking_pulses_after_nonblocking,
+ "
+NONBLOCKING PULSE 0 \"ro_tx\" test(duration: 1e6)
+PULSE 0 \"rf\" test(duration: 1e6)
+PULSE 0 \"ro_rx\" test(duration: 1e6)
+"
+ );
+
+ build_dot_format_snapshot_test_case!(
+ blocking_2q_pulse,
+ "
+PULSE 0 \"rf\" test(duration: 1e-6)
+PULSE 1 \"rf\" test(duration: 1e-6)
+PULSE 0 1 \"cz\" test(duration: 1e-6)
+"
+ );
+
  build_dot_format_snapshot_test_case!(
  fence_all_with_nonblocking_pulses,
  "

src/program/mod.rs

@@ -130,15 +130,10 @@ impl Program {
  &'a self,
  instruction: &'a Instruction,
  include_blocked: bool,
- ) -> Option<Vec<&'a FrameIdentifier>> {
+ ) -> Option<HashSet<&'a FrameIdentifier>> {
  instruction
  .get_frame_match_condition(include_blocked)
- .map(|condition| {
- self.frames
- .get_matching_keys(condition)
- .into_iter()
- .collect()
- })
+ .map(|condition| self.frames.get_matching_keys(condition))
  }
 
  /// Returns a HashSet consisting of every Qubit that is used in the program.

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__active_reset_single_frame.snap

@@ -26,7 +26,8 @@ frame"];
  node [style="filled"];
  "feedback_start" [shape=circle, label="start"];
  "feedback_start" -> "feedback_0" [label="frame"];
- "feedback_start" -> "feedback_end" [label="ordering"];
+ "feedback_start" -> "feedback_end" [label="frame
+ordering"];
  "feedback_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "feedback_0" -> "feedback_end" [label="frame"];
  "feedback_end" [shape=circle, label="end"];

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_2q_pulse.snap

@@ -0,0 +1,28 @@
+---
+source: src/program/graphviz_dot.rs
+expression: dot_format
+---
+digraph {
+ entry -> "block_0_start";
+ entry [label="Entry Point"];
+ subgraph cluster_0 {
+ label="block_0";
+ node [style="filled"];
+ "block_0_start" [shape=circle, label="start"];
+ "block_0_start" -> "block_0_0" [label="frame"];
+ "block_0_start" -> "block_0_1" [label="frame"];
+ "block_0_start" -> "block_0_2" [label="frame"];
+ "block_0_start" -> "block_0_end" [label="frame
+ordering"];
+ "block_0_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1e-6)"];
+ "block_0_0" -> "block_0_2" [label="frame"];
+ "block_0_0" -> "block_0_end" [label="frame"];
+ "block_0_1" [shape=rectangle, label="[1] PULSE 1 \"rf\" test(duration: 1e-6)"];
+ "block_0_1" -> "block_0_2" [label="frame"];
+ "block_0_1" -> "block_0_end" [label="frame"];
+ "block_0_2" [shape=rectangle, label="[2] PULSE 0 1 \"cz\" test(duration: 1e-6)"];
+ "block_0_2" -> "block_0_end" [label="frame"];
+ "block_0_end" [shape=circle, label="end"];
+ }
+}
+

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_after_nonblocking.snap

@@ -0,0 +1,29 @@
+---
+source: src/program/graphviz_dot.rs
+expression: dot_format
+---
+digraph {
+ entry -> "block_0_start";
+ entry [label="Entry Point"];
+ subgraph cluster_0 {
+ label="block_0";
+ node [style="filled"];
+ "block_0_start" [shape=circle, label="start"];
+ "block_0_start" -> "block_0_0" [label="frame"];
+ "block_0_start" -> "block_0_1" [label="frame"];
+ "block_0_start" -> "block_0_2" [label="frame"];
+ "block_0_start" -> "block_0_end" [label="frame
+ordering"];
+ "block_0_0" [shape=rectangle, label="[0] NONBLOCKING PULSE 0 \"ro_tx\" test(duration: 1000000.0)"];
+ "block_0_0" -> "block_0_1" [label="frame"];
+ "block_0_0" -> "block_0_2" [label="frame"];
+ "block_0_0" -> "block_0_end" [label="frame"];
+ "block_0_1" [shape=rectangle, label="[1] PULSE 0 \"rf\" test(duration: 1000000.0)"];
+ "block_0_1" -> "block_0_2" [label="frame"];
+ "block_0_1" -> "block_0_end" [label="frame"];
+ "block_0_2" [shape=rectangle, label="[2] PULSE 0 \"ro_rx\" test(duration: 1000000.0)"];
+ "block_0_2" -> "block_0_end" [label="frame"];
+ "block_0_end" [shape=circle, label="end"];
+ }
+}
+

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_wrap_nonblocking.snap

@@ -0,0 +1,32 @@
+---
+source: src/program/graphviz_dot.rs
+expression: dot_format
+---
+digraph {
+ entry -> "block_0_start";
+ entry [label="Entry Point"];
+ subgraph cluster_0 {
+ label="block_0";
+ node [style="filled"];
+ "block_0_start" [shape=circle, label="start"];
+ "block_0_start" -> "block_0_0" [label="frame"];
+ "block_0_start" -> "block_0_1" [label="frame"];
+ "block_0_start" -> "block_0_3" [label="frame"];
+ "block_0_start" -> "block_0_end" [label="ordering"];
+ "block_0_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
+ "block_0_0" -> "block_0_1" [label="frame"];
+ "block_0_0" -> "block_0_2" [label="frame"];
+ "block_0_0" -> "block_0_3" [label="frame"];
+ "block_0_1" [shape=rectangle, label="[1] NONBLOCKING PULSE 0 \"ro_tx\" test(duration: 1000000.0)"];
+ "block_0_1" -> "block_0_2" [label="frame"];
+ "block_0_1" -> "block_0_3" [label="frame"];
+ "block_0_2" [shape=rectangle, label="[2] PULSE 0 \"rf\" test(duration: 1000000.0)"];
+ "block_0_2" -> "block_0_3" [label="frame"];
+ "block_0_3" [shape=rectangle, label="[3] FENCE 0"];
+ "block_0_3" -> "block_0_4" [label="frame"];
+ "block_0_4" [shape=rectangle, label="[4] FENCE 0"];
+ "block_0_4" -> "block_0_end" [label="frame"];
+ "block_0_end" [shape=circle, label="end"];
+ }
+}
+

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__chained_pulses.snap

@@ -10,15 +10,20 @@ digraph {
  node [style="filled"];
  "block_0_start" [shape=circle, label="start"];
  "block_0_start" -> "block_0_0" [label="frame"];
- "block_0_start" -> "block_0_end" [label="ordering"];
+ "block_0_start" -> "block_0_end" [label="frame
+ordering"];
  "block_0_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_0" -> "block_0_1" [label="frame"];
+ "block_0_0" -> "block_0_end" [label="frame"];
  "block_0_1" [shape=rectangle, label="[1] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_1" -> "block_0_2" [label="frame"];
+ "block_0_1" -> "block_0_end" [label="frame"];
  "block_0_2" [shape=rectangle, label="[2] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_2" -> "block_0_3" [label="frame"];
+ "block_0_2" -> "block_0_end" [label="frame"];
  "block_0_3" [shape=rectangle, label="[3] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_3" -> "block_0_4" [label="frame"];
+ "block_0_3" -> "block_0_end" [label="frame"];
  "block_0_4" [shape=rectangle, label="[4] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_4" -> "block_0_end" [label="frame"];
  "block_0_end" [shape=circle, label="end"];

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__different_frames_blocking.snap

@@ -12,7 +12,8 @@ digraph {
  "block_0_start" -> "block_0_0" [label="frame"];
  "block_0_start" -> "block_0_1" [label="frame"];
  "block_0_start" -> "block_0_2" [label="frame"];
- "block_0_start" -> "block_0_end" [label="ordering"];
+ "block_0_start" -> "block_0_end" [label="frame
+ordering"];
  "block_0_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "block_0_0" -> "block_0_end" [label="frame"];
  "block_0_1" [shape=rectangle, label="[1] PULSE 1 \"rf\" test(duration: 1000000.0)"];

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__jump.snap

@@ -10,7 +10,8 @@ digraph {
  node [style="filled"];
  "first-block_start" [shape=circle, label="start"];
  "first-block_start" -> "first-block_0" [label="frame"];
- "first-block_start" -> "first-block_end" [label="ordering"];
+ "first-block_start" -> "first-block_end" [label="frame
+ordering"];
  "first-block_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "first-block_0" -> "first-block_end" [label="frame"];
  "first-block_end" [shape=circle, label="end"];
@@ -22,7 +23,8 @@ digraph {
  node [style="filled"];
  "second-block_start" [shape=circle, label="start"];
  "second-block_start" -> "second-block_0" [label="frame"];
- "second-block_start" -> "second-block_end" [label="ordering"];
+ "second-block_start" -> "second-block_end" [label="frame
+ordering"];
  "second-block_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "second-block_0" -> "second-block_end" [label="frame"];
  "second-block_end" [shape=circle, label="end"];
@@ -33,7 +35,8 @@ digraph {
  node [style="filled"];
  "third-block_start" [shape=circle, label="start"];
  "third-block_start" -> "third-block_0" [label="frame"];
- "third-block_start" -> "third-block_end" [label="ordering"];
+ "third-block_start" -> "third-block_end" [label="frame
+ordering"];
  "third-block_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(duration: 1000000.0)"];
  "third-block_0" -> "third-block_end" [label="frame"];
  "third-block_end" [shape=circle, label="end"];

src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulse.snap

@@ -10,10 +10,13 @@ digraph {
  node [style="filled"];
  "block_0_start" [shape=circle, label="start"];
  "block_0_start" -> "block_0_0" [label="frame"];
- "block_0_start" -> "block_0_end" [label="ordering"];
+ "block_0_start" -> "block_0_1" [label="frame"];
+ "block_0_start" -> "block_0_end" [label="frame
+ordering"];
  "block_0_0" [shape=rectangle, label="[0] PULSE 0 \"rf\" test(a: param[0])"];
  "block_0_0" -> "block_0_1" [label="frame"];
- "block_0_0" -> "block_0_end" [label="await read"];
+ "block_0_0" -> "block_0_end" [label="await read
+frame"];
  "block_0_1" [shape=rectangle, label="[1] CAPTURE 0 \"ro_rx\" test(a: param[0]) ro[0]"];
  "block_0_1" -> "block_0_end" [label="await capture
 await read