From 47cdc3fe4012ce149ce3b35061aad81db5d32ed9 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 10:21:55 +0100 Subject: [PATCH 01/41] simple_replace.rs: use HugrMut::remove_node, includes clearing op_types --- src/hugr/rewrite/simple_replace.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index e688fd342..32fd58698 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use itertools::Itertools; -use portgraph::{LinkMut, LinkView, MultiMut, NodeIndex, PortMut, PortView}; +use portgraph::{LinkMut, LinkView, MultiMut, NodeIndex, PortView}; use crate::hugr::{HugrMut, HugrView}; use crate::{ @@ -227,8 +227,7 @@ impl Rewrite for SimpleReplacement { } // 3.5. Remove all nodes in self.removal and edges between them. for node in &self.removal { - h.graph.remove_node(node.index); - h.hierarchy.remove(node.index); + h.remove_node(*node).unwrap(); } Ok(()) } From f4707361cf2d2140397155dea99cdef6fb7f65ed Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Fri, 23 Jun 2023 11:57:28 +0100 Subject: [PATCH 02/41] WIP add outline_cfg.rs --- src/hugr/rewrite.rs | 1 + src/hugr/rewrite/outline_cfg.rs | 121 ++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 src/hugr/rewrite/outline_cfg.rs diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index dda3cde8d..9f9face99 100644 --- a/src/hugr/rewrite.rs +++ b/src/hugr/rewrite.rs @@ -1,5 +1,6 @@ //! Rewrite operations on the HUGR - replacement, outlining, etc. +pub mod outline_cfg; pub mod replace; pub mod simple_replace; use std::mem; diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs new file mode 100644 index 000000000..6004e8439 --- /dev/null +++ b/src/hugr/rewrite/outline_cfg.rs @@ -0,0 +1,121 @@ +use std::collections::{VecDeque, HashSet}; + +use thiserror::Error; + +use crate::hugr::{HugrView, HugrMut}; +use crate::ops::{OpType, BasicBlock, CFG, OpTrait}; +use crate::types::{TypeRow, SimpleType}; +use crate::{Hugr, Node, type_row}; +use crate::hugr::rewrite::Rewrite; + +/// Moves part of a Control-flow Sibling Graph into a new CFG-node +/// that is the only child of a new Basic Block in the original CSG. +pub struct OutlineCfg { + /// Will become the entry block in the new sub-cfg. + /// All edges "in" to this block will be redirected to the new block + /// (excluding backedges from internal BBs) + entry_node: Node, + /// All successors of this node that are *not* already forwards-reachable + /// from the entry_node (avoiding the exit_node) will become successors + /// of the sub-cfg. + exit_node: Node +} + +impl OutlineCfg { + fn compute_all_blocks(&self, h: &Hugr) -> Result, OutlineCfgError> { + let cfg_n = match (h.get_parent(self.entry_node), h.get_parent(self.exit_node)) { + (Some(p1), Some(p2)) if p1==p2 => p1, + (p1, p2) => {return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2))} + }; + match h.get_optype(cfg_n) { + OpType::CFG(_) => (), + o => {return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone()));} + }; + let mut all_blocks = HashSet::new(); + let mut queue = VecDeque::new(); + queue.push_back(self.entry_node); + while let Some(n) = queue.pop_front() { + // This puts the exit_node into 'all_blocks' but not its successors + if all_blocks.insert(n) && n != self.exit_node { + queue.extend(h.output_neighbours(n)); + } + } + if !all_blocks.contains(&self.exit_node) { return Err(OutlineCfgError::ExitNodeUnreachable);} + Ok(all_blocks) + } +} + +impl Rewrite for OutlineCfg { + type Error = OutlineCfgError; + const UNCHANGED_ON_FAILURE: bool = true; + fn verify(&self, h: &Hugr) -> Result<(), OutlineCfgError> { + self.compute_all_blocks(h)?; + Ok(()) + } + fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { + let all_blocks = self.compute_all_blocks(h)?; + // 1. Add new CFG node + // These panic()s only happen if the Hugr would not have passed validate() + let inputs = h.get_optype(h.children(self.entry_node).next().unwrap() // input node is first child + ).signature().output; + let (pred_vars, outputs) = match h.get_optype(self.exit_node) { + OpType::BasicBlock(BasicBlock::Exit { cfg_outputs }) => (vec![], cfg_outputs.clone()), + OpType::BasicBlock(BasicBlock::DFB { inputs: _, other_outputs, predicate_variants }) => { + assert!(predicate_variants.len() > 0); + // Return predicate variants for the edges that exit + let exitting_predicate_variants: Vec = predicate_variants.into_iter().zip(h.output_neighbours(self.exit_node)).filter_map( + |(t,suc)|if all_blocks.contains(&suc) {None} else {Some(t.clone())}).collect(); + assert!(exitting_predicate_variants.len() > 0); // not guaranteed, if the "exit block" always loops back - TODO handle this case + (exitting_predicate_variants, other_outputs.clone()) + }, + _ => panic!("Cfg had non-BB child") + }; + let cfg_outputs = if pred_vars.len()==0 {outputs.clone()} else { + // When this block is moved into the sub-cfg, we'll need to route all of the exitting edges + // to an actual exit-block, which will pass through a sum-of-tuple type. + // Each exitting edge will need to go through an additional basic-block that + // tuples and then sum-injects those arguments to make a value of that predicate type. + let mut outputs2 = vec![SimpleType::new_predicate(pred_vars.clone())]; + outputs2.extend_from_slice(&outputs); + outputs2.into() + }; + let sub_cfg = h.add_op(CFG { + inputs: inputs.clone(), outputs: cfg_outputs.clone() + }); + // 2. New CFG node will be contained in new BB + let new_block = h.add_op(BasicBlock::DFB { + inputs, other_outputs: outputs, predicate_variants: if pred_vars.len()==0 {vec![type_row![]]} else {pred_vars.clone()} + }); + //TODO: dfg Input and Output nodes. Use builder instead? + h.hierarchy.push_child(sub_cfg.index, new_block.index); + // 3. Inner CFG needs an exit node + let inner_exit = h.add_op(OpType::BasicBlock(BasicBlock::Exit { cfg_outputs })); + h.hierarchy.push_child(inner_exit.index, sub_cfg.index); + // 4. Reparent nodes - entry node must be first + h.hierarchy.detach(self.entry_node.index); + h.hierarchy.insert_before(self.entry_node.index, inner_exit.index); + for n in all_blocks { + // Do not move the entry node, as we have already; + // don't move the exit_node if it's the exit-block of the outer CFG + if n == self.entry_node || (n == self.exit_node && pred_vars.len()==0) {continue}; + h.hierarchy.detach(n.index); + h.hierarchy.insert_before(n.index, inner_exit.index); + } + + // 5.a. and redirect exitting edges from exit_node to the new (inner) exit block + // 5.b. and also from new_block to the old successors + // 6. redirect edges to entry_block from outside, to new_block + + Ok(()) + } +} + +#[derive(Debug, Error)] +pub enum OutlineCfgError { + #[error("The exit node could not be reached from the entry node")] + ExitNodeUnreachable, + #[error("Entry node {0:?} and exit node {1:?} are not siblings")] + EntryExitNotSiblings(Option,Option), + #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] + ParentNotCfg(Node, OpType) +} \ No newline at end of file From b40aae49b3d240edbf105415958970d9fead3b1a Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Fri, 23 Jun 2023 14:40:46 +0100 Subject: [PATCH 03/41] Simplify spec --- specification/hugr.md | 20 ++++++--- src/hugr/rewrite/outline_cfg.rs | 72 ++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index 41dde2050..f71c6171f 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1230,11 +1230,21 @@ nodes as children. ###### `OutlineCFG` -Replace a CFG-convex subgraph (of sibling BasicBlock nodes) having a -single entry node with a single BasicBlock node having a CFG node child -which has as its children the original BasicBlock nodes and an exit node -that has inputs coming from all edges of the original CFG that don’t -terminate in it. +Replace a set of CFG sibling nodes with a single BasicBlock node having a +CFG node child which has as its children the set of BasicBlock nodes +originally specified. The set of basic blocks must satisfy constraints: +* All (controlflow) edges whose target is in the set but whose source is outside, + must have the same target +* There must be at most one edge leaving the set; specifically, + * *either* the set does not contain an Exit block, and there is exactly one edge leaving the set; + * *or* the set contains the Exit block, and there are not edges out of the set. + +Situations in which multiple nodes have edges leaving the set, can be converted to +this form by a combination of InsertIdentity operations and one Replace. + +** Implementation Note ** The required form of set can be easily identified by two +nodes: the unique entry node, and an exit node (which may be the Exit block of the +CFG, or the source of the unique exit edge). ##### Inlining methods diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 6004e8439..f529a7198 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -4,7 +4,6 @@ use thiserror::Error; use crate::hugr::{HugrView, HugrMut}; use crate::ops::{OpType, BasicBlock, CFG, OpTrait}; -use crate::types::{TypeRow, SimpleType}; use crate::{Hugr, Node, type_row}; use crate::hugr::rewrite::Rewrite; @@ -15,14 +14,14 @@ pub struct OutlineCfg { /// All edges "in" to this block will be redirected to the new block /// (excluding backedges from internal BBs) entry_node: Node, - /// All successors of this node that are *not* already forwards-reachable - /// from the entry_node (avoiding the exit_node) will become successors - /// of the sub-cfg. + /// Either the exit node of the parent CFG; or, another node, with + /// exactly one successor that is not reachable from the entry_node + /// (without going through the exit_node). exit_node: Node } impl OutlineCfg { - fn compute_all_blocks(&self, h: &Hugr) -> Result, OutlineCfgError> { + fn compute_all_blocks(&self, h: &Hugr) -> Result<(HashSet, Option), OutlineCfgError> { let cfg_n = match (h.get_parent(self.entry_node), h.get_parent(self.exit_node)) { (Some(p1), Some(p2)) if p1==p2 => p1, (p1, p2) => {return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2))} @@ -41,7 +40,19 @@ impl OutlineCfg { } } if !all_blocks.contains(&self.exit_node) { return Err(OutlineCfgError::ExitNodeUnreachable);} - Ok(all_blocks) + if matches!(h.get_optype(self.exit_node), OpType::BasicBlock(BasicBlock::Exit { .. })) { + return Ok((all_blocks, None)); + }; + let mut succ = None; + for exit_succ in h.output_neighbours(self.exit_node) { + if all_blocks.contains(&exit_succ) {continue;} + if let Some(s) = succ { + return Err(OutlineCfgError::MultipleExitSuccessors(s, exit_succ)); + } + succ = Some(exit_succ); + } + assert!(succ.is_some()); + Ok((all_blocks, succ)) } } @@ -53,38 +64,33 @@ impl Rewrite for OutlineCfg { Ok(()) } fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { - let all_blocks = self.compute_all_blocks(h)?; + let (all_blocks, cfg_succ) = self.compute_all_blocks(h)?; // 1. Add new CFG node // These panic()s only happen if the Hugr would not have passed validate() let inputs = h.get_optype(h.children(self.entry_node).next().unwrap() // input node is first child ).signature().output; - let (pred_vars, outputs) = match h.get_optype(self.exit_node) { - OpType::BasicBlock(BasicBlock::Exit { cfg_outputs }) => (vec![], cfg_outputs.clone()), - OpType::BasicBlock(BasicBlock::DFB { inputs: _, other_outputs, predicate_variants }) => { - assert!(predicate_variants.len() > 0); - // Return predicate variants for the edges that exit - let exitting_predicate_variants: Vec = predicate_variants.into_iter().zip(h.output_neighbours(self.exit_node)).filter_map( - |(t,suc)|if all_blocks.contains(&suc) {None} else {Some(t.clone())}).collect(); - assert!(exitting_predicate_variants.len() > 0); // not guaranteed, if the "exit block" always loops back - TODO handle this case - (exitting_predicate_variants, other_outputs.clone()) - }, - _ => panic!("Cfg had non-BB child") - }; - let cfg_outputs = if pred_vars.len()==0 {outputs.clone()} else { - // When this block is moved into the sub-cfg, we'll need to route all of the exitting edges - // to an actual exit-block, which will pass through a sum-of-tuple type. - // Each exitting edge will need to go through an additional basic-block that - // tuples and then sum-injects those arguments to make a value of that predicate type. - let mut outputs2 = vec![SimpleType::new_predicate(pred_vars.clone())]; - outputs2.extend_from_slice(&outputs); - outputs2.into() - }; + let cfg_outputs = { + let OpType::BasicBlock(exit_type) = h.get_optype(self.exit_node) else {panic!()}; + match exit_type { + BasicBlock::Exit { cfg_outputs } => cfg_outputs, + BasicBlock::DFB { .. } => { + let Some(s) = cfg_succ else {panic!();}; + assert!(h.output_neighbours(self.exit_node).collect::>().contains(&s)); + let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; + match s_type { + BasicBlock::Exit { cfg_outputs } => cfg_outputs, + BasicBlock::DFB { inputs, .. } => inputs + } + } + } + }.clone(); + let sub_cfg = h.add_op(CFG { inputs: inputs.clone(), outputs: cfg_outputs.clone() }); // 2. New CFG node will be contained in new BB let new_block = h.add_op(BasicBlock::DFB { - inputs, other_outputs: outputs, predicate_variants: if pred_vars.len()==0 {vec![type_row![]]} else {pred_vars.clone()} + inputs, other_outputs: cfg_outputs.clone(), predicate_variants: vec![type_row![]] }); //TODO: dfg Input and Output nodes. Use builder instead? h.hierarchy.push_child(sub_cfg.index, new_block.index); @@ -96,8 +102,8 @@ impl Rewrite for OutlineCfg { h.hierarchy.insert_before(self.entry_node.index, inner_exit.index); for n in all_blocks { // Do not move the entry node, as we have already; - // don't move the exit_node if it's the exit-block of the outer CFG - if n == self.entry_node || (n == self.exit_node && pred_vars.len()==0) {continue}; + // TODO??? don't move the exit_node if it's the exit-block of the outer CFG + if n == self.entry_node {continue}; h.hierarchy.detach(n.index); h.hierarchy.insert_before(n.index, inner_exit.index); } @@ -117,5 +123,7 @@ pub enum OutlineCfgError { #[error("Entry node {0:?} and exit node {1:?} are not siblings")] EntryExitNotSiblings(Option,Option), #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] - ParentNotCfg(Node, OpType) + ParentNotCfg(Node, OpType), + #[error("Exit node had multiple successors outside CFG - at least {0:?} and {1:?}")] + MultipleExitSuccessors(Node, Node) } \ No newline at end of file From 9d4b2613fd718446e625350afeaf3bfd500acd93 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 10:22:37 +0100 Subject: [PATCH 04/41] Outline-cfg compiles, non-exit-block-moving version; expose DFGBuilder::create_with_io --- src/builder/dataflow.rs | 2 +- src/hugr/rewrite/outline_cfg.rs | 167 ++++++++++++++++++++++++-------- 2 files changed, 126 insertions(+), 43 deletions(-) diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index 0804bad48..ebc56eba1 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -21,7 +21,7 @@ pub struct DFGBuilder { } impl + AsRef> DFGBuilder { - pub(super) fn create_with_io( + pub(crate) fn create_with_io( mut base: T, parent: Node, signature: Signature, diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index f529a7198..d5d932532 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -1,11 +1,14 @@ -use std::collections::{VecDeque, HashSet}; +use std::collections::{HashSet, VecDeque}; +use itertools::Itertools; use thiserror::Error; -use crate::hugr::{HugrView, HugrMut}; -use crate::ops::{OpType, BasicBlock, CFG, OpTrait}; -use crate::{Hugr, Node, type_row}; +use crate::builder::{Container, DFGBuilder, Dataflow, DataflowSubContainer, SubContainer}; use crate::hugr::rewrite::Rewrite; +use crate::hugr::{HugrMut, HugrView}; +use crate::ops::{BasicBlock, OpTrait, OpType}; +use crate::types::Signature; +use crate::{type_row, Hugr, Node}; /// Moves part of a Control-flow Sibling Graph into a new CFG-node /// that is the only child of a new Basic Block in the original CSG. @@ -17,18 +20,23 @@ pub struct OutlineCfg { /// Either the exit node of the parent CFG; or, another node, with /// exactly one successor that is not reachable from the entry_node /// (without going through the exit_node). - exit_node: Node + exit_node: Node, } impl OutlineCfg { - fn compute_all_blocks(&self, h: &Hugr) -> Result<(HashSet, Option), OutlineCfgError> { + fn compute_all_blocks( + &self, + h: &Hugr, + ) -> Result<(HashSet, Option), OutlineCfgError> { let cfg_n = match (h.get_parent(self.entry_node), h.get_parent(self.exit_node)) { - (Some(p1), Some(p2)) if p1==p2 => p1, - (p1, p2) => {return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2))} + (Some(p1), Some(p2)) if p1 == p2 => p1, + (p1, p2) => return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2)), }; match h.get_optype(cfg_n) { OpType::CFG(_) => (), - o => {return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone()));} + o => { + return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone())); + } }; let mut all_blocks = HashSet::new(); let mut queue = VecDeque::new(); @@ -39,13 +47,20 @@ impl OutlineCfg { queue.extend(h.output_neighbours(n)); } } - if !all_blocks.contains(&self.exit_node) { return Err(OutlineCfgError::ExitNodeUnreachable);} - if matches!(h.get_optype(self.exit_node), OpType::BasicBlock(BasicBlock::Exit { .. })) { + if !all_blocks.contains(&self.exit_node) { + return Err(OutlineCfgError::ExitNodeUnreachable); + } + if matches!( + h.get_optype(self.exit_node), + OpType::BasicBlock(BasicBlock::Exit { .. }) + ) { return Ok((all_blocks, None)); }; let mut succ = None; for exit_succ in h.output_neighbours(self.exit_node) { - if all_blocks.contains(&exit_succ) {continue;} + if all_blocks.contains(&exit_succ) { + continue; + } if let Some(s) = succ { return Err(OutlineCfgError::MultipleExitSuccessors(s, exit_succ)); } @@ -65,53 +80,121 @@ impl Rewrite for OutlineCfg { } fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { let (all_blocks, cfg_succ) = self.compute_all_blocks(h)?; - // 1. Add new CFG node + // Gather data. // These panic()s only happen if the Hugr would not have passed validate() - let inputs = h.get_optype(h.children(self.entry_node).next().unwrap() // input node is first child - ).signature().output; + let inputs = h + .get_optype( + h.children(self.entry_node).next().unwrap(), // input node is first child + ) + .signature() + .output; let cfg_outputs = { let OpType::BasicBlock(exit_type) = h.get_optype(self.exit_node) else {panic!()}; match exit_type { BasicBlock::Exit { cfg_outputs } => cfg_outputs, BasicBlock::DFB { .. } => { let Some(s) = cfg_succ else {panic!();}; - assert!(h.output_neighbours(self.exit_node).collect::>().contains(&s)); + assert!(h + .output_neighbours(self.exit_node) + .collect::>() + .contains(&s)); let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; match s_type { BasicBlock::Exit { cfg_outputs } => cfg_outputs, - BasicBlock::DFB { inputs, .. } => inputs + BasicBlock::DFB { inputs, .. } => inputs, } } } - }.clone(); - - let sub_cfg = h.add_op(CFG { - inputs: inputs.clone(), outputs: cfg_outputs.clone() - }); - // 2. New CFG node will be contained in new BB + } + .clone(); + let parent = h.get_parent(self.entry_node).unwrap(); + + // 2. New CFG node will be contained in new single-successor BB let new_block = h.add_op(BasicBlock::DFB { - inputs, other_outputs: cfg_outputs.clone(), predicate_variants: vec![type_row![]] + inputs: inputs.clone(), + other_outputs: cfg_outputs.clone(), + predicate_variants: vec![type_row![]], }); - //TODO: dfg Input and Output nodes. Use builder instead? - h.hierarchy.push_child(sub_cfg.index, new_block.index); - // 3. Inner CFG needs an exit node - let inner_exit = h.add_op(OpType::BasicBlock(BasicBlock::Exit { cfg_outputs })); - h.hierarchy.push_child(inner_exit.index, sub_cfg.index); - // 4. Reparent nodes - entry node must be first + h.hierarchy + .push_child(new_block.index, parent.index) + .unwrap(); + // Change any edges into entry_block from outside, to target new_block + let preds: Vec<_> = h + .linked_ports( + self.entry_node, + h.node_inputs(self.entry_node).exactly_one().unwrap(), + ) + .collect(); + for (pred, br) in preds { + if !all_blocks.contains(&pred) { + h.disconnect(pred, br).unwrap(); + h.connect(pred, br.index(), new_block, 0).unwrap(); + } + } + // 3. new_block contains input node, sub-cfg, exit node all connected + let mut b = DFGBuilder::create_with_io( + &mut *h, + new_block, + Signature::new_df(inputs.clone(), cfg_outputs.clone()), + ) + .unwrap(); + let wires_in = inputs.into_iter().cloned().zip(b.input_wires()); + let sub_cfg = b.cfg_builder(wires_in, cfg_outputs.clone()).unwrap(); + let sub_cfg_node = sub_cfg.container_node(); + let sub_cfg_outputs = sub_cfg.finish_sub_container().unwrap().outputs(); + b.finish_with_outputs(sub_cfg_outputs).unwrap(); + + // 4. Children of new CFG - entry node must be first h.hierarchy.detach(self.entry_node.index); - h.hierarchy.insert_before(self.entry_node.index, inner_exit.index); + h.hierarchy + .push_child(self.entry_node.index, sub_cfg_node.index) + .unwrap(); + // Make an exit node, next + let inner_exit = h + .add_op_after( + self.entry_node, + OpType::BasicBlock(BasicBlock::Exit { cfg_outputs }), + ) + .unwrap(); + // Then reparent remainder for n in all_blocks { // Do not move the entry node, as we have already; - // TODO??? don't move the exit_node if it's the exit-block of the outer CFG - if n == self.entry_node {continue}; + // Also don't move the exit_node if it's the exit-block of the outer CFG + if n == self.entry_node || (n == self.exit_node && cfg_succ.is_none()) { + continue; + }; h.hierarchy.detach(n.index); - h.hierarchy.insert_before(n.index, inner_exit.index); + h.hierarchy.insert_after(n.index, inner_exit.index).unwrap(); + } + // Connect new_block to (old) successor of exit block + if let Some(s) = cfg_succ { + let exit_port = h + .node_outputs(self.exit_node) + .filter(|p| { + let (t, p2) = h + .linked_ports(self.exit_node, *p) + .exactly_one() + .ok() + .unwrap(); + assert!(p2.index() == 0); + t == s + }) + .exactly_one() + .unwrap(); + h.disconnect(self.exit_node, exit_port).unwrap(); // TODO need to connect this port to the inner exit block + h.connect(self.exit_node, exit_port.index(), inner_exit, 0).unwrap(); + h.connect(new_block, 0, s, 0).unwrap(); + } else { + // We left the exit block outside. So, it's *predecessors* need to be retargetted to the inner_exit. + let in_p = h.node_inputs(self.exit_node).exactly_one().unwrap(); + assert!(in_p.index() == 0); + let preds = h.linked_ports(self.exit_node, in_p).collect::>(); + for (src_n,src_p) in preds { + h.disconnect(src_n, src_p).unwrap(); + h.connect(src_n, src_p.index(), inner_exit, 0).unwrap(); + } + h.connect(new_block, 0, self.exit_node, 0).unwrap(); } - - // 5.a. and redirect exitting edges from exit_node to the new (inner) exit block - // 5.b. and also from new_block to the old successors - // 6. redirect edges to entry_block from outside, to new_block - Ok(()) } } @@ -121,9 +204,9 @@ pub enum OutlineCfgError { #[error("The exit node could not be reached from the entry node")] ExitNodeUnreachable, #[error("Entry node {0:?} and exit node {1:?} are not siblings")] - EntryExitNotSiblings(Option,Option), + EntryExitNotSiblings(Option, Option), #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] ParentNotCfg(Node, OpType), #[error("Exit node had multiple successors outside CFG - at least {0:?} and {1:?}")] - MultipleExitSuccessors(Node, Node) -} \ No newline at end of file + MultipleExitSuccessors(Node, Node), +} From 610f1c79a8e943e41b88e7421bd6c810301544a1 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:38:26 +0100 Subject: [PATCH 05/41] Common-up connecting new_block's successor --- src/hugr/rewrite/outline_cfg.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index d5d932532..1d33e0b0b 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -183,7 +183,6 @@ impl Rewrite for OutlineCfg { .unwrap(); h.disconnect(self.exit_node, exit_port).unwrap(); // TODO need to connect this port to the inner exit block h.connect(self.exit_node, exit_port.index(), inner_exit, 0).unwrap(); - h.connect(new_block, 0, s, 0).unwrap(); } else { // We left the exit block outside. So, it's *predecessors* need to be retargetted to the inner_exit. let in_p = h.node_inputs(self.exit_node).exactly_one().unwrap(); @@ -193,8 +192,8 @@ impl Rewrite for OutlineCfg { h.disconnect(src_n, src_p).unwrap(); h.connect(src_n, src_p.index(), inner_exit, 0).unwrap(); } - h.connect(new_block, 0, self.exit_node, 0).unwrap(); } + h.connect(new_block, 0, cfg_succ.unwrap_or(self.exit_node), 0).unwrap(); Ok(()) } } From e0f75c6060bc7f21e7c7e9988b85cd76bd5c096e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:40:39 +0100 Subject: [PATCH 06/41] Rename cfg_outputs => outputs --- src/hugr/rewrite/outline_cfg.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 1d33e0b0b..6a08b73ce 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -88,7 +88,7 @@ impl Rewrite for OutlineCfg { ) .signature() .output; - let cfg_outputs = { + let outputs = { let OpType::BasicBlock(exit_type) = h.get_optype(self.exit_node) else {panic!()}; match exit_type { BasicBlock::Exit { cfg_outputs } => cfg_outputs, @@ -112,7 +112,7 @@ impl Rewrite for OutlineCfg { // 2. New CFG node will be contained in new single-successor BB let new_block = h.add_op(BasicBlock::DFB { inputs: inputs.clone(), - other_outputs: cfg_outputs.clone(), + other_outputs: outputs.clone(), predicate_variants: vec![type_row![]], }); h.hierarchy @@ -135,11 +135,11 @@ impl Rewrite for OutlineCfg { let mut b = DFGBuilder::create_with_io( &mut *h, new_block, - Signature::new_df(inputs.clone(), cfg_outputs.clone()), + Signature::new_df(inputs.clone(), outputs.clone()), ) .unwrap(); let wires_in = inputs.into_iter().cloned().zip(b.input_wires()); - let sub_cfg = b.cfg_builder(wires_in, cfg_outputs.clone()).unwrap(); + let sub_cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); let sub_cfg_node = sub_cfg.container_node(); let sub_cfg_outputs = sub_cfg.finish_sub_container().unwrap().outputs(); b.finish_with_outputs(sub_cfg_outputs).unwrap(); @@ -153,7 +153,7 @@ impl Rewrite for OutlineCfg { let inner_exit = h .add_op_after( self.entry_node, - OpType::BasicBlock(BasicBlock::Exit { cfg_outputs }), + OpType::BasicBlock(BasicBlock::Exit { cfg_outputs: outputs }), ) .unwrap(); // Then reparent remainder From 8f9b7c4baac3c1e144d7bda0f5191acaa2094735 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:42:25 +0100 Subject: [PATCH 07/41] sub_cfg(_XXX) => cfg(_XXX), also _node.index => _index --- src/hugr/rewrite/outline_cfg.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 6a08b73ce..38ff49e36 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -139,15 +139,15 @@ impl Rewrite for OutlineCfg { ) .unwrap(); let wires_in = inputs.into_iter().cloned().zip(b.input_wires()); - let sub_cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); - let sub_cfg_node = sub_cfg.container_node(); - let sub_cfg_outputs = sub_cfg.finish_sub_container().unwrap().outputs(); - b.finish_with_outputs(sub_cfg_outputs).unwrap(); + let cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); + let cfg_index = cfg.container_node().index; + let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); + b.finish_with_outputs(cfg_outputs).unwrap(); // 4. Children of new CFG - entry node must be first h.hierarchy.detach(self.entry_node.index); h.hierarchy - .push_child(self.entry_node.index, sub_cfg_node.index) + .push_child(self.entry_node.index, cfg_index) .unwrap(); // Make an exit node, next let inner_exit = h From 7981793cd28a0fd029c4438bdc7138bf22a053b6 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:46:51 +0100 Subject: [PATCH 08/41] fmt, remove obsolete comment --- src/hugr/rewrite/outline_cfg.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 38ff49e36..6325b404e 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -153,7 +153,9 @@ impl Rewrite for OutlineCfg { let inner_exit = h .add_op_after( self.entry_node, - OpType::BasicBlock(BasicBlock::Exit { cfg_outputs: outputs }), + OpType::BasicBlock(BasicBlock::Exit { + cfg_outputs: outputs, + }), ) .unwrap(); // Then reparent remainder @@ -181,19 +183,21 @@ impl Rewrite for OutlineCfg { }) .exactly_one() .unwrap(); - h.disconnect(self.exit_node, exit_port).unwrap(); // TODO need to connect this port to the inner exit block - h.connect(self.exit_node, exit_port.index(), inner_exit, 0).unwrap(); + h.disconnect(self.exit_node, exit_port).unwrap(); + h.connect(self.exit_node, exit_port.index(), inner_exit, 0) + .unwrap(); } else { // We left the exit block outside. So, it's *predecessors* need to be retargetted to the inner_exit. let in_p = h.node_inputs(self.exit_node).exactly_one().unwrap(); assert!(in_p.index() == 0); let preds = h.linked_ports(self.exit_node, in_p).collect::>(); - for (src_n,src_p) in preds { + for (src_n, src_p) in preds { h.disconnect(src_n, src_p).unwrap(); h.connect(src_n, src_p.index(), inner_exit, 0).unwrap(); } } - h.connect(new_block, 0, cfg_succ.unwrap_or(self.exit_node), 0).unwrap(); + h.connect(new_block, 0, cfg_succ.unwrap_or(self.exit_node), 0) + .unwrap(); Ok(()) } } From e8460dc3502d020793a689efb532537a4641d626 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:47:40 +0100 Subject: [PATCH 09/41] Compute outputs by first matching on cfg_succ --- src/hugr/rewrite/outline_cfg.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 6325b404e..e3370d353 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -88,21 +88,16 @@ impl Rewrite for OutlineCfg { ) .signature() .output; - let outputs = { - let OpType::BasicBlock(exit_type) = h.get_optype(self.exit_node) else {panic!()}; - match exit_type { - BasicBlock::Exit { cfg_outputs } => cfg_outputs, - BasicBlock::DFB { .. } => { - let Some(s) = cfg_succ else {panic!();}; - assert!(h - .output_neighbours(self.exit_node) - .collect::>() - .contains(&s)); - let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; - match s_type { - BasicBlock::Exit { cfg_outputs } => cfg_outputs, - BasicBlock::DFB { inputs, .. } => inputs, - } + let outputs = match cfg_succ { + None => { + let OpType::BasicBlock(BasicBlock::Exit {cfg_outputs}) = h.get_optype(self.exit_node) else {panic!()}; + cfg_outputs + }, + Some(s) => { + let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; + match s_type { + BasicBlock::Exit { cfg_outputs } => cfg_outputs, + BasicBlock::DFB { inputs, .. } => inputs, } } } From 57a35a53da2ef020b8541e29f6004ec01cabac27 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 13:48:25 +0100 Subject: [PATCH 10/41] Rename cfg_succ => exit_succ --- src/hugr/rewrite/outline_cfg.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index e3370d353..c13a2a776 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -79,7 +79,7 @@ impl Rewrite for OutlineCfg { Ok(()) } fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { - let (all_blocks, cfg_succ) = self.compute_all_blocks(h)?; + let (all_blocks, exit_succ) = self.compute_all_blocks(h)?; // Gather data. // These panic()s only happen if the Hugr would not have passed validate() let inputs = h @@ -88,7 +88,7 @@ impl Rewrite for OutlineCfg { ) .signature() .output; - let outputs = match cfg_succ { + let outputs = match exit_succ { None => { let OpType::BasicBlock(BasicBlock::Exit {cfg_outputs}) = h.get_optype(self.exit_node) else {panic!()}; cfg_outputs @@ -157,14 +157,14 @@ impl Rewrite for OutlineCfg { for n in all_blocks { // Do not move the entry node, as we have already; // Also don't move the exit_node if it's the exit-block of the outer CFG - if n == self.entry_node || (n == self.exit_node && cfg_succ.is_none()) { + if n == self.entry_node || (n == self.exit_node && exit_succ.is_none()) { continue; }; h.hierarchy.detach(n.index); h.hierarchy.insert_after(n.index, inner_exit.index).unwrap(); } // Connect new_block to (old) successor of exit block - if let Some(s) = cfg_succ { + if let Some(s) = exit_succ { let exit_port = h .node_outputs(self.exit_node) .filter(|p| { @@ -191,7 +191,7 @@ impl Rewrite for OutlineCfg { h.connect(src_n, src_p.index(), inner_exit, 0).unwrap(); } } - h.connect(new_block, 0, cfg_succ.unwrap_or(self.exit_node), 0) + h.connect(new_block, 0, exit_succ.unwrap_or(self.exit_node), 0) .unwrap(); Ok(()) } From 0fd40c9221369ff2704cb51792b19a56ba25e3ae Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 14:44:53 +0100 Subject: [PATCH 11/41] Try moving the exit_block into the inner CFG. Not actually easier... --- src/hugr/rewrite/outline_cfg.rs | 111 +++++++++++++++++--------------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index c13a2a776..7349d374b 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -80,7 +80,7 @@ impl Rewrite for OutlineCfg { } fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { let (all_blocks, exit_succ) = self.compute_all_blocks(h)?; - // Gather data. + // 1. Compute signature // These panic()s only happen if the Hugr would not have passed validate() let inputs = h .get_optype( @@ -93,7 +93,7 @@ impl Rewrite for OutlineCfg { let OpType::BasicBlock(BasicBlock::Exit {cfg_outputs}) = h.get_optype(self.exit_node) else {panic!()}; cfg_outputs }, - Some(s) => { + Some(_) => { let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; match s_type { BasicBlock::Exit { cfg_outputs } => cfg_outputs, @@ -113,7 +113,7 @@ impl Rewrite for OutlineCfg { h.hierarchy .push_child(new_block.index, parent.index) .unwrap(); - // Change any edges into entry_block from outside, to target new_block + // 3. Entry edges. Change any edges into entry_block from outside, to target new_block let preds: Vec<_> = h .linked_ports( self.entry_node, @@ -126,7 +126,7 @@ impl Rewrite for OutlineCfg { h.connect(pred, br.index(), new_block, 0).unwrap(); } } - // 3. new_block contains input node, sub-cfg, exit node all connected + // 4. new_block contains input node, sub-cfg, exit node all connected let mut b = DFGBuilder::create_with_io( &mut *h, new_block, @@ -135,64 +135,69 @@ impl Rewrite for OutlineCfg { .unwrap(); let wires_in = inputs.into_iter().cloned().zip(b.input_wires()); let cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); - let cfg_index = cfg.container_node().index; + let cfg_node = cfg.container_node(); let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); b.finish_with_outputs(cfg_outputs).unwrap(); - // 4. Children of new CFG - entry node must be first + // 5. Children of new CFG, and exit edges + // Entry node must be first h.hierarchy.detach(self.entry_node.index); h.hierarchy - .push_child(self.entry_node.index, cfg_index) - .unwrap(); - // Make an exit node, next - let inner_exit = h - .add_op_after( - self.entry_node, - OpType::BasicBlock(BasicBlock::Exit { - cfg_outputs: outputs, - }), - ) + .push_child(self.entry_node.index, cfg_node.index) .unwrap(); - // Then reparent remainder + // Then find or make exit node. We'll need a new exit node *somewhere* + let new_exit = h.add_op(OpType::BasicBlock(BasicBlock::Exit { + cfg_outputs: outputs.clone(), + })); + let mut all_blocks = all_blocks.into_iter().collect::>(); + let cfg_succ = match exit_succ { + Some(s) => { + h.hierarchy + .push_child(new_exit.index, cfg_node.index) + .unwrap(); + // Retarget edge from exit_node (that used to target exit_succ) to inner_exit + let exit_port = h + .node_outputs(self.exit_node) + .filter(|p| { + let (t, p2) = h + .linked_ports(self.exit_node, *p) + .exactly_one() + .ok() + .unwrap(); + assert!(p2.index() == 0); + t == s + }) + .exactly_one() + .unwrap(); + h.disconnect(self.exit_node, exit_port).unwrap(); + h.connect(self.exit_node, exit_port.index(), new_exit, 0) + .unwrap(); + s + } + None => { + // Moving exit node - make sure it's first (i.e. next after entry node) + let (ex_i, _) = all_blocks + .iter() + .enumerate() + .find_or_first(|(_, n)| **n == self.exit_node) + .unwrap(); + all_blocks.swap(ex_i, 0); + // Outer CFG is about to lose its exit node, so the new one will be successor of new_block + h.hierarchy + .insert_after(new_exit.index, h.children(parent).next().unwrap().index) + .unwrap(); + new_exit + } + }; + h.connect(new_block, 0, cfg_succ, 0).unwrap(); + // Reparent remaining nodes for n in all_blocks { - // Do not move the entry node, as we have already; - // Also don't move the exit_node if it's the exit-block of the outer CFG - if n == self.entry_node || (n == self.exit_node && exit_succ.is_none()) { - continue; - }; - h.hierarchy.detach(n.index); - h.hierarchy.insert_after(n.index, inner_exit.index).unwrap(); - } - // Connect new_block to (old) successor of exit block - if let Some(s) = exit_succ { - let exit_port = h - .node_outputs(self.exit_node) - .filter(|p| { - let (t, p2) = h - .linked_ports(self.exit_node, *p) - .exactly_one() - .ok() - .unwrap(); - assert!(p2.index() == 0); - t == s - }) - .exactly_one() - .unwrap(); - h.disconnect(self.exit_node, exit_port).unwrap(); - h.connect(self.exit_node, exit_port.index(), inner_exit, 0) - .unwrap(); - } else { - // We left the exit block outside. So, it's *predecessors* need to be retargetted to the inner_exit. - let in_p = h.node_inputs(self.exit_node).exactly_one().unwrap(); - assert!(in_p.index() == 0); - let preds = h.linked_ports(self.exit_node, in_p).collect::>(); - for (src_n, src_p) in preds { - h.disconnect(src_n, src_p).unwrap(); - h.connect(src_n, src_p.index(), inner_exit, 0).unwrap(); + // Do not move the entry node, as we have already + if n != self.entry_node { + h.hierarchy.detach(n.index); + h.hierarchy.push_child(n.index, cfg_node.index).unwrap(); } } - h.connect(new_block, 0, exit_succ.unwrap_or(self.exit_node), 0) - .unwrap(); Ok(()) } } From 4ae2fc9cd6ceefe1a83732c508271f4f2bc8538e Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 15:10:37 +0100 Subject: [PATCH 12/41] Simplify: always require an exitting edge. InsertIdentity is enough to ensure this. --- specification/hugr.md | 8 +- src/hugr/rewrite/outline_cfg.rs | 130 ++++++++++++-------------------- 2 files changed, 54 insertions(+), 84 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index f71c6171f..74711441e 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1235,12 +1235,12 @@ CFG node child which has as its children the set of BasicBlock nodes originally specified. The set of basic blocks must satisfy constraints: * All (controlflow) edges whose target is in the set but whose source is outside, must have the same target -* There must be at most one edge leaving the set; specifically, - * *either* the set does not contain an Exit block, and there is exactly one edge leaving the set; - * *or* the set contains the Exit block, and there are not edges out of the set. +* There must be exactly one edge from a node in the set to a node outside it. Situations in which multiple nodes have edges leaving the set, can be converted to -this form by a combination of InsertIdentity operations and one Replace. +this form by a combination of InsertIdentity operations and one Replace. Similarly, +rather than movng the Exit block into the nested CFG, an identity node with a single +successor can be added just before, with that edge used as the exitting edge. ** Implementation Note ** The required form of set can be easily identified by two nodes: the unique entry node, and an exit node (which may be the Exit block of the diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 7349d374b..d7cde4cf5 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -6,7 +6,7 @@ use thiserror::Error; use crate::builder::{Container, DFGBuilder, Dataflow, DataflowSubContainer, SubContainer}; use crate::hugr::rewrite::Rewrite; use crate::hugr::{HugrMut, HugrView}; -use crate::ops::{BasicBlock, OpTrait, OpType}; +use crate::ops::{BasicBlock, OpType}; use crate::types::Signature; use crate::{type_row, Hugr, Node}; @@ -17,17 +17,13 @@ pub struct OutlineCfg { /// All edges "in" to this block will be redirected to the new block /// (excluding backedges from internal BBs) entry_node: Node, - /// Either the exit node of the parent CFG; or, another node, with - /// exactly one successor that is not reachable from the entry_node - /// (without going through the exit_node). + /// Unique node in the parent CFG that has a successor (indeed, exactly one) + /// that is not reachable from the entry_node (without going through this exit_node). exit_node: Node, } impl OutlineCfg { - fn compute_all_blocks( - &self, - h: &Hugr, - ) -> Result<(HashSet, Option), OutlineCfgError> { + fn compute_all_blocks(&self, h: &Hugr) -> Result<(HashSet, Node), OutlineCfgError> { let cfg_n = match (h.get_parent(self.entry_node), h.get_parent(self.exit_node)) { (Some(p1), Some(p2)) if p1 == p2 => p1, (p1, p2) => return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2)), @@ -50,12 +46,6 @@ impl OutlineCfg { if !all_blocks.contains(&self.exit_node) { return Err(OutlineCfgError::ExitNodeUnreachable); } - if matches!( - h.get_optype(self.exit_node), - OpType::BasicBlock(BasicBlock::Exit { .. }) - ) { - return Ok((all_blocks, None)); - }; let mut succ = None; for exit_succ in h.output_neighbours(self.exit_node) { if all_blocks.contains(&exit_succ) { @@ -66,8 +56,10 @@ impl OutlineCfg { } succ = Some(exit_succ); } - assert!(succ.is_some()); - Ok((all_blocks, succ)) + match succ { + None => Err(OutlineCfgError::NoExitSuccessors), + Some(s) => Ok((all_blocks, s)), + } } } @@ -82,23 +74,13 @@ impl Rewrite for OutlineCfg { let (all_blocks, exit_succ) = self.compute_all_blocks(h)?; // 1. Compute signature // These panic()s only happen if the Hugr would not have passed validate() - let inputs = h - .get_optype( - h.children(self.entry_node).next().unwrap(), // input node is first child - ) - .signature() - .output; - let outputs = match exit_succ { - None => { - let OpType::BasicBlock(BasicBlock::Exit {cfg_outputs}) = h.get_optype(self.exit_node) else {panic!()}; - cfg_outputs - }, - Some(_) => { - let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; - match s_type { - BasicBlock::Exit { cfg_outputs } => cfg_outputs, - BasicBlock::DFB { inputs, .. } => inputs, - } + let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(self.entry_node) else {panic!("Entry node is not a basic block")}; + let inputs = inputs.clone(); + let outputs = { + let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; + match s_type { + BasicBlock::Exit { cfg_outputs } => cfg_outputs, + BasicBlock::DFB { inputs, .. } => inputs, } } .clone(); @@ -139,58 +121,22 @@ impl Rewrite for OutlineCfg { let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); b.finish_with_outputs(cfg_outputs).unwrap(); - // 5. Children of new CFG, and exit edges + // 5. Children of new CFG. // Entry node must be first h.hierarchy.detach(self.entry_node.index); h.hierarchy .push_child(self.entry_node.index, cfg_node.index) .unwrap(); - // Then find or make exit node. We'll need a new exit node *somewhere* - let new_exit = h.add_op(OpType::BasicBlock(BasicBlock::Exit { - cfg_outputs: outputs.clone(), - })); - let mut all_blocks = all_blocks.into_iter().collect::>(); - let cfg_succ = match exit_succ { - Some(s) => { - h.hierarchy - .push_child(new_exit.index, cfg_node.index) - .unwrap(); - // Retarget edge from exit_node (that used to target exit_succ) to inner_exit - let exit_port = h - .node_outputs(self.exit_node) - .filter(|p| { - let (t, p2) = h - .linked_ports(self.exit_node, *p) - .exactly_one() - .ok() - .unwrap(); - assert!(p2.index() == 0); - t == s - }) - .exactly_one() - .unwrap(); - h.disconnect(self.exit_node, exit_port).unwrap(); - h.connect(self.exit_node, exit_port.index(), new_exit, 0) - .unwrap(); - s - } - None => { - // Moving exit node - make sure it's first (i.e. next after entry node) - let (ex_i, _) = all_blocks - .iter() - .enumerate() - .find_or_first(|(_, n)| **n == self.exit_node) - .unwrap(); - all_blocks.swap(ex_i, 0); - // Outer CFG is about to lose its exit node, so the new one will be successor of new_block - h.hierarchy - .insert_after(new_exit.index, h.children(parent).next().unwrap().index) - .unwrap(); - new_exit - } - }; - h.connect(new_block, 0, cfg_succ, 0).unwrap(); - // Reparent remaining nodes + // Then exit node + let inner_exit = h + .add_op_with_parent( + cfg_node, + OpType::BasicBlock(BasicBlock::Exit { + cfg_outputs: outputs.clone(), + }), + ) + .unwrap(); + // And remaining nodes for n in all_blocks { // Do not move the entry node, as we have already if n != self.entry_node { @@ -198,6 +144,28 @@ impl Rewrite for OutlineCfg { h.hierarchy.push_child(n.index, cfg_node.index).unwrap(); } } + + // 6. Exit edges. + // Retarget edge from exit_node (that used to target exit_succ) to inner_exit + let exit_port = h + .node_outputs(self.exit_node) + .filter(|p| { + let (t, p2) = h + .linked_ports(self.exit_node, *p) + .exactly_one() + .ok() + .unwrap(); + assert!(p2.index() == 0); + t == exit_succ + }) + .exactly_one() + .unwrap(); + h.disconnect(self.exit_node, exit_port).unwrap(); + h.connect(self.exit_node, exit_port.index(), inner_exit, 0) + .unwrap(); + // And connect new_block to exit_succ instead + h.connect(new_block, 0, exit_succ, 0).unwrap(); + Ok(()) } } @@ -206,6 +174,8 @@ impl Rewrite for OutlineCfg { pub enum OutlineCfgError { #[error("The exit node could not be reached from the entry node")] ExitNodeUnreachable, + #[error("Exit node does not exit - all its successors can be reached from the entry")] + NoExitSuccessors, #[error("Entry node {0:?} and exit node {1:?} are not siblings")] EntryExitNotSiblings(Option, Option), #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] From 59c2ce7f0afbe95cac6f6f246c32309c5ef20d3d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 15:11:43 +0100 Subject: [PATCH 13/41] clippy --- src/hugr/rewrite/outline_cfg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index d7cde4cf5..f5ea70c37 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -115,7 +115,7 @@ impl Rewrite for OutlineCfg { Signature::new_df(inputs.clone(), outputs.clone()), ) .unwrap(); - let wires_in = inputs.into_iter().cloned().zip(b.input_wires()); + let wires_in = inputs.iter().cloned().zip(b.input_wires()); let cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); let cfg_node = cfg.container_node(); let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); @@ -132,7 +132,7 @@ impl Rewrite for OutlineCfg { .add_op_with_parent( cfg_node, OpType::BasicBlock(BasicBlock::Exit { - cfg_outputs: outputs.clone(), + cfg_outputs: outputs, }), ) .unwrap(); From 010061f1a84171ba5b26dfc13af70896bd3e3f2d Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 15:21:13 +0100 Subject: [PATCH 14/41] Use matches! --- src/hugr/rewrite/outline_cfg.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index f5ea70c37..9eeedc6f9 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -28,11 +28,9 @@ impl OutlineCfg { (Some(p1), Some(p2)) if p1 == p2 => p1, (p1, p2) => return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2)), }; - match h.get_optype(cfg_n) { - OpType::CFG(_) => (), - o => { - return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone())); - } + let o = h.get_optype(cfg_n); + if !matches!(o, OpType::CFG(_)) { + return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone())); }; let mut all_blocks = HashSet::new(); let mut queue = VecDeque::new(); From 3528a657bb53b224d927865d9b436d920bee41d9 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 15:35:04 +0100 Subject: [PATCH 15/41] Use at_most_one rather than manual loop --- src/hugr/rewrite/outline_cfg.rs | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 9eeedc6f9..9187c9ed9 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -44,19 +44,14 @@ impl OutlineCfg { if !all_blocks.contains(&self.exit_node) { return Err(OutlineCfgError::ExitNodeUnreachable); } - let mut succ = None; - for exit_succ in h.output_neighbours(self.exit_node) { - if all_blocks.contains(&exit_succ) { - continue; - } - if let Some(s) = succ { - return Err(OutlineCfgError::MultipleExitSuccessors(s, exit_succ)); - } - succ = Some(exit_succ); - } - match succ { - None => Err(OutlineCfgError::NoExitSuccessors), - Some(s) => Ok((all_blocks, s)), + let exit_succs = h + .output_neighbours(self.exit_node) + .filter(|n| !all_blocks.contains(n)) + .collect::>(); + match exit_succs.into_iter().at_most_one() { + Err(e) => Err(OutlineCfgError::MultipleExitSuccessors(Box::new(e))), + Ok(None) => Err(OutlineCfgError::NoExitSuccessors), + Ok(Some(exit_succ)) => Ok((all_blocks, exit_succ)), } } } @@ -178,6 +173,6 @@ pub enum OutlineCfgError { EntryExitNotSiblings(Option, Option), #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] ParentNotCfg(Node, OpType), - #[error("Exit node had multiple successors outside CFG - at least {0:?} and {1:?}")] - MultipleExitSuccessors(Node, Node), + #[error("Exit node had multiple successors outside CFG: {0}")] + MultipleExitSuccessors(#[source] Box), } From 9de20df624f78031d9ad02ef91c5800ffd54dccb Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Sun, 25 Jun 2023 15:38:46 +0100 Subject: [PATCH 16/41] ws --- src/hugr/rewrite/outline_cfg.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 9187c9ed9..008515021 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -88,6 +88,7 @@ impl Rewrite for OutlineCfg { h.hierarchy .push_child(new_block.index, parent.index) .unwrap(); + // 3. Entry edges. Change any edges into entry_block from outside, to target new_block let preds: Vec<_> = h .linked_ports( @@ -101,6 +102,7 @@ impl Rewrite for OutlineCfg { h.connect(pred, br.index(), new_block, 0).unwrap(); } } + // 4. new_block contains input node, sub-cfg, exit node all connected let mut b = DFGBuilder::create_with_io( &mut *h, From a7940f3e12e78f3ae41bbe2571be42b279b4d796 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 13:00:36 +0100 Subject: [PATCH 17/41] CFGBuilder: add from_existing --- src/builder/cfg.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/builder/cfg.rs b/src/builder/cfg.rs index 5279b5a3a..63ffac87e 100644 --- a/src/builder/cfg.rs +++ b/src/builder/cfg.rs @@ -1,3 +1,5 @@ +use itertools::Itertools; + use super::{ build_traits::SubContainer, dataflow::{DFGBuilder, DFGWrapper}, @@ -95,6 +97,20 @@ impl + AsRef> CFGBuilder { inputs: Some(input), }) } + pub(crate) fn from_existing(base: B, cfg_node: Node) -> Result { + 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, + 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`. From e9c3c12be47d9f03423ee77497f87770b5062218 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 13:16:48 +0100 Subject: [PATCH 18/41] Re-hide create_with_io; use CFGBuilder::from_existing --- src/builder/dataflow.rs | 2 +- src/hugr/rewrite/outline_cfg.rs | 55 +++++++++++++++++---------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/builder/dataflow.rs b/src/builder/dataflow.rs index 1ceda8f23..332651492 100644 --- a/src/builder/dataflow.rs +++ b/src/builder/dataflow.rs @@ -22,7 +22,7 @@ pub struct DFGBuilder { } impl + AsRef> DFGBuilder { - pub(crate) fn create_with_io( + pub(super) fn create_with_io( mut base: T, parent: Node, signature: Signature, diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 008515021..a8d47baef 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -3,10 +3,13 @@ use std::collections::{HashSet, VecDeque}; use itertools::Itertools; use thiserror::Error; -use crate::builder::{Container, DFGBuilder, Dataflow, DataflowSubContainer, SubContainer}; +use crate::builder::{ + BlockBuilder, CFGBuilder, Container, DFGBuilder, Dataflow, DataflowSubContainer, SubContainer, +}; use crate::hugr::rewrite::Rewrite; use crate::hugr::{HugrMut, HugrView}; -use crate::ops::{BasicBlock, OpType}; +use crate::ops::handle::NodeHandle; +use crate::ops::{BasicBlock, ConstValue, OpType}; use crate::types::Signature; use crate::{type_row, Hugr, Node}; @@ -77,19 +80,32 @@ impl Rewrite for OutlineCfg { } } .clone(); - let parent = h.get_parent(self.entry_node).unwrap(); + let mut existing_cfg = { + let parent = h.get_parent(self.entry_node).unwrap(); + CFGBuilder::from_existing(h, parent).unwrap() + }; // 2. New CFG node will be contained in new single-successor BB - let new_block = h.add_op(BasicBlock::DFB { - inputs: inputs.clone(), - other_outputs: outputs.clone(), - predicate_variants: vec![type_row![]], - }); - h.hierarchy - .push_child(new_block.index, parent.index) + let mut new_block = existing_cfg + .block_builder(inputs.clone(), vec![type_row![]], outputs.clone()) .unwrap(); - // 3. Entry edges. Change any edges into entry_block from outside, to target new_block + // 3. new_block contains input node, sub-cfg, exit node all connected + let wires_in = inputs.iter().cloned().zip(new_block.input_wires()); + let cfg = new_block.cfg_builder(wires_in, outputs.clone()).unwrap(); + let cfg_node = cfg.container_node(); + let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); + let predicate = new_block + .add_constant(ConstValue::simple_predicate(0, 1)) + .unwrap(); + let pred_wire = new_block.load_const(&predicate).unwrap(); + let new_block = new_block + .finish_with_outputs(pred_wire, cfg_outputs) + .unwrap(); + + // 4. Entry edges. Change any edges into entry_block from outside, to target new_block + let h = existing_cfg.hugr_mut(); + let preds: Vec<_> = h .linked_ports( self.entry_node, @@ -99,23 +115,10 @@ impl Rewrite for OutlineCfg { for (pred, br) in preds { if !all_blocks.contains(&pred) { h.disconnect(pred, br).unwrap(); - h.connect(pred, br.index(), new_block, 0).unwrap(); + h.connect(pred, br.index(), new_block.node(), 0).unwrap(); } } - // 4. new_block contains input node, sub-cfg, exit node all connected - let mut b = DFGBuilder::create_with_io( - &mut *h, - new_block, - Signature::new_df(inputs.clone(), outputs.clone()), - ) - .unwrap(); - let wires_in = inputs.iter().cloned().zip(b.input_wires()); - let cfg = b.cfg_builder(wires_in, outputs.clone()).unwrap(); - let cfg_node = cfg.container_node(); - let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); - b.finish_with_outputs(cfg_outputs).unwrap(); - // 5. Children of new CFG. // Entry node must be first h.hierarchy.detach(self.entry_node.index); @@ -159,7 +162,7 @@ impl Rewrite for OutlineCfg { h.connect(self.exit_node, exit_port.index(), inner_exit, 0) .unwrap(); // And connect new_block to exit_succ instead - h.connect(new_block, 0, exit_succ, 0).unwrap(); + h.connect(new_block.node(), 0, exit_succ, 0).unwrap(); Ok(()) } From df4f1d7f8337860bc27bd1728c4fc7cf10088db1 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 13:26:25 +0100 Subject: [PATCH 19/41] Fix entry/exit of CFG node --- src/hugr/rewrite/outline_cfg.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index a8d47baef..5022a557f 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -3,14 +3,11 @@ use std::collections::{HashSet, VecDeque}; use itertools::Itertools; use thiserror::Error; -use crate::builder::{ - BlockBuilder, CFGBuilder, Container, DFGBuilder, Dataflow, DataflowSubContainer, SubContainer, -}; +use crate::builder::{CFGBuilder, Container, Dataflow, SubContainer}; use crate::hugr::rewrite::Rewrite; use crate::hugr::{HugrMut, HugrView}; use crate::ops::handle::NodeHandle; use crate::ops::{BasicBlock, ConstValue, OpType}; -use crate::types::Signature; use crate::{type_row, Hugr, Node}; /// Moves part of a Control-flow Sibling Graph into a new CFG-node @@ -94,6 +91,7 @@ impl Rewrite for OutlineCfg { let wires_in = inputs.iter().cloned().zip(new_block.input_wires()); let cfg = new_block.cfg_builder(wires_in, outputs.clone()).unwrap(); let cfg_node = cfg.container_node(); + let inner_exit = cfg.exit_block().node(); let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); let predicate = new_block .add_constant(ConstValue::simple_predicate(0, 1)) @@ -123,16 +121,7 @@ impl Rewrite for OutlineCfg { // Entry node must be first h.hierarchy.detach(self.entry_node.index); h.hierarchy - .push_child(self.entry_node.index, cfg_node.index) - .unwrap(); - // Then exit node - let inner_exit = h - .add_op_with_parent( - cfg_node, - OpType::BasicBlock(BasicBlock::Exit { - cfg_outputs: outputs, - }), - ) + .insert_before(self.entry_node.index, inner_exit.index) .unwrap(); // And remaining nodes for n in all_blocks { From 85a05ad422a00fa15e5b5147c473aa1e015a19c2 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 13:26:45 +0100 Subject: [PATCH 20/41] nest_cfgs: update test, easiest to get split/merge using (in/out)put_neighbours --- src/algorithm/nest_cfgs.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/algorithm/nest_cfgs.rs b/src/algorithm/nest_cfgs.rs index ac1cae40d..411553f44 100644 --- a/src/algorithm/nest_cfgs.rs +++ b/src/algorithm/nest_cfgs.rs @@ -513,23 +513,14 @@ pub(crate) mod test { // entry -> head -> split > merge -> tail -> exit // | \-> right -/ | // \---<---<---<---<---<---<---<---<---/ + // split is unique successor of head + let split = h.output_neighbours(head).exactly_one().unwrap(); + // merge is unique predecessor of tail + let merge = h.input_neighbours(tail).exactly_one().unwrap(); + let v = SimpleCfgView::new(&h); let edge_classes = EdgeClassifier::get_edge_classes(&v); let SimpleCfgView { h: _, entry, exit } = v; - // split is unique successor of head - let split = *edge_classes - .keys() - .filter(|(s, _)| *s == head) - .map(|(_, t)| t) - .exactly_one() - .unwrap(); - // merge is unique predecessor of tail - let merge = *edge_classes - .keys() - .filter(|(_, t)| *t == tail) - .map(|(s, _)| s) - .exactly_one() - .unwrap(); let [&left,&right] = edge_classes.keys().filter(|(s,_)| *s == split).map(|(_,t)|t).collect::>()[..] else {panic!("Split node should have two successors");}; let classes = group_by(edge_classes); assert_eq!( From e90de4d3690a23e8674ebdc4e25a0ce0c08e29ac Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 13:27:14 +0100 Subject: [PATCH 21/41] Add first test of outline_cfg working! --- src/hugr/rewrite/outline_cfg.rs | 82 +++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 5022a557f..cc5ddc0bc 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -170,3 +170,85 @@ pub enum OutlineCfgError { #[error("Exit node had multiple successors outside CFG: {0}")] MultipleExitSuccessors(#[source] Box), } + +#[cfg(test)] +mod test { + use crate::algorithm::nest_cfgs::test::build_conditional_in_loop_cfg; + use crate::ops::handle::NodeHandle; + use crate::{HugrView, Node}; + use cool_asserts::assert_matches; + use itertools::Itertools; + + use super::{OutlineCfg, OutlineCfgError}; + + fn depth(h: &impl HugrView, n: Node) -> u32 { + match h.get_parent(n) { + Some(p) => 1 + depth(h, p), + None => 0, + } + } + + // TODO test cond_then_loop_separate (head, tail) keeps the backedge in the sub-cfg + // (== merge -> sub -> exit, hmm, maybe not that hard) + + #[test] + fn test_outline_cfg_errors() { + let (mut h, head, tail) = build_conditional_in_loop_cfg(false).unwrap(); + let head = head.node(); + let tail = tail.node(); + // /-> left --\ + // entry -> head > merge -> tail -> exit + // | \-> right -/ | + // \---<---<---<---<---<--<---/ + // merge is unique predecessor of tail + let merge = h.input_neighbours(tail).exactly_one().unwrap(); + h.validate().unwrap(); + let backup = h.clone(); + let r = h.apply_rewrite(OutlineCfg { + entry_node: merge, + exit_node: tail, + }); + assert_matches!(r, Err(OutlineCfgError::MultipleExitSuccessors(_))); + assert_eq!(h, backup); + let r = h.apply_rewrite(OutlineCfg { + entry_node: head, + exit_node: h.children(h.root()).next().unwrap(), + }); + assert_matches!(r, Err(OutlineCfgError::ExitNodeUnreachable)); + assert_eq!(h, backup); + // Here all the edges from the exit node target a node (tail) in the region + let r = h.apply_rewrite(OutlineCfg { + entry_node: tail, + exit_node: merge, + }); + // All the edges from the exit node target a node (tail) in the region + assert_matches!(r, Err(OutlineCfgError::NoExitSuccessors)); + assert_eq!(h, backup); + } + + #[test] + fn test_outline_cfg() { + let (mut h, head, tail) = build_conditional_in_loop_cfg(false).unwrap(); + let head = head.node(); + let tail = tail.node(); + // /-> left --\ + // entry -> head > merge -> tail -> exit + // | \-> right -/ | + // \---<---<---<---<---<--<---/ + // merge is unique predecessor of tail + let merge = h.input_neighbours(tail).exactly_one().unwrap(); + for n in [head, tail, merge] { + assert_eq!(depth(&h, n), 1); + } + h.validate().unwrap(); + h.apply_rewrite(OutlineCfg { + entry_node: head, + exit_node: merge, + }) + .unwrap(); + h.validate().unwrap(); + assert_eq!(depth(&h, head), 3); + assert_eq!(depth(&h, merge), 3); + assert_eq!(depth(&h, tail), 1); + } +} From 7572924cc7611c1770a468edcc025473a377dc51 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 15:28:47 +0100 Subject: [PATCH 22/41] Specify set of blocks not entry/exit --- src/hugr/rewrite/outline_cfg.rs | 211 +++++++++++++++++--------------- 1 file changed, 115 insertions(+), 96 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index cc5ddc0bc..4eec5079f 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -1,4 +1,5 @@ -use std::collections::{HashSet, VecDeque}; +//! Rewrite for inserting a CFG-node into the hierarchy containing a subsection of an existing CFG +use std::collections::HashSet; use itertools::Itertools; use thiserror::Error; @@ -12,46 +13,60 @@ use crate::{type_row, Hugr, Node}; /// Moves part of a Control-flow Sibling Graph into a new CFG-node /// that is the only child of a new Basic Block in the original CSG. -pub struct OutlineCfg { - /// Will become the entry block in the new sub-cfg. - /// All edges "in" to this block will be redirected to the new block - /// (excluding backedges from internal BBs) - entry_node: Node, - /// Unique node in the parent CFG that has a successor (indeed, exactly one) - /// that is not reachable from the entry_node (without going through this exit_node). - exit_node: Node, -} +pub struct OutlineCfg(HashSet); impl OutlineCfg { - fn compute_all_blocks(&self, h: &Hugr) -> Result<(HashSet, Node), OutlineCfgError> { - let cfg_n = match (h.get_parent(self.entry_node), h.get_parent(self.exit_node)) { - (Some(p1), Some(p2)) if p1 == p2 => p1, - (p1, p2) => return Err(OutlineCfgError::EntryExitNotSiblings(p1, p2)), + /// Create a new OutlineCfg rewrite that will move the provided blocks. + pub fn new(blocks: impl IntoIterator) -> Self { + Self(HashSet::from_iter(blocks)) + } + + fn compute_entry_exit_outside(&self, h: &Hugr) -> Result<(Node, Node, Node), OutlineCfgError> { + let cfg_n = match self + .0 + .iter() + .map(|n| h.get_parent(*n)) + .unique() + .exactly_one() + { + Ok(Some(n)) => n, + _ => return Err(OutlineCfgError::NotSiblings), }; let o = h.get_optype(cfg_n); if !matches!(o, OpType::CFG(_)) { return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone())); }; - let mut all_blocks = HashSet::new(); - let mut queue = VecDeque::new(); - queue.push_back(self.entry_node); - while let Some(n) = queue.pop_front() { - // This puts the exit_node into 'all_blocks' but not its successors - if all_blocks.insert(n) && n != self.exit_node { - queue.extend(h.output_neighbours(n)); + let mut entry = None; + let mut exit_succ = None; + for &n in self.0.iter() { + if h.input_neighbours(n).any(|pred| !self.0.contains(&pred)) { + match entry { + None => { + entry = Some(n); + } + Some(prev) => { + return Err(OutlineCfgError::MultipleEntryNodes(prev, n)); + } + } } + let external = h.output_neighbours(n).filter(|s| !self.0.contains(&s)); + match external.at_most_one() { + Ok(None) => (), // No external successors + Ok(Some(o)) => match exit_succ { + None => { + exit_succ = Some((n, o)); + } + Some((prev, _)) => { + return Err(OutlineCfgError::MultipleExitNodes(prev, n)); + } + }, + Err(ext) => return Err(OutlineCfgError::MultipleExitEdges(n, ext.collect())), + }; } - if !all_blocks.contains(&self.exit_node) { - return Err(OutlineCfgError::ExitNodeUnreachable); - } - let exit_succs = h - .output_neighbours(self.exit_node) - .filter(|n| !all_blocks.contains(n)) - .collect::>(); - match exit_succs.into_iter().at_most_one() { - Err(e) => Err(OutlineCfgError::MultipleExitSuccessors(Box::new(e))), - Ok(None) => Err(OutlineCfgError::NoExitSuccessors), - Ok(Some(exit_succ)) => Ok((all_blocks, exit_succ)), + match (entry, exit_succ) { + (Some(e), Some((x, o))) => Ok((e, x, o)), + (None, _) => Err(OutlineCfgError::NoEntryNode), + (_, None) => Err(OutlineCfgError::NoExitNode), } } } @@ -60,17 +75,17 @@ impl Rewrite for OutlineCfg { type Error = OutlineCfgError; const UNCHANGED_ON_FAILURE: bool = true; fn verify(&self, h: &Hugr) -> Result<(), OutlineCfgError> { - self.compute_all_blocks(h)?; + self.compute_entry_exit_outside(h)?; Ok(()) } fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { - let (all_blocks, exit_succ) = self.compute_all_blocks(h)?; + let (entry, exit, outside) = self.compute_entry_exit_outside(h)?; // 1. Compute signature // These panic()s only happen if the Hugr would not have passed validate() - let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(self.entry_node) else {panic!("Entry node is not a basic block")}; + let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(entry) else {panic!("Entry node is not a basic block")}; let inputs = inputs.clone(); let outputs = { - let OpType::BasicBlock(s_type) = h.get_optype(self.exit_node) else {panic!()}; + let OpType::BasicBlock(s_type) = h.get_optype(exit) else {panic!()}; match s_type { BasicBlock::Exit { cfg_outputs } => cfg_outputs, BasicBlock::DFB { inputs, .. } => inputs, @@ -78,7 +93,7 @@ impl Rewrite for OutlineCfg { } .clone(); let mut existing_cfg = { - let parent = h.get_parent(self.entry_node).unwrap(); + let parent = h.get_parent(entry).unwrap(); CFGBuilder::from_existing(h, parent).unwrap() }; @@ -105,13 +120,10 @@ impl Rewrite for OutlineCfg { let h = existing_cfg.hugr_mut(); let preds: Vec<_> = h - .linked_ports( - self.entry_node, - h.node_inputs(self.entry_node).exactly_one().unwrap(), - ) + .linked_ports(entry, h.node_inputs(entry).exactly_one().unwrap()) .collect(); for (pred, br) in preds { - if !all_blocks.contains(&pred) { + if !self.0.contains(&pred) { h.disconnect(pred, br).unwrap(); h.connect(pred, br.index(), new_block.node(), 0).unwrap(); } @@ -119,60 +131,69 @@ impl Rewrite for OutlineCfg { // 5. Children of new CFG. // Entry node must be first - h.hierarchy.detach(self.entry_node.index); + h.hierarchy.detach(entry.index); h.hierarchy - .insert_before(self.entry_node.index, inner_exit.index) + .insert_before(entry.index, inner_exit.index) .unwrap(); // And remaining nodes - for n in all_blocks { + for n in self.0 { // Do not move the entry node, as we have already - if n != self.entry_node { + if n != entry { h.hierarchy.detach(n.index); h.hierarchy.push_child(n.index, cfg_node.index).unwrap(); } } // 6. Exit edges. - // Retarget edge from exit_node (that used to target exit_succ) to inner_exit + // Retarget edge from exit_node (that used to target outside) to inner_exit let exit_port = h - .node_outputs(self.exit_node) + .node_outputs(exit) .filter(|p| { - let (t, p2) = h - .linked_ports(self.exit_node, *p) - .exactly_one() - .ok() - .unwrap(); + let (t, p2) = h.linked_ports(exit, *p).exactly_one().ok().unwrap(); assert!(p2.index() == 0); - t == exit_succ + t == outside }) .exactly_one() .unwrap(); - h.disconnect(self.exit_node, exit_port).unwrap(); - h.connect(self.exit_node, exit_port.index(), inner_exit, 0) - .unwrap(); - // And connect new_block to exit_succ instead - h.connect(new_block.node(), 0, exit_succ, 0).unwrap(); + h.disconnect(exit, exit_port).unwrap(); + h.connect(exit, exit_port.index(), inner_exit, 0).unwrap(); + // And connect new_block to outside instead + h.connect(new_block.node(), 0, outside, 0).unwrap(); Ok(()) } } +/// Errors that can occur in expressing an OutlineCfg rewrite. #[derive(Debug, Error)] pub enum OutlineCfgError { - #[error("The exit node could not be reached from the entry node")] - ExitNodeUnreachable, - #[error("Exit node does not exit - all its successors can be reached from the entry")] - NoExitSuccessors, - #[error("Entry node {0:?} and exit node {1:?} are not siblings")] - EntryExitNotSiblings(Option, Option), - #[error("The parent node {0:?} of entry and exit was not a CFG but an {1:?}")] + /// The set of blocks were not siblings + #[error("The nodes did not all have the same parent")] + NotSiblings, + /// The parent node was not a CFG node + #[error("The parent node {0:?} was not a CFG but an {1:?}")] ParentNotCfg(Node, OpType), - #[error("Exit node had multiple successors outside CFG: {0}")] - MultipleExitSuccessors(#[source] Box), + /// Multiple blocks had incoming edges + #[error("Multiple blocks had predecessors outside the set - at least {0:?} and {1:?}")] + MultipleEntryNodes(Node, Node), + /// Multiple blocks had outgoing edegs + #[error("Multiple blocks had edges leaving the set - at least {0:?} and {1:?}")] + MultipleExitNodes(Node, Node), + /// One block had multiple outgoing edges + #[error("Exit block {0:?} had edges to multiple external blocks {1:?}")] + MultipleExitEdges(Node, Vec), + /// No block was identified as an entry block + #[error("No block had predecessors outside the set")] + NoEntryNode, + /// No block was identified as an exit block + #[error("No block had a successor outside the set")] + NoExitNode, } #[cfg(test)] mod test { + use std::collections::HashSet; + use crate::algorithm::nest_cfgs::test::build_conditional_in_loop_cfg; use crate::ops::handle::NodeHandle; use crate::{HugrView, Node}; @@ -188,9 +209,6 @@ mod test { } } - // TODO test cond_then_loop_separate (head, tail) keeps the backedge in the sub-cfg - // (== merge -> sub -> exit, hmm, maybe not that hard) - #[test] fn test_outline_cfg_errors() { let (mut h, head, tail) = build_conditional_in_loop_cfg(false).unwrap(); @@ -204,25 +222,17 @@ mod test { let merge = h.input_neighbours(tail).exactly_one().unwrap(); h.validate().unwrap(); let backup = h.clone(); - let r = h.apply_rewrite(OutlineCfg { - entry_node: merge, - exit_node: tail, - }); - assert_matches!(r, Err(OutlineCfgError::MultipleExitSuccessors(_))); + let r = h.apply_rewrite(OutlineCfg::new([merge, tail])); + assert_matches!(r, Err(OutlineCfgError::MultipleExitEdges(_, _))); assert_eq!(h, backup); - let r = h.apply_rewrite(OutlineCfg { - entry_node: head, - exit_node: h.children(h.root()).next().unwrap(), - }); - assert_matches!(r, Err(OutlineCfgError::ExitNodeUnreachable)); + + let [left, right]: [Node; 2] = h.output_neighbours(head).collect_vec().try_into().unwrap(); + let r = h.apply_rewrite(OutlineCfg::new([left, right, head])); + assert_matches!(r, Err(OutlineCfgError::MultipleExitNodes(a,b)) => HashSet::from([a,b]) == HashSet::from_iter([left, right, head])); assert_eq!(h, backup); - // Here all the edges from the exit node target a node (tail) in the region - let r = h.apply_rewrite(OutlineCfg { - entry_node: tail, - exit_node: merge, - }); - // All the edges from the exit node target a node (tail) in the region - assert_matches!(r, Err(OutlineCfgError::NoExitSuccessors)); + + let r = h.apply_rewrite(OutlineCfg::new([left, right, merge])); + assert_matches!(r, Err(OutlineCfgError::MultipleEntryNodes(a,b)) => HashSet::from([a,b]) == HashSet::from([left, right])); assert_eq!(h, backup); } @@ -231,24 +241,33 @@ mod test { let (mut h, head, tail) = build_conditional_in_loop_cfg(false).unwrap(); let head = head.node(); let tail = tail.node(); + let parent = h.get_parent(head).unwrap(); + let [entry, exit]: [Node; 2] = h.children(parent).take(2).collect_vec().try_into().unwrap(); // /-> left --\ // entry -> head > merge -> tail -> exit // | \-> right -/ | // \---<---<---<---<---<--<---/ // merge is unique predecessor of tail let merge = h.input_neighbours(tail).exactly_one().unwrap(); + let [left, right]: [Node; 2] = h.output_neighbours(head).collect_vec().try_into().unwrap(); for n in [head, tail, merge] { assert_eq!(depth(&h, n), 1); } h.validate().unwrap(); - h.apply_rewrite(OutlineCfg { - entry_node: head, - exit_node: merge, - }) - .unwrap(); + let blocks = [head, left, right, merge]; + h.apply_rewrite(OutlineCfg::new(blocks)).unwrap(); h.validate().unwrap(); - assert_eq!(depth(&h, head), 3); - assert_eq!(depth(&h, merge), 3); - assert_eq!(depth(&h, tail), 1); + for n in blocks { + assert_eq!(depth(&h, n), 3); + } + let new_block = h.output_neighbours(entry).exactly_one().unwrap(); + for n in [entry, exit, tail, new_block] { + assert_eq!(depth(&h, n), 1); + } + assert_eq!(h.input_neighbours(tail).exactly_one().unwrap(), new_block); + assert_eq!( + h.output_neighbours(tail).take(2).collect::>(), + HashSet::from([exit, new_block]) + ); } } From 6b38017945db08a9f5039ab5033dd01caa22b722 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 15:31:08 +0100 Subject: [PATCH 23/41] And update spec, noting deficiency --- specification/hugr.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index 5b78392e5..00686c0e6 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1238,9 +1238,7 @@ this form by a combination of InsertIdentity operations and one Replace. Similar rather than movng the Exit block into the nested CFG, an identity node with a single successor can be added just before, with that edge used as the exitting edge. -** Implementation Note ** The required form of set can be easily identified by two -nodes: the unique entry node, and an exit node (which may be the Exit block of the -CFG, or the source of the unique exit edge). +** Note ** this does not allow moving the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. ##### Inlining methods From eea616c269ba7df0fbd1de50f0cbdf5313845ced Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 15:35:29 +0100 Subject: [PATCH 24/41] Further clarify spec --- specification/hugr.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index 00686c0e6..f71de2111 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1229,8 +1229,9 @@ nodes as children. Replace a set of CFG sibling nodes with a single BasicBlock node having a CFG node child which has as its children the set of BasicBlock nodes originally specified. The set of basic blocks must satisfy constraints: -* All (controlflow) edges whose target is in the set but whose source is outside, - must have the same target +* There must be exactly one node in the set with incoming (controlflow) edges + from nodes outside the set. +* The set may not include the entry block * There must be exactly one edge from a node in the set to a node outside it. Situations in which multiple nodes have edges leaving the set, can be converted to @@ -1238,7 +1239,7 @@ this form by a combination of InsertIdentity operations and one Replace. Similar rather than movng the Exit block into the nested CFG, an identity node with a single successor can be added just before, with that edge used as the exitting edge. -** Note ** this does not allow moving the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. +** Note ** This means there is no way to move the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. ##### Inlining methods From cc446305de3f55cdc04e3806be83f9718ce7f9a2 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Mon, 3 Jul 2023 15:35:46 +0100 Subject: [PATCH 25/41] clippy --- src/hugr/rewrite/outline_cfg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 4eec5079f..90e83ef12 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -49,7 +49,7 @@ impl OutlineCfg { } } } - let external = h.output_neighbours(n).filter(|s| !self.0.contains(&s)); + let external = h.output_neighbours(n).filter(|s| !self.0.contains(s)); match external.at_most_one() { Ok(None) => (), // No external successors Ok(Some(o)) => match exit_succ { @@ -104,7 +104,7 @@ impl Rewrite for OutlineCfg { // 3. new_block contains input node, sub-cfg, exit node all connected let wires_in = inputs.iter().cloned().zip(new_block.input_wires()); - let cfg = new_block.cfg_builder(wires_in, outputs.clone()).unwrap(); + let cfg = new_block.cfg_builder(wires_in, outputs).unwrap(); let cfg_node = cfg.container_node(); let inner_exit = cfg.exit_block().node(); let cfg_outputs = cfg.finish_sub_container().unwrap().outputs(); From 6cbba9354ed322b8e2e4762a7e1788f785f233de Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 14:59:38 +0100 Subject: [PATCH 26/41] [Refactor] Rename external => external_succs --- src/hugr/rewrite/outline_cfg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 90e83ef12..1703f3ac2 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -49,8 +49,8 @@ impl OutlineCfg { } } } - let external = h.output_neighbours(n).filter(|s| !self.0.contains(s)); - match external.at_most_one() { + let external_succs = h.output_neighbours(n).filter(|s| !self.0.contains(s)); + match external_succs.at_most_one() { Ok(None) => (), // No external successors Ok(Some(o)) => match exit_succ { None => { From 5f4062b46290d5564cea8dc2f9b3b600f8b132d7 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 15:02:20 +0100 Subject: [PATCH 27/41] Region exit node is NOT the CFG exit, so simplify signature computation --- src/hugr/rewrite/outline_cfg.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 1703f3ac2..010c372b0 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -84,14 +84,10 @@ impl Rewrite for OutlineCfg { // These panic()s only happen if the Hugr would not have passed validate() let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(entry) else {panic!("Entry node is not a basic block")}; let inputs = inputs.clone(); - let outputs = { - let OpType::BasicBlock(s_type) = h.get_optype(exit) else {panic!()}; - match s_type { - BasicBlock::Exit { cfg_outputs } => cfg_outputs, - BasicBlock::DFB { inputs, .. } => inputs, - } - } - .clone(); + let outputs = match h.get_optype(outside) { + OpType::BasicBlock(b) => b.dataflow_input().clone(), + _ => panic!("External successor not a basic block"), + }; let mut existing_cfg = { let parent = h.get_parent(entry).unwrap(); CFGBuilder::from_existing(h, parent).unwrap() From 4512bf708ef6bf409047011ef1671cd5d7bbf548 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 15:03:29 +0100 Subject: [PATCH 28/41] Whitespace --- src/hugr/rewrite/outline_cfg.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 010c372b0..dd84b1a44 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -82,18 +82,19 @@ impl Rewrite for OutlineCfg { let (entry, exit, outside) = self.compute_entry_exit_outside(h)?; // 1. Compute signature // These panic()s only happen if the Hugr would not have passed validate() - let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(entry) else {panic!("Entry node is not a basic block")}; + let OpType::BasicBlock(BasicBlock::DFB {inputs, ..}) = h.get_optype(entry) + else {panic!("Entry node is not a basic block")}; let inputs = inputs.clone(); let outputs = match h.get_optype(outside) { OpType::BasicBlock(b) => b.dataflow_input().clone(), _ => panic!("External successor not a basic block"), }; + + // 2. New CFG node will be contained in new single-successor BB let mut existing_cfg = { let parent = h.get_parent(entry).unwrap(); CFGBuilder::from_existing(h, parent).unwrap() }; - - // 2. New CFG node will be contained in new single-successor BB let mut new_block = existing_cfg .block_builder(inputs.clone(), vec![type_row![]], outputs.clone()) .unwrap(); From ddf8fb62facc8a8294a2705c227ea3e5a0085419 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 15:18:03 +0100 Subject: [PATCH 29/41] Allow blocks to include CFG entry --- src/hugr/rewrite/outline_cfg.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index dd84b1a44..0c1e02aea 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -36,10 +36,11 @@ impl OutlineCfg { if !matches!(o, OpType::CFG(_)) { return Err(OutlineCfgError::ParentNotCfg(cfg_n, o.clone())); }; + let cfg_entry = h.children(cfg_n).next().unwrap(); let mut entry = None; let mut exit_succ = None; for &n in self.0.iter() { - if h.input_neighbours(n).any(|pred| !self.0.contains(&pred)) { + if n == cfg_entry || h.input_neighbours(n).any(|pred| !self.0.contains(&pred)) { match entry { None => { entry = Some(n); @@ -89,6 +90,7 @@ impl Rewrite for OutlineCfg { OpType::BasicBlock(b) => b.dataflow_input().clone(), _ => panic!("External successor not a basic block"), }; + let is_outer_entry = h.children(h.get_parent(entry).unwrap()).next().unwrap() == entry; // 2. New CFG node will be contained in new single-successor BB let mut existing_cfg = { @@ -111,7 +113,8 @@ impl Rewrite for OutlineCfg { let pred_wire = new_block.load_const(&predicate).unwrap(); let new_block = new_block .finish_with_outputs(pred_wire, cfg_outputs) - .unwrap(); + .unwrap() + .node(); // 4. Entry edges. Change any edges into entry_block from outside, to target new_block let h = existing_cfg.hugr_mut(); @@ -122,9 +125,17 @@ impl Rewrite for OutlineCfg { for (pred, br) in preds { if !self.0.contains(&pred) { h.disconnect(pred, br).unwrap(); - h.connect(pred, br.index(), new_block.node(), 0).unwrap(); + h.connect(pred, br.index(), new_block, 0).unwrap(); } } + if is_outer_entry { + // new_block must be the entry node, i.e. first child, of the enclosing CFG + // (the current entry node will be reparented inside new_block below) + let parent = h.hierarchy.detach(new_block.index).unwrap(); + h.hierarchy + .push_front_child(new_block.index, parent) + .unwrap(); + } // 5. Children of new CFG. // Entry node must be first @@ -155,7 +166,7 @@ impl Rewrite for OutlineCfg { h.disconnect(exit, exit_port).unwrap(); h.connect(exit, exit_port.index(), inner_exit, 0).unwrap(); // And connect new_block to outside instead - h.connect(new_block.node(), 0, outside, 0).unwrap(); + h.connect(new_block, 0, outside, 0).unwrap(); Ok(()) } From 381dc8089b6d3f86ab350ef2aae82a70c758f401 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 15:59:52 +0100 Subject: [PATCH 30/41] nest_cfgs::test: Break out build_cond_then_loop_cfg --- src/algorithm/nest_cfgs.rs | 44 +++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/algorithm/nest_cfgs.rs b/src/algorithm/nest_cfgs.rs index 411553f44..5c77c5de1 100644 --- a/src/algorithm/nest_cfgs.rs +++ b/src/algorithm/nest_cfgs.rs @@ -471,22 +471,15 @@ pub(crate) mod test { // \-> right -/ \-<--<-/ // Here we would like two consecutive regions, but there is no *edge* between // the conditional and the loop to indicate the boundary, so we cannot separate them. - let mut cfg_builder = CFGBuilder::new(type_row![NAT], type_row![NAT])?; - let mut entry = cfg_builder.simple_entry_builder(type_row![NAT], 2)?; - let pred_const = entry.add_constant(ConstValue::simple_predicate(0, 2))?; // Nothing here cares which - let const_unit = entry.add_constant(ConstValue::simple_unary_predicate())?; - - let entry = n_identity(entry, &pred_const)?; - let merge = build_then_else_merge_from_if(&mut cfg_builder, &const_unit, entry)?; - let tail = build_loop_from_header(&mut cfg_builder, &pred_const, merge)?; - cfg_builder.branch(&merge, 0, &tail)?; // trivial "loop body" - let exit = cfg_builder.exit_block(); - cfg_builder.branch(&tail, 0, &exit)?; - - let h = cfg_builder.finish_hugr()?; - - let (entry, exit) = (entry.node(), exit.node()); + let (h, merge, tail) = build_cond_then_loop_cfg()?; let (merge, tail) = (merge.node(), tail.node()); + let [entry, exit]: [Node; 2] = h + .children(h.root()) + .take(2) + .collect_vec() + .try_into() + .unwrap(); + let edge_classes = EdgeClassifier::get_edge_classes(&SimpleCfgView::new(&h)); let [&left,&right] = edge_classes.keys().filter(|(s,_)| *s == entry).map(|(_,t)|t).collect::>()[..] else {panic!("Entry node should have two successors");}; @@ -646,6 +639,27 @@ pub(crate) mod test { Ok((header, tail)) } + // /-> left --\ + // entry > merge -> tail -> exit + // \-> right -/ \-<--<-/ + pub fn build_cond_then_loop_cfg( + ) -> Result<(Hugr, BasicBlockID, BasicBlockID), BuildError> { + let mut cfg_builder = CFGBuilder::new(type_row![NAT], type_row![NAT])?; + let mut entry = cfg_builder.simple_entry_builder(type_row![NAT], 2)?; + let pred_const = entry.add_constant(ConstValue::simple_predicate(0, 2))?; // Nothing here cares which + let const_unit = entry.add_constant(ConstValue::simple_unary_predicate())?; + + let entry = n_identity(entry, &pred_const)?; + let merge = build_then_else_merge_from_if(&mut cfg_builder, &const_unit, entry)?; + let tail = build_loop_from_header(&mut cfg_builder, &pred_const, merge)?; + cfg_builder.branch(&merge, 0, &tail)?; // trivial "loop body" + let exit = cfg_builder.exit_block(); + cfg_builder.branch(&tail, 0, &exit)?; + + let h = cfg_builder.finish_hugr()?; + Ok((h, merge, tail)) + } + // Build a CFG, returning the Hugr pub fn build_conditional_in_loop_cfg( separate_headers: bool, From 27463ae3640fc6f745405aa8683fca587ee99e55 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 16:22:37 +0100 Subject: [PATCH 31/41] build_cond_then_loop: add separate param (use false) --- src/algorithm/nest_cfgs.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/algorithm/nest_cfgs.rs b/src/algorithm/nest_cfgs.rs index 5c77c5de1..d86d3eefd 100644 --- a/src/algorithm/nest_cfgs.rs +++ b/src/algorithm/nest_cfgs.rs @@ -471,7 +471,7 @@ pub(crate) mod test { // \-> right -/ \-<--<-/ // Here we would like two consecutive regions, but there is no *edge* between // the conditional and the loop to indicate the boundary, so we cannot separate them. - let (h, merge, tail) = build_cond_then_loop_cfg()?; + let (h, merge, tail) = build_cond_then_loop_cfg(false)?; let (merge, tail) = (merge.node(), tail.node()); let [entry, exit]: [Node; 2] = h .children(h.root()) @@ -639,10 +639,9 @@ pub(crate) mod test { Ok((header, tail)) } - // /-> left --\ - // entry > merge -> tail -> exit - // \-> right -/ \-<--<-/ + // Result is merge and tail; loop header is (merge, if separate==true; unique successor of merge, if separate==false) pub fn build_cond_then_loop_cfg( + separate: bool, ) -> Result<(Hugr, BasicBlockID, BasicBlockID), BuildError> { let mut cfg_builder = CFGBuilder::new(type_row![NAT], type_row![NAT])?; let mut entry = cfg_builder.simple_entry_builder(type_row![NAT], 2)?; @@ -651,8 +650,18 @@ pub(crate) mod test { let entry = n_identity(entry, &pred_const)?; let merge = build_then_else_merge_from_if(&mut cfg_builder, &const_unit, entry)?; - let tail = build_loop_from_header(&mut cfg_builder, &pred_const, merge)?; - cfg_builder.branch(&merge, 0, &tail)?; // trivial "loop body" + let head = if separate { + let h = n_identity( + cfg_builder.simple_block_builder(type_row![NAT], type_row![NAT], 1)?, + &const_unit, + )?; + cfg_builder.branch(&merge, 0, &h)?; + h + } else { + merge + }; + let tail = build_loop_from_header(&mut cfg_builder, &pred_const, head)?; + cfg_builder.branch(&head, 0, &tail)?; // trivial "loop body" let exit = cfg_builder.exit_block(); cfg_builder.branch(&tail, 0, &exit)?; From f0d36c1ea33a4e4f7771da9f808774da48eda284 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 16:41:45 +0100 Subject: [PATCH 32/41] Test moving entry --- src/hugr/rewrite/outline_cfg.rs | 34 ++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 0c1e02aea..7fd97c51c 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -202,7 +202,9 @@ pub enum OutlineCfgError { mod test { use std::collections::HashSet; - use crate::algorithm::nest_cfgs::test::build_conditional_in_loop_cfg; + use crate::algorithm::nest_cfgs::test::{ + build_cond_then_loop_cfg, build_conditional_in_loop_cfg, + }; use crate::ops::handle::NodeHandle; use crate::{HugrView, Node}; use cool_asserts::assert_matches; @@ -278,4 +280,34 @@ mod test { HashSet::from([exit, new_block]) ); } + + #[test] + fn test_outline_cfg_move_entry() { + // /-> left --\ + // entry > merge -> head -> tail -> exit + // \-> right -/ \-<--<-/ + let (mut h, merge, tail) = build_cond_then_loop_cfg(true).unwrap(); + + let (entry, exit) = h.children(h.root()).take(2).collect_tuple().unwrap(); + let (left, right) = h.output_neighbours(entry).take(2).collect_tuple().unwrap(); + let (merge, tail) = (merge.node(), tail.node()); + let head = h.output_neighbours(merge).exactly_one().unwrap(); + + h.validate().unwrap(); + let blocks_to_move = [entry, left, right, merge]; + let other_blocks = [head, tail, exit]; + for &n in blocks_to_move.iter().chain(other_blocks.iter()) { + assert_eq!(depth(&h, n), 1); + } + h.apply_rewrite(OutlineCfg::new(blocks_to_move.iter().copied())) + .unwrap(); + h.validate().unwrap(); + let new_entry = h.children(h.root()).next().unwrap(); + for n in other_blocks { + assert_eq!(depth(&h, n), 1); + } + for n in blocks_to_move { + assert_eq!(h.get_parent(h.get_parent(n).unwrap()).unwrap(), new_entry); + } + } } From 1588016795dd53bffa2177f14db294d7004bfdb2 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 16:48:35 +0100 Subject: [PATCH 33/41] Refine spec to allow entry node in/out of set, but exclude exit node --- specification/hugr.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index f71de2111..e6c35d01f 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1229,15 +1229,23 @@ nodes as children. Replace a set of CFG sibling nodes with a single BasicBlock node having a CFG node child which has as its children the set of BasicBlock nodes originally specified. The set of basic blocks must satisfy constraints: -* There must be exactly one node in the set with incoming (controlflow) edges - from nodes outside the set. -* The set may not include the entry block +* There must be at most one node in the set with incoming (controlflow) edges + from nodes outside the set. Specifically, + * *either* the set includes the CFG's entry node, and any edges from outside + the set (there may be none or more) target said entry node; + * *or* the set does not include the CFG's entry node, but contains exactly one + node which is the target of edges from outside the set. +* The set may not include the exit block. * There must be exactly one edge from a node in the set to a node outside it. Situations in which multiple nodes have edges leaving the set, can be converted to -this form by a combination of InsertIdentity operations and one Replace. Similarly, -rather than movng the Exit block into the nested CFG, an identity node with a single -successor can be added just before, with that edge used as the exitting edge. +this form by a combination of InsertIdentity operations and one Replace. Similarly/for example, +rather than movng the Exit block into the nested CFG: +1. an identity node with a single successor can be added onto each edge into the Exit +2. if there is more than one such incoming edge, these identity nodes can then all be combined + by a Replace operation that removes them all and adds a single node (keeping the same number + of in-edges, but reducing to one out-edge to the Exit) +3. The single edge to the Exit node can then be used as the exitting edge. ** Note ** This means there is no way to move the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. From 86e3fe2fa0c951e9c23244f6b45e72c21f9ffca8 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 16:50:56 +0100 Subject: [PATCH 34/41] Revert "simple_replace.rs: use HugrMut::remove_node, includes clearing op_types" This reverts commit 47cdc3fe4012ce149ce3b35061aad81db5d32ed9. --- src/hugr/rewrite/simple_replace.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hugr/rewrite/simple_replace.rs b/src/hugr/rewrite/simple_replace.rs index 32fd58698..e688fd342 100644 --- a/src/hugr/rewrite/simple_replace.rs +++ b/src/hugr/rewrite/simple_replace.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use itertools::Itertools; -use portgraph::{LinkMut, LinkView, MultiMut, NodeIndex, PortView}; +use portgraph::{LinkMut, LinkView, MultiMut, NodeIndex, PortMut, PortView}; use crate::hugr::{HugrMut, HugrView}; use crate::{ @@ -227,7 +227,8 @@ impl Rewrite for SimpleReplacement { } // 3.5. Remove all nodes in self.removal and edges between them. for node in &self.removal { - h.remove_node(*node).unwrap(); + h.graph.remove_node(node.index); + h.hierarchy.remove(node.index); } Ok(()) } From dad6c5f1eee9bc3940a8af251525837fa37a1830 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 17:21:55 +0100 Subject: [PATCH 35/41] exitting -> exiting --- specification/hugr.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/hugr.md b/specification/hugr.md index e6c35d01f..7a1f926df 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1245,7 +1245,7 @@ rather than movng the Exit block into the nested CFG: 2. if there is more than one such incoming edge, these identity nodes can then all be combined by a Replace operation that removes them all and adds a single node (keeping the same number of in-edges, but reducing to one out-edge to the Exit) -3. The single edge to the Exit node can then be used as the exitting edge. +3. The single edge to the Exit node can then be used as the exiting edge. ** Note ** This means there is no way to move the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. From 382e70c7ebf985940478e7c8df8678acb88649fc Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 17:24:18 +0100 Subject: [PATCH 36/41] [spec] Remove bogus note on inserting identity before entry block --- specification/hugr.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index 7a1f926df..195b20852 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1247,8 +1247,6 @@ rather than movng the Exit block into the nested CFG: of in-edges, but reducing to one out-edge to the Exit) 3. The single edge to the Exit node can then be used as the exiting edge. -** Note ** This means there is no way to move the entry block (creating a new block to replace the entry block). Neither does `InsertIdentity` allow inserting a new identity block before the entry block ("inbetween" the containing CFG node and the entry block), as this is not an edge. - ##### Inlining methods These are the exact inverses of the above. From 369f62e0b556fdf1c4dbae6be72a95f64e732c29 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 17:27:23 +0100 Subject: [PATCH 37/41] an -> a for consistency --- src/hugr/rewrite/outline_cfg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index 7fd97c51c..bd4282cc2 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -179,7 +179,7 @@ pub enum OutlineCfgError { #[error("The nodes did not all have the same parent")] NotSiblings, /// The parent node was not a CFG node - #[error("The parent node {0:?} was not a CFG but an {1:?}")] + #[error("The parent node {0:?} was not a CFG but a {1:?}")] ParentNotCfg(Node, OpType), /// Multiple blocks had incoming edges #[error("Multiple blocks had predecessors outside the set - at least {0:?} and {1:?}")] From e5f83e6128d59024d6d7f91a3df87d2623edf019 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 18:51:05 +0100 Subject: [PATCH 38/41] Comment and test from_existing --- src/builder/cfg.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/builder/cfg.rs b/src/builder/cfg.rs index 63ffac87e..5d7188d57 100644 --- a/src/builder/cfg.rs +++ b/src/builder/cfg.rs @@ -97,6 +97,8 @@ impl + AsRef> CFGBuilder { 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 { 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"});}; @@ -105,7 +107,7 @@ impl + AsRef> CFGBuilder { Ok(Self { base, cfg_node, - inputs: None, + inputs: None, // This will prevent creating an entry node exit_node, n_out_wires, }) @@ -294,6 +296,8 @@ impl BlockBuilder { #[cfg(test)] mod test { + use std::collections::HashSet; + use cool_asserts::assert_matches; use crate::builder::build_traits::HugrBuilder; @@ -335,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::>(); + assert_eq!(expected_nodes, HashSet::from_iter(h2.children(h2.root()))); + + Ok(()) + } + fn build_basic_cfg + AsRef>( cfg_builder: &mut CFGBuilder, ) -> Result<(), BuildError> { From 6eca8a73424cf62247f99ce21b4dcbb38e0e417c Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 4 Jul 2023 18:54:26 +0100 Subject: [PATCH 39/41] Make OutlineCfg a struct with blocks --- src/hugr/rewrite/outline_cfg.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/hugr/rewrite/outline_cfg.rs b/src/hugr/rewrite/outline_cfg.rs index bd4282cc2..18bf68b22 100644 --- a/src/hugr/rewrite/outline_cfg.rs +++ b/src/hugr/rewrite/outline_cfg.rs @@ -13,17 +13,21 @@ use crate::{type_row, Hugr, Node}; /// Moves part of a Control-flow Sibling Graph into a new CFG-node /// that is the only child of a new Basic Block in the original CSG. -pub struct OutlineCfg(HashSet); +pub struct OutlineCfg { + blocks: HashSet, +} impl OutlineCfg { /// Create a new OutlineCfg rewrite that will move the provided blocks. pub fn new(blocks: impl IntoIterator) -> Self { - Self(HashSet::from_iter(blocks)) + Self { + blocks: HashSet::from_iter(blocks), + } } fn compute_entry_exit_outside(&self, h: &Hugr) -> Result<(Node, Node, Node), OutlineCfgError> { let cfg_n = match self - .0 + .blocks .iter() .map(|n| h.get_parent(*n)) .unique() @@ -39,8 +43,11 @@ impl OutlineCfg { let cfg_entry = h.children(cfg_n).next().unwrap(); let mut entry = None; let mut exit_succ = None; - for &n in self.0.iter() { - if n == cfg_entry || h.input_neighbours(n).any(|pred| !self.0.contains(&pred)) { + for &n in self.blocks.iter() { + if n == cfg_entry + || h.input_neighbours(n) + .any(|pred| !self.blocks.contains(&pred)) + { match entry { None => { entry = Some(n); @@ -50,7 +57,7 @@ impl OutlineCfg { } } } - let external_succs = h.output_neighbours(n).filter(|s| !self.0.contains(s)); + let external_succs = h.output_neighbours(n).filter(|s| !self.blocks.contains(s)); match external_succs.at_most_one() { Ok(None) => (), // No external successors Ok(Some(o)) => match exit_succ { @@ -123,7 +130,7 @@ impl Rewrite for OutlineCfg { .linked_ports(entry, h.node_inputs(entry).exactly_one().unwrap()) .collect(); for (pred, br) in preds { - if !self.0.contains(&pred) { + if !self.blocks.contains(&pred) { h.disconnect(pred, br).unwrap(); h.connect(pred, br.index(), new_block, 0).unwrap(); } @@ -144,7 +151,7 @@ impl Rewrite for OutlineCfg { .insert_before(entry.index, inner_exit.index) .unwrap(); // And remaining nodes - for n in self.0 { + for n in self.blocks { // Do not move the entry node, as we have already if n != entry { h.hierarchy.detach(n.index); From 37b2b7afa167682e789df22ebc5481a31a5690e0 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 5 Jul 2023 10:08:51 +0100 Subject: [PATCH 40/41] Spec wording tweaks --- specification/hugr.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index 195b20852..d5e42090c 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1238,13 +1238,13 @@ originally specified. The set of basic blocks must satisfy constraints: * The set may not include the exit block. * There must be exactly one edge from a node in the set to a node outside it. -Situations in which multiple nodes have edges leaving the set, can be converted to -this form by a combination of InsertIdentity operations and one Replace. Similarly/for example, -rather than movng the Exit block into the nested CFG: -1. an identity node with a single successor can be added onto each edge into the Exit -2. if there is more than one such incoming edge, these identity nodes can then all be combined - by a Replace operation that removes them all and adds a single node (keeping the same number - of in-edges, but reducing to one out-edge to the Exit) +Situations in which multiple nodes have edges leaving the set, or where the Exit block +would be in the set, can be converted to this form by a combination of InsertIdentity +operations and one Replace. For example, rather than moving the Exit block into the nested CFG: +1. An Identity node with a single successor can be added onto each edge into the Exit +2. If there is more than one edge into the Exit, these Identity nodes can then all be combined + by a Replace operation that removes them all and adds a single Identity (keeping the same number + of in-edges, but reducing to one out-edge to the Exit). 3. The single edge to the Exit node can then be used as the exiting edge. ##### Inlining methods From 8d0d0b15f009a7894f0743927804d5d36a14eb59 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 5 Jul 2023 10:11:19 +0100 Subject: [PATCH 41/41] And some more. --- specification/hugr.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specification/hugr.md b/specification/hugr.md index d5e42090c..5fcfa585d 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1234,8 +1234,8 @@ originally specified. The set of basic blocks must satisfy constraints: * *either* the set includes the CFG's entry node, and any edges from outside the set (there may be none or more) target said entry node; * *or* the set does not include the CFG's entry node, but contains exactly one - node which is the target of edges from outside the set. -* The set may not include the exit block. + node which is the target of at least one edge(s) from outside the set. +* The set may not include the Exit block. * There must be exactly one edge from a node in the set to a node outside it. Situations in which multiple nodes have edges leaving the set, or where the Exit block @@ -1243,7 +1243,7 @@ would be in the set, can be converted to this form by a combination of InsertIde operations and one Replace. For example, rather than moving the Exit block into the nested CFG: 1. An Identity node with a single successor can be added onto each edge into the Exit 2. If there is more than one edge into the Exit, these Identity nodes can then all be combined - by a Replace operation that removes them all and adds a single Identity (keeping the same number + by a Replace operation changing them all for a single Identity (keeping the same number of in-edges, but reducing to one out-edge to the Exit). 3. The single edge to the Exit node can then be used as the exiting edge.