-
Notifications
You must be signed in to change notification settings - Fork 8
Re-spec and Implement OutlineCfg rewrite #225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
47cdc3f
f470736
b40aae4
9d4b261
610f1c7
e0f75c6
8f9b7c4
7981793
e8460dc
57a35a5
0fd40c9
4ae2fc9
59c2ce7
010061f
3528a65
9de20df
00092ad
ff2d0ac
a7940f3
e9c3c12
df4f1d7
85a05ad
e90de4d
7572924
6b38017
eea616c
cc44630
d64c3e0
6cbba93
5f4062b
4512bf7
ddf8fb6
381dc80
27463ae
f0d36c1
d7e9ad9
1588016
86e3fe2
dad6c5f
382e70c
369f62e
e5f83e6
6eca8a7
a4d02d9
37b2b7a
8d0d0b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
use itertools::Itertools; | ||
|
||
use super::{ | ||
build_traits::SubContainer, | ||
dataflow::{DFGBuilder, DFGWrapper}, | ||
|
@@ -95,6 +97,22 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> CFGBuilder<B> { | |
inputs: Some(input), | ||
}) | ||
} | ||
|
||
/// Create a CFGBuilder for an existing CFG node (that already has entry + exit nodes) | ||
pub(crate) fn from_existing(base: B, cfg_node: Node) -> Result<Self, BuildError> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably add a docstring for this? I must be missing something here but I don't see how we get away with throwing away the entry node... Is there a test for this method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not pub - but yes, docstring added. I've clued about the entry node (the thing is that creating a CFGBuilder doesn't create an entry node - you have to do that explicitly later using e.g. And added a test. (That creates a disconnected basic block, but it'll do for the test.) |
||
let OpType::CFG(crate::ops::controlflow::CFG {outputs, ..}) = base.get_optype(cfg_node) | ||
else {return Err(BuildError::UnexpectedType{node: cfg_node, op_desc: "Any CFG"});}; | ||
let n_out_wires = outputs.len(); | ||
let (_, exit_node) = base.children(cfg_node).take(2).collect_tuple().unwrap(); | ||
Ok(Self { | ||
base, | ||
cfg_node, | ||
inputs: None, // This will prevent creating an entry node | ||
exit_node, | ||
n_out_wires, | ||
}) | ||
} | ||
|
||
/// Return a builder for a non-entry [`BasicBlock::DFB`] child graph with `inputs` | ||
/// and `outputs` and the variants of the branching predicate Sum value | ||
/// specified by `predicate_variants`. | ||
|
@@ -278,6 +296,8 @@ impl BlockBuilder<Hugr> { | |
|
||
#[cfg(test)] | ||
mod test { | ||
use std::collections::HashSet; | ||
|
||
use cool_asserts::assert_matches; | ||
|
||
use crate::builder::build_traits::HugrBuilder; | ||
|
@@ -319,6 +339,36 @@ mod test { | |
|
||
Ok(()) | ||
} | ||
#[test] | ||
fn from_existing() -> Result<(), BuildError> { | ||
let mut cfg_builder = CFGBuilder::new(type_row![NAT], type_row![NAT])?; | ||
build_basic_cfg(&mut cfg_builder)?; | ||
let h = cfg_builder.finish_hugr()?; | ||
|
||
let mut new_builder = CFGBuilder::from_existing(h.clone(), h.root())?; | ||
assert_matches!(new_builder.simple_entry_builder(type_row![NAT], 1), Err(_)); | ||
let h2 = new_builder.finish_hugr()?; | ||
assert_eq!(h, h2); // No new nodes added | ||
|
||
let mut new_builder = CFGBuilder::from_existing(h.clone(), h.root())?; | ||
let block_builder = new_builder.simple_block_builder( | ||
vec![SimpleType::new_simple_predicate(1), NAT].into(), | ||
type_row![NAT], | ||
1, | ||
)?; | ||
let new_bb = block_builder.container_node(); | ||
let [pred, nat]: [Wire; 2] = block_builder.input_wires_arr(); | ||
block_builder.finish_with_outputs(pred, [nat])?; | ||
let h2 = new_builder.finish_hugr()?; | ||
let expected_nodes = h | ||
.children(h.root()) | ||
.chain([new_bb]) | ||
.collect::<HashSet<Node>>(); | ||
assert_eq!(expected_nodes, HashSet::from_iter(h2.children(h2.root()))); | ||
|
||
Ok(()) | ||
} | ||
|
||
fn build_basic_cfg<T: AsMut<Hugr> + AsRef<Hugr>>( | ||
cfg_builder: &mut CFGBuilder<T>, | ||
) -> Result<(), BuildError> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a driveby, but I'm doing the same in the new code. (It seemed small enough just to duplicate.)