Skip to content

Commit

Permalink
fix: BasicBlockExits should not be OpTag::DataflowParent (#1409)
Browse files Browse the repository at this point in the history
The optags for basic blocks where defined as
```mermaid
graph TD
  classDef transparent fill:#0000,stroke:#0000
  a[...]:::transparent-->DataflowParent
  b[...]:::transparent-->ControlFlowChild
  DataflowParent --> BasicBlock
  ControlFlowChild --> BasicBlock
  BasicBlock --> BasicBlockExit
```
notice that `BasicBlockExit` was a descendant from `DataflowParent`.
This caused errors on code that checked for the tag, such as the
[`force_order`
pass](https://github.com/CQCL/hugr/blob/e485a23e0cb73b0070e5d6f3374f6de16f545d04/hugr-passes/src/force_order.rs#L58-L59).

This PR changes the hierarchy to
```mermaid
graph TD
  classDef transparent fill:#0000,stroke:#0000
  a[...]:::transparent-->DataflowParent
  b[...]:::transparent-->ControlFlowChild
  DataflowParent --> DataflowBlock
  ControlFlowChild --> BasicBlockExit
  ControlFlowChild --> DataflowBlock
```

Closes #1408 

BREAKING CHANGE: Removed `OpTag::BasicBlock`, replaced with
`OpTag::DataflowBlock` and `OpTag::ControlFlowChild`

---------

Co-authored-by: Seyon Sivarajah <[email protected]>
  • Loading branch information
aborgna-q and ss2165 authored Aug 9, 2024
1 parent c5d1a74 commit 373fb22
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 14 deletions.
4 changes: 2 additions & 2 deletions hugr-core/src/builder/build_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,8 @@ fn wire_up<T: Dataflow + ?Sized>(
});
};

if !OpTag::BasicBlock.is_superset(base.get_optype(src).tag())
&& !OpTag::BasicBlock.is_superset(base.get_optype(src_sibling).tag())
if !OpTag::ControlFlowChild.is_superset(base.get_optype(src).tag())
&& !OpTag::ControlFlowChild.is_superset(base.get_optype(src_sibling).tag())
{
// Add a state order constraint unless one of the nodes is a CFG BasicBlock
base.add_other_edge(src, src_sibling);
Expand Down
2 changes: 1 addition & 1 deletion hugr-core/src/hugr/views/root_checked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ mod test {
r,
Err(HugrError::InvalidTag {
required: OpTag::Dfg,
actual: ops::OpTag::BasicBlock
actual: ops::OpTag::DataflowBlock
})
);
// That didn't do anything:
Expand Down
2 changes: 1 addition & 1 deletion hugr-core/src/ops/controlflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl NamedOp for ExitBlock {
}

impl StaticTag for DataflowBlock {
const TAG: OpTag = OpTag::BasicBlock;
const TAG: OpTag = OpTag::DataflowBlock;
}

impl StaticTag for ExitBlock {
Expand Down
2 changes: 1 addition & 1 deletion hugr-core/src/ops/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl_nodehandle!(ModuleRootID, OpTag::ModuleRoot);
impl_nodehandle!(ModuleID, OpTag::ModuleOp);
impl_nodehandle!(ConstID, OpTag::Const);

impl_nodehandle!(BasicBlockID, OpTag::BasicBlock);
impl_nodehandle!(BasicBlockID, OpTag::DataflowBlock);

impl<const DEF: bool> NodeHandle for FuncID<DEF> {
const TAG: OpTag = OpTag::Function;
Expand Down
20 changes: 12 additions & 8 deletions hugr-core/src/ops/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ pub enum OpTag {
/// A leaf operation.
Leaf,

/// A control flow basic block.
BasicBlock,
/// A control flow basic block defining a dataflow graph.
DataflowBlock,
/// A control flow exit node.
BasicBlockExit,
}
Expand Down Expand Up @@ -113,8 +113,8 @@ impl OpTag {
OpTag::Function => &[OpTag::ModuleOp, OpTag::StaticOutput],
OpTag::Alias => &[OpTag::ScopedDefn],
OpTag::FuncDefn => &[OpTag::Function, OpTag::ScopedDefn, OpTag::DataflowParent],
OpTag::BasicBlock => &[OpTag::ControlFlowChild, OpTag::DataflowParent],
OpTag::BasicBlockExit => &[OpTag::BasicBlock],
OpTag::DataflowBlock => &[OpTag::ControlFlowChild, OpTag::DataflowParent],
OpTag::BasicBlockExit => &[OpTag::ControlFlowChild],
OpTag::Case => &[OpTag::Any, OpTag::DataflowParent],
OpTag::ModuleRoot => &[OpTag::Any],
OpTag::Const => &[OpTag::ScopedDefn, OpTag::StaticOutput],
Expand Down Expand Up @@ -148,7 +148,7 @@ impl OpTag {
OpTag::Input => "Input node",
OpTag::Output => "Output node",
OpTag::FuncDefn => "Function definition",
OpTag::BasicBlock => "Basic block",
OpTag::DataflowBlock => "Basic block containing a dataflow graph",
OpTag::BasicBlockExit => "Exit basic block node",
OpTag::Case => "Case",
OpTag::ModuleRoot => "Module root node",
Expand Down Expand Up @@ -213,16 +213,20 @@ mod test {
assert!(OpTag::None.is_superset(OpTag::None));
assert!(OpTag::ModuleOp.is_superset(OpTag::ModuleOp));
assert!(OpTag::DataflowChild.is_superset(OpTag::DataflowChild));
assert!(OpTag::BasicBlock.is_superset(OpTag::BasicBlock));
assert!(OpTag::ControlFlowChild.is_superset(OpTag::ControlFlowChild));

assert!(OpTag::Any.is_superset(OpTag::None));
assert!(OpTag::Any.is_superset(OpTag::ModuleOp));
assert!(OpTag::Any.is_superset(OpTag::DataflowChild));
assert!(OpTag::Any.is_superset(OpTag::BasicBlock));
assert!(OpTag::Any.is_superset(OpTag::ControlFlowChild));

assert!(!OpTag::None.is_superset(OpTag::Any));
assert!(!OpTag::None.is_superset(OpTag::ModuleOp));
assert!(!OpTag::None.is_superset(OpTag::DataflowChild));
assert!(!OpTag::None.is_superset(OpTag::BasicBlock));
assert!(!OpTag::None.is_superset(OpTag::ControlFlowChild));

// Other specific checks
assert!(!OpTag::DataflowParent.is_superset(OpTag::BasicBlockExit));
assert!(!OpTag::DataflowParent.is_superset(OpTag::Cfg));
}
}
2 changes: 1 addition & 1 deletion hugr-core/src/ops/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl ValidateOp for super::CFG {
fn validity_flags(&self) -> OpValidityFlags {
OpValidityFlags {
allowed_children: OpTag::ControlFlowChild,
allowed_first_child: OpTag::BasicBlock,
allowed_first_child: OpTag::DataflowBlock,
allowed_second_child: OpTag::BasicBlockExit,
requires_children: true,
requires_dag: false,
Expand Down

0 comments on commit 373fb22

Please sign in to comment.