From 11a8e97c55076ab09ef209a5d72da2f7efe1b728 Mon Sep 17 00:00:00 2001 From: kalzoo <22137047+kalzoo@users.noreply.github.com> Date: Wed, 1 Jun 2022 22:11:33 -0700 Subject: [PATCH 1/5] Fix: frame dependency calculation --- src/program/graph.rs | 123 +++++++++++++++--- src/program/graphviz_dot.rs | 29 +++++ ...sts__graph__active_reset_single_frame.snap | 3 +- ..._dot__tests__graph__blocking_2q_pulse.snap | 28 ++++ ...ph__blocking_pulses_after_nonblocking.snap | 29 +++++ ...aph__blocking_pulses_wrap_nonblocking.snap | 32 +++++ ...viz_dot__tests__graph__chained_pulses.snap | 7 +- ...sts__graph__different_frames_blocking.snap | 3 +- ...ram__graphviz_dot__tests__graph__jump.snap | 9 +- ...z_dot__tests__graph__parametric_pulse.snap | 7 +- ...rametric_pulses_using_capture_results.snap | 11 +- ...ot__tests__graph__pulse_after_capture.snap | 7 +- ...viz_dot__tests__graph__simple_capture.snap | 3 +- ..._dot__tests__graph__single_dependency.snap | 4 +- ...dot__tests__graph__single_instruction.snap | 3 +- 15 files changed, 267 insertions(+), 31 deletions(-) create mode 100644 src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_2q_pulse.snap create mode 100644 src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_after_nonblocking.snap create mode 100644 src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_wrap_nonblocking.snap diff --git a/src/program/graph.rs b/src/program/graph.rs index 2734be68..7a9bff82 100644 --- a/src/program/graph.rs +++ b/src/program/graph.rs @@ -200,6 +200,75 @@ 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, + blocking: HashSet, +} + +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. + pub fn register_user(&mut self, node: ScheduledGraphNode) -> HashSet { + 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. + pub fn register_blocker(&mut self, node: ScheduledGraphNode) -> Option { + self.blocking.insert(node); + self.using + } + + /// Consume the [PreviousNodes] and return all nodes within. + pub fn drain(mut self) -> HashSet { + if let Some(using) = self.using { + self.blocking.insert(using); + } + self.blocking + } +} + impl InstructionBlock { pub fn build( instructions: Vec, @@ -213,8 +282,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 = - HashMap::new(); + let mut last_instruction_by_frame: HashMap = 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. @@ -233,24 +301,47 @@ impl InstructionBlock { Ok(()) } InstructionRole::RFControl => { - let used_frames = program + let used_frames: HashSet<&FrameIdentifier> = program .get_frames_for_instruction(instruction, false) - .unwrap_or_default(); - let blocked_frames = program + .unwrap_or_default() + .into_iter() + .collect(); + let blocked_frames: HashSet<&FrameIdentifier> = program .get_frames_for_instruction(instruction, true) - .unwrap_or_default(); + .unwrap_or_default() + .into_iter() + .filter(|f| !used_frames.contains(f)) + .collect(); - // 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 previous_node_ids = last_instruction_by_frame + .entry(frame.clone()) + .or_insert(PreviousNodes { + using: None, + blocking: vec![ScheduledGraphNode::BlockStart] + .into_iter() + .collect(), + }) + .register_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); + if let Some(previous_node_id) = last_instruction_by_frame + .entry(frame.clone()) + .or_insert(PreviousNodes { + using: None, + blocking: vec![ScheduledGraphNode::BlockStart] + .into_iter() + .collect(), + }) + .register_blocker(node) + { + add_dependency!(graph, previous_node_id => node, ExecutionDependency::ReferenceFrame); + } } Ok(()) @@ -295,8 +386,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 (_, last_instructions) in last_instruction_by_frame { + for node in last_instructions.drain() { + add_dependency!(graph, node => ScheduledGraphNode::BlockEnd, ExecutionDependency::ReferenceFrame); + } } // Examine all "pending" memory operations for all regions diff --git a/src/program/graphviz_dot.rs b/src/program/graphviz_dot.rs index 1aecb7c1..b57d179b 100644 --- a/src/program/graphviz_dot.rs +++ b/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, " diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__active_reset_single_frame.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__active_reset_single_frame.snap index c000579d..8390726c 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__active_reset_single_frame.snap +++ b/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"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_2q_pulse.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_2q_pulse.snap new file mode 100644 index 00000000..a59ace26 --- /dev/null +++ b/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"]; + } +} + diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_after_nonblocking.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_after_nonblocking.snap new file mode 100644 index 00000000..280e2199 --- /dev/null +++ b/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"]; + } +} + diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_wrap_nonblocking.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__blocking_pulses_wrap_nonblocking.snap new file mode 100644 index 00000000..564ba450 --- /dev/null +++ b/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"]; + } +} + diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__chained_pulses.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__chained_pulses.snap index 8022d73d..f9d05c43 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__chained_pulses.snap +++ b/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"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__different_frames_blocking.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__different_frames_blocking.snap index 49e23e32..780cf10b 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__different_frames_blocking.snap +++ b/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)"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__jump.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__jump.snap index 822216f0..b7d023bd 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__jump.snap +++ b/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"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulse.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulse.snap index 0dce3c01..5adbce3d 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulse.snap +++ b/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 diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulses_using_capture_results.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulses_using_capture_results.snap index 79060814..42b0ff40 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulses_using_capture_results.snap +++ b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__parametric_pulses_using_capture_results.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_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] CAPTURE 0 \"ro_rx\" test(a: param[0]) ro[0]"]; "block_0_0" -> "block_0_1" [label="await capture frame"]; "block_0_0" -> "block_0_3" [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] NONBLOCKING PULSE 0 \"rf\" test(a: ro[0])"]; - "block_0_1" -> "block_0_3" [label="await read"]; + "block_0_1" -> "block_0_3" [label="await read +frame"]; + "block_0_1" -> "block_0_4" [label="frame"]; "block_0_2" [shape=rectangle, label="[2] NONBLOCKING PULSE 1 \"rf\" test(a: ro[0])"]; "block_0_2" -> "block_0_3" [label="await read"]; "block_0_2" -> "block_0_5" [label="frame"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__pulse_after_capture.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__pulse_after_capture.snap index c78798ff..4462cd8a 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__pulse_after_capture.snap +++ b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__pulse_after_capture.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] CAPTURE 0 \"ro_rx\" test() ro[0]"]; "block_0_0" -> "block_0_1" [label="frame"]; - "block_0_0" -> "block_0_end" [label="await capture"]; + "block_0_0" -> "block_0_end" [label="await capture +frame"]; "block_0_1" [shape=rectangle, label="[1] PULSE 0 \"rf\" test()"]; "block_0_1" -> "block_0_end" [label="frame"]; "block_0_end" [shape=circle, label="end"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__simple_capture.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__simple_capture.snap index aeacf601..6a8f05ae 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__simple_capture.snap +++ b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__simple_capture.snap @@ -10,7 +10,8 @@ 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] CAPTURE 0 \"ro_rx\" test() ro[0]"]; "block_0_0" -> "block_0_end" [label="await capture frame"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_dependency.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_dependency.snap index 708c543c..033b2bbe 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_dependency.snap +++ b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_dependency.snap @@ -10,9 +10,11 @@ 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_end" [label="frame"]; "block_0_end" [shape=circle, label="end"]; diff --git a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_instruction.snap b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_instruction.snap index 4fcc3157..c67d0e04 100644 --- a/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_instruction.snap +++ b/src/program/snapshots/quil_rs__program__graphviz_dot__tests__graph__single_instruction.snap @@ -10,7 +10,8 @@ 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_end" [label="frame"]; "block_0_end" [shape=circle, label="end"]; From 6263e8fa4ff49b1433d9b6bf9f50f6a30a06d9c6 Mon Sep 17 00:00:00 2001 From: kalzoo <22137047+kalzoo@users.noreply.github.com> Date: Fri, 3 Jun 2022 16:30:57 -0700 Subject: [PATCH 2/5] Breaking: Program.get_frames_for_instruction returns HashSet --- src/program/graph.rs | 20 +++++++++----------- src/program/mod.rs | 9 ++------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/program/graph.rs b/src/program/graph.rs index 7a9bff82..0cea03bc 100644 --- a/src/program/graph.rs +++ b/src/program/graph.rs @@ -301,19 +301,17 @@ impl InstructionBlock { Ok(()) } InstructionRole::RFControl => { - let used_frames: HashSet<&FrameIdentifier> = program + let used_frames = program .get_frames_for_instruction(instruction, false) - .unwrap_or_default() - .into_iter() - .collect(); - let blocked_frames: HashSet<&FrameIdentifier> = program + .unwrap_or_default(); + + let blocked_frames = program .get_frames_for_instruction(instruction, true) - .unwrap_or_default() - .into_iter() - .filter(|f| !used_frames.contains(f)) - .collect(); + .unwrap_or_default(); + + let blocked_but_not_used_frames = blocked_frames.difference(&used_frames); - for frame in used_frames { + for frame in &used_frames { let previous_node_ids = last_instruction_by_frame .entry(frame.clone()) .or_insert(PreviousNodes { @@ -329,7 +327,7 @@ impl InstructionBlock { } } - for frame in blocked_frames { + for frame in blocked_but_not_used_frames { if let Some(previous_node_id) = last_instruction_by_frame .entry(frame.clone()) .or_insert(PreviousNodes { diff --git a/src/program/mod.rs b/src/program/mod.rs index e18644cf..5a1d0804 100644 --- a/src/program/mod.rs +++ b/src/program/mod.rs @@ -130,15 +130,10 @@ impl Program { &'a self, instruction: &'a Instruction, include_blocked: bool, - ) -> Option> { + ) -> Option> { 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)) } pub fn to_instructions(&self, include_headers: bool) -> Vec { From 2786a207c424bb2d549b439b38b7a62ed72ad970 Mon Sep 17 00:00:00 2001 From: kalzoo <22137047+kalzoo@users.noreply.github.com> Date: Fri, 3 Jun 2022 16:31:34 -0700 Subject: [PATCH 3/5] Chore: code cleanup --- src/program/graph.rs | 36 ++++++++++++++++-------------------- src/program/mod.rs | 2 +- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/program/graph.rs b/src/program/graph.rs index 0cea03bc..8fdd53d1 100644 --- a/src/program/graph.rs +++ b/src/program/graph.rs @@ -240,7 +240,10 @@ 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. - pub fn register_user(&mut self, node: ScheduledGraphNode) -> HashSet { + pub fn get_dependencies_for_next_user( + &mut self, + node: ScheduledGraphNode, + ) -> HashSet { let mut result = std::mem::take(&mut self.blocking); if let Some(previous_user) = self.using.replace(node) { result.insert(previous_user); @@ -255,7 +258,10 @@ impl PreviousNodes { /// /// 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. - pub fn register_blocker(&mut self, node: ScheduledGraphNode) -> Option { + pub fn get_dependency_for_next_blocker( + &mut self, + node: ScheduledGraphNode, + ) -> Option { self.blocking.insert(node); self.using } @@ -313,14 +319,9 @@ impl InstructionBlock { for frame in &used_frames { let previous_node_ids = last_instruction_by_frame - .entry(frame.clone()) - .or_insert(PreviousNodes { - using: None, - blocking: vec![ScheduledGraphNode::BlockStart] - .into_iter() - .collect(), - }) - .register_user(node); + .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); @@ -329,14 +330,9 @@ impl InstructionBlock { for frame in blocked_but_not_used_frames { if let Some(previous_node_id) = last_instruction_by_frame - .entry(frame.clone()) - .or_insert(PreviousNodes { - using: None, - blocking: vec![ScheduledGraphNode::BlockStart] - .into_iter() - .collect(), - }) - .register_blocker(node) + .entry((*frame).clone()) + .or_default() + .get_dependency_for_next_blocker(node) { add_dependency!(graph, previous_node_id => node, ExecutionDependency::ReferenceFrame); } @@ -384,8 +380,8 @@ impl InstructionBlock { // does not terminate until these are complete add_dependency!(graph, last_classical_instruction => ScheduledGraphNode::BlockEnd, ExecutionDependency::StableOrdering); - for (_, last_instructions) in last_instruction_by_frame { - for node in last_instructions.drain() { + for previous_nodes in last_instruction_by_frame.into_values() { + for node in previous_nodes.drain() { add_dependency!(graph, node => ScheduledGraphNode::BlockEnd, ExecutionDependency::ReferenceFrame); } } diff --git a/src/program/mod.rs b/src/program/mod.rs index 5a1d0804..8b918ef8 100644 --- a/src/program/mod.rs +++ b/src/program/mod.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::str::FromStr; use crate::instruction::{ From 7a73d361f4ac3e8a9819602889137e796516e257 Mon Sep 17 00:00:00 2001 From: kalzoo <22137047+kalzoo@users.noreply.github.com> Date: Wed, 8 Jun 2022 11:09:08 -0700 Subject: [PATCH 4/5] Chore: correct docstring --- src/program/frame.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/program/frame.rs b/src/program/frame.rs index 53bfb8ef..8d941268 100644 --- a/src/program/frame.rs +++ b/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 From 73616a048c805d7dc7743ed716523891b88bc013 Mon Sep 17 00:00:00 2001 From: kalzoo <22137047+kalzoo@users.noreply.github.com> Date: Wed, 8 Jun 2022 11:09:58 -0700 Subject: [PATCH 5/5] Fix function visibility --- src/program/graph.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/program/graph.rs b/src/program/graph.rs index 8fdd53d1..529c89f5 100644 --- a/src/program/graph.rs +++ b/src/program/graph.rs @@ -240,7 +240,7 @@ 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. - pub fn get_dependencies_for_next_user( + fn get_dependencies_for_next_user( &mut self, node: ScheduledGraphNode, ) -> HashSet { @@ -258,7 +258,7 @@ impl PreviousNodes { /// /// 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. - pub fn get_dependency_for_next_blocker( + fn get_dependency_for_next_blocker( &mut self, node: ScheduledGraphNode, ) -> Option {