From 0e6fcc7e2f1945b3cc0a3a0589f01e3f6fb9ffcb Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Wed, 5 Jul 2023 10:58:28 +0100 Subject: [PATCH] Re-spec and Implement OutlineCfg rewrite (#225) * Spec: rewrite requires exactly one entry node, and exactly one exit edge; excludes Exit block. * Add src/hugr/rewrite/outline_cfg.rs with OutlineCfg struct * src/algorithm/nest_cfgs.rs (test): break out build_loop_then_cfg; use (in/out)put_neighbours * src/builder/cfg.rs: add CFGBuilder::from_existing --- specification/hugr.md | 25 ++- src/algorithm/nest_cfgs.rs | 72 ++++--- src/builder/cfg.rs | 50 +++++ src/hugr/rewrite.rs | 1 + src/hugr/rewrite/outline_cfg.rs | 320 ++++++++++++++++++++++++++++++++ 5 files changed, 434 insertions(+), 34 deletions(-) create mode 100644 src/hugr/rewrite/outline_cfg.rs diff --git a/specification/hugr.md b/specification/hugr.md index d25ae5e29..5fcfa585d 100644 --- a/specification/hugr.md +++ b/specification/hugr.md @@ -1226,11 +1226,26 @@ 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: +* 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 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 +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 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. ##### Inlining methods diff --git a/src/algorithm/nest_cfgs.rs b/src/algorithm/nest_cfgs.rs index ac1cae40d..d86d3eefd 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(false)?; 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");}; @@ -513,23 +506,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!( @@ -655,6 +639,36 @@ pub(crate) mod test { Ok((header, tail)) } + // 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)?; + 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 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)?; + + 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, diff --git a/src/builder/cfg.rs b/src/builder/cfg.rs index 5279b5a3a..5d7188d57 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,22 @@ 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"});}; + let n_out_wires = outputs.len(); + let (_, exit_node) = base.children(cfg_node).take(2).collect_tuple().unwrap(); + Ok(Self { + base, + cfg_node, + inputs: None, // This will prevent creating an entry node + exit_node, + n_out_wires, + }) + } + /// Return a builder for a non-entry [`BasicBlock::DFB`] child graph with `inputs` /// and `outputs` and the variants of the branching predicate Sum value /// specified by `predicate_variants`. @@ -278,6 +296,8 @@ impl BlockBuilder { #[cfg(test)] mod test { + use std::collections::HashSet; + use cool_asserts::assert_matches; use crate::builder::build_traits::HugrBuilder; @@ -319,6 +339,36 @@ mod test { Ok(()) } + #[test] + fn from_existing() -> Result<(), BuildError> { + let mut cfg_builder = CFGBuilder::new(type_row![NAT], type_row![NAT])?; + build_basic_cfg(&mut cfg_builder)?; + let h = cfg_builder.finish_hugr()?; + + let mut new_builder = CFGBuilder::from_existing(h.clone(), h.root())?; + assert_matches!(new_builder.simple_entry_builder(type_row![NAT], 1), Err(_)); + let h2 = new_builder.finish_hugr()?; + assert_eq!(h, h2); // No new nodes added + + let mut new_builder = CFGBuilder::from_existing(h.clone(), h.root())?; + let block_builder = new_builder.simple_block_builder( + vec![SimpleType::new_simple_predicate(1), NAT].into(), + type_row![NAT], + 1, + )?; + let new_bb = block_builder.container_node(); + let [pred, nat]: [Wire; 2] = block_builder.input_wires_arr(); + block_builder.finish_with_outputs(pred, [nat])?; + let h2 = new_builder.finish_hugr()?; + let expected_nodes = h + .children(h.root()) + .chain([new_bb]) + .collect::>(); + assert_eq!(expected_nodes, HashSet::from_iter(h2.children(h2.root()))); + + Ok(()) + } + fn build_basic_cfg + AsRef>( cfg_builder: &mut CFGBuilder, ) -> Result<(), BuildError> { diff --git a/src/hugr/rewrite.rs b/src/hugr/rewrite.rs index 73faacc47..f5bbb7088 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 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..18bf68b22 --- /dev/null +++ b/src/hugr/rewrite/outline_cfg.rs @@ -0,0 +1,320 @@ +//! 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; + +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::{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 { + blocks: HashSet, +} + +impl OutlineCfg { + /// Create a new OutlineCfg rewrite that will move the provided blocks. + pub fn new(blocks: impl IntoIterator) -> Self { + Self { + blocks: HashSet::from_iter(blocks), + } + } + + fn compute_entry_exit_outside(&self, h: &Hugr) -> Result<(Node, Node, Node), OutlineCfgError> { + let cfg_n = match self + .blocks + .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 cfg_entry = h.children(cfg_n).next().unwrap(); + let mut entry = None; + let mut exit_succ = None; + 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); + } + Some(prev) => { + return Err(OutlineCfgError::MultipleEntryNodes(prev, n)); + } + } + } + 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 { + None => { + exit_succ = Some((n, o)); + } + Some((prev, _)) => { + return Err(OutlineCfgError::MultipleExitNodes(prev, n)); + } + }, + Err(ext) => return Err(OutlineCfgError::MultipleExitEdges(n, ext.collect())), + }; + } + match (entry, exit_succ) { + (Some(e), Some((x, o))) => Ok((e, x, o)), + (None, _) => Err(OutlineCfgError::NoEntryNode), + (_, None) => Err(OutlineCfgError::NoExitNode), + } + } +} + +impl Rewrite for OutlineCfg { + type Error = OutlineCfgError; + const UNCHANGED_ON_FAILURE: bool = true; + fn verify(&self, h: &Hugr) -> Result<(), OutlineCfgError> { + self.compute_entry_exit_outside(h)?; + Ok(()) + } + fn apply(self, h: &mut Hugr) -> Result<(), OutlineCfgError> { + 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 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 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 = { + let parent = h.get_parent(entry).unwrap(); + CFGBuilder::from_existing(h, parent).unwrap() + }; + let mut new_block = existing_cfg + .block_builder(inputs.clone(), vec![type_row![]], outputs.clone()) + .unwrap(); + + // 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).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)) + .unwrap(); + let pred_wire = new_block.load_const(&predicate).unwrap(); + let new_block = new_block + .finish_with_outputs(pred_wire, cfg_outputs) + .unwrap() + .node(); + + // 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(entry, h.node_inputs(entry).exactly_one().unwrap()) + .collect(); + for (pred, br) in preds { + if !self.blocks.contains(&pred) { + h.disconnect(pred, br).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 + h.hierarchy.detach(entry.index); + h.hierarchy + .insert_before(entry.index, inner_exit.index) + .unwrap(); + // And remaining nodes + for n in self.blocks { + // Do not move the entry node, as we have already + 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 outside) to inner_exit + let exit_port = h + .node_outputs(exit) + .filter(|p| { + let (t, p2) = h.linked_ports(exit, *p).exactly_one().ok().unwrap(); + assert!(p2.index() == 0); + t == outside + }) + .exactly_one() + .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, 0, outside, 0).unwrap(); + + Ok(()) + } +} + +/// Errors that can occur in expressing an OutlineCfg rewrite. +#[derive(Debug, Error)] +pub enum OutlineCfgError { + /// 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 a {1:?}")] + ParentNotCfg(Node, OpType), + /// 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_cond_then_loop_cfg, 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, + } + } + + #[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::new([merge, tail])); + assert_matches!(r, Err(OutlineCfgError::MultipleExitEdges(_, _))); + assert_eq!(h, backup); + + 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); + + 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); + } + + #[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(); + 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(); + let blocks = [head, left, right, merge]; + h.apply_rewrite(OutlineCfg::new(blocks)).unwrap(); + h.validate().unwrap(); + 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]) + ); + } + + #[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); + } + } +}