Skip to content

Commit ccd2eb2

Browse files
zrhozrho
authored
fix!: Fixed bugs in model CFG handling and improved CFG signatures (#2334)
This PR fixes some bugs and a design error related to model CFGs. The signature of a control flow region and of a dataflow block is now of the form `(core.ctrl ?inputs ?outputs)` where `?inputs` and `?outputs` are lists of lists of types. Each item in the outer list corresponds to a branch and the items in the inner lists of the types of the values that are transmitted in that branch. BREAKING CHANGE: The model CFG signature types were changed.
1 parent cd9a7b7 commit ccd2eb2

File tree

8 files changed

+89
-95
lines changed

8 files changed

+89
-95
lines changed

‎hugr-core/src/export.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::{
1414
},
1515
types::{
1616
CustomType, EdgeKind, FuncTypeBase, MaybeRV, PolyFuncTypeBase, RowVariable, SumType,
17-
TypeArg, TypeBase, TypeBound, TypeEnum, TypeRow,
17+
TypeArg, TypeBase, TypeBound, TypeEnum,
1818
type_param::{TypeArgVariable, TypeParam},
1919
type_row::TypeRowBase,
2020
},
@@ -578,7 +578,6 @@ impl<'a> Context<'a> {
578578
pub fn export_block_signature(&mut self, block: &DataflowBlock) -> table::TermId {
579579
let inputs = {
580580
let inputs = self.export_type_row(&block.inputs);
581-
let inputs = self.make_term_apply(model::CORE_CTRL, &[inputs]);
582581
self.make_term(table::Term::List(
583582
self.bump.alloc_slice_copy(&[table::SeqPart::Item(inputs)]),
584583
))
@@ -590,13 +589,12 @@ impl<'a> Context<'a> {
590589
let mut outputs = BumpVec::with_capacity_in(block.sum_rows.len(), self.bump);
591590
for sum_row in &block.sum_rows {
592591
let variant = self.export_type_row_with_tail(sum_row, Some(tail));
593-
let control = self.make_term_apply(model::CORE_CTRL, &[variant]);
594-
outputs.push(table::SeqPart::Item(control));
592+
outputs.push(table::SeqPart::Item(variant));
595593
}
596594
self.make_term(table::Term::List(outputs.into_bump_slice()))
597595
};
598596

599-
self.make_term_apply(model::CORE_FN, &[inputs, outputs])
597+
self.make_term_apply(model::CORE_CTRL, &[inputs, outputs])
600598
}
601599

602600
/// Creates a data flow region from the given node's children.
@@ -740,18 +738,21 @@ impl<'a> Context<'a> {
740738
let signature = {
741739
let node_signature = self.hugr.signature(node).unwrap();
742740

743-
let mut wrap_ctrl = |types: &TypeRow| {
744-
let types = self.export_type_row(types);
745-
let types_ctrl = self.make_term_apply(model::CORE_CTRL, &[types]);
741+
let inputs = {
742+
let types = self.export_type_row(node_signature.input());
746743
self.make_term(table::Term::List(
747-
self.bump
748-
.alloc_slice_copy(&[table::SeqPart::Item(types_ctrl)]),
744+
self.bump.alloc_slice_copy(&[table::SeqPart::Item(types)]),
749745
))
750746
};
751747

752-
let inputs = wrap_ctrl(node_signature.input());
753-
let outputs = wrap_ctrl(node_signature.output());
754-
Some(self.make_term_apply(model::CORE_FN, &[inputs, outputs]))
748+
let outputs = {
749+
let types = self.export_type_row(node_signature.output());
750+
self.make_term(table::Term::List(
751+
self.bump.alloc_slice_copy(&[table::SeqPart::Item(types)]),
752+
))
753+
};
754+
755+
Some(self.make_term_apply(model::CORE_CTRL, &[inputs, outputs]))
755756
};
756757

757758
let scope = match closure {

‎hugr-core/src/import.rs

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,7 @@ impl<'a> Context<'a> {
826826
}
827827

828828
let region_target_types = (|| {
829-
let [_, region_targets] = self.get_func_type(
829+
let [_, region_targets] = self.get_ctrl_type(
830830
region_data
831831
.signature
832832
.ok_or_else(|| error_uninferred!("region signature"))?,
@@ -860,25 +860,18 @@ impl<'a> Context<'a> {
860860
};
861861

862862
// The entry node in core control flow regions is identified by being
863-
// the first child node of the CFG node. We therefore import the entry
864-
// node first and follow it up by every other node.
863+
// the first child node of the CFG node. We therefore import the entry node first.
865864
self.import_node(entry_node, node)?;
866865

867-
for child in region_data.children {
868-
if *child != entry_node {
869-
self.import_node(*child, node)?;
870-
}
871-
}
872-
873-
// Create the exit node for the control flow region.
866+
// Create the exit node for the control flow region. This always needs
867+
// to be second in the node list.
874868
{
875869
let cfg_outputs = {
876-
let [ctrl_type] = region_target_types.as_slice() else {
870+
let [target_types] = region_target_types.as_slice() else {
877871
return Err(error_invalid!("cfg region expects a single target"));
878872
};
879873

880-
let [types] = self.expect_symbol(*ctrl_type, model::CORE_CTRL)?;
881-
self.import_type_row(types)?
874+
self.import_type_row(*target_types)?
882875
};
883876

884877
let exit = self
@@ -887,6 +880,13 @@ impl<'a> Context<'a> {
887880
self.record_links(exit, Direction::Incoming, region_data.targets);
888881
}
889882

883+
// Finally we import all other nodes.
884+
for child in region_data.children {
885+
if *child != entry_node {
886+
self.import_node(*child, node)?;
887+
}
888+
}
889+
890890
for meta_item in region_data.meta {
891891
self.import_node_metadata(node, *meta_item)
892892
.map_err(|err| error_context!(err, "node metadata"))?;
@@ -1245,13 +1245,6 @@ impl<'a> Context<'a> {
12451245
return Err(error_unsupported!("`{}` as `TypeParam`", model::CORE_CONST));
12461246
}
12471247

1248-
if let Some([]) = self.match_symbol(term_id, model::CORE_CTRL_TYPE)? {
1249-
return Err(error_unsupported!(
1250-
"`{}` as `TypeParam`",
1251-
model::CORE_CTRL_TYPE
1252-
));
1253-
}
1254-
12551248
if let Some([item_type]) = self.match_symbol(term_id, model::CORE_LIST_TYPE)? {
12561249
// At present `hugr-model` has no way to express that the item
12571250
// type of a list must be copyable. Therefore we import it as `Any`.
@@ -1339,13 +1332,6 @@ impl<'a> Context<'a> {
13391332
return Err(error_unsupported!("`{}` as `TypeArg`", model::CORE_STATIC));
13401333
}
13411334

1342-
if let Some([]) = self.match_symbol(term_id, model::CORE_CTRL_TYPE)? {
1343-
return Err(error_unsupported!(
1344-
"`{}` as `TypeArg`",
1345-
model::CORE_CTRL_TYPE
1346-
));
1347-
}
1348-
13491335
if let Some([]) = self.match_symbol(term_id, model::CORE_CONST)? {
13501336
return Err(error_unsupported!("`{}` as `TypeArg`", model::CORE_CONST));
13511337
}
@@ -1510,6 +1496,11 @@ impl<'a> Context<'a> {
15101496
.ok_or(error_invalid!("expected a function type"))
15111497
}
15121498

1499+
fn get_ctrl_type(&mut self, term_id: table::TermId) -> Result<[table::TermId; 2], ImportError> {
1500+
self.match_symbol(term_id, model::CORE_CTRL)?
1501+
.ok_or(error_invalid!("expected a control type"))
1502+
}
1503+
15131504
fn import_func_type<RV: MaybeRV>(
15141505
&mut self,
15151506
term_id: table::TermId,

‎hugr-core/tests/snapshots/model__roundtrip_cfg.snap

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
source: hugr-core/tests/model.rs
3-
expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.edn\"))"
3+
expression: ast
44
---
55
(hugr 0)
66

@@ -22,10 +22,9 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.
2222
(cfg [%0] [%1]
2323
(signature (core.fn [?0] [?0]))
2424
(cfg [%2] [%3]
25-
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
25+
(signature (core.ctrl [[?0]] [[?0]]))
2626
(block [%2] [%3 %2]
27-
(signature
28-
(core.fn [(core.ctrl [?0])] [(core.ctrl [?0]) (core.ctrl [?0])]))
27+
(signature (core.ctrl [[?0]] [[?0] [?0]]))
2928
(dfg [%4] [%5]
3029
(signature (core.fn [?0] [(core.adt [[?0] [?0]])]))
3130
((core.make_adt 0) [%4] [%5]
@@ -37,15 +36,15 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-cfg.
3736
(cfg [%0] [%1]
3837
(signature (core.fn [?0] [?0]))
3938
(cfg [%2] [%3]
40-
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
39+
(signature (core.ctrl [[?0]] [[?0]]))
4140
(block [%2] [%6]
42-
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
41+
(signature (core.ctrl [[?0]] [[?0]]))
4342
(dfg [%4] [%5]
4443
(signature (core.fn [?0] [(core.adt [[?0]])]))
4544
((core.make_adt 0) [%4] [%5]
4645
(signature (core.fn [?0] [(core.adt [[?0]])])))))
4746
(block [%6] [%3]
48-
(signature (core.fn [(core.ctrl [?0])] [(core.ctrl [?0])]))
47+
(signature (core.ctrl [[?0]] [[?0]]))
4948
(dfg [%7] [%8]
5049
(signature (core.fn [?0] [(core.adt [[?0]])]))
5150
((core.make_adt 0) [%7] [%8]

‎hugr-core/tests/snapshots/model__roundtrip_entrypoint.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
source: hugr-core/tests/model.rs
3-
expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-entrypoint.edn\"))"
3+
expression: ast
44
---
55
(hugr 0)
66

@@ -40,10 +40,10 @@ expression: "roundtrip(include_str!(\"../../hugr-model/tests/fixtures/model-entr
4040
(cfg
4141
(signature (core.fn [] []))
4242
(cfg [%0] [%1]
43-
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
43+
(signature (core.ctrl [[]] [[]]))
4444
(meta core.entrypoint)
4545
(block [%0] [%1]
46-
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
46+
(signature (core.ctrl [[]] [[]]))
4747
(dfg [] [%2]
4848
(signature (core.fn [] [(core.adt [[]])]))
4949
((core.make_adt 0) [] [%2]

‎hugr-model/src/v0/mod.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,13 @@ pub const CORE_BYTES_TYPE: &str = "core.bytes";
163163
/// - **Result:** `core.static`
164164
pub const CORE_FLOAT_TYPE: &str = "core.float";
165165

166-
/// Type of a control flow edge.
166+
/// Type of control flow regions.
167167
///
168-
/// - **Parameter:** `?types : (core.list core.type)`
169-
/// - **Result:** `core.ctrl_type`
168+
/// - **Parameter:** `?inputs : (core.list (core.list core.type))`
169+
/// - **Parameter:** `?outputs : (core.list (core.list core.type))`
170+
/// - **Result:** `core.type`
170171
pub const CORE_CTRL: &str = "core.ctrl";
171172

172-
/// The type of the types for control flow edges.
173-
///
174-
/// - **Result:** `?type : core.static`
175-
pub const CORE_CTRL_TYPE: &str = "core.ctrl_type";
176-
177173
/// The type for runtime constants.
178174
///
179175
/// - **Parameter:** `?type : core.type`

‎hugr-model/tests/fixtures/model-cfg.edn

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,17 @@
66
(param ?a core.type)
77
(core.fn [?a] [?a])
88
(dfg [%0] [%1]
9-
(signature (core.fn [?a] [?a]))
10-
(cfg [%0] [%1]
11-
(signature (core.fn [?a] [?a]))
12-
(cfg [%2] [%4]
13-
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
14-
(block [%2] [%4 %2]
15-
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a]) (core.ctrl [?a])]))
16-
(dfg [%5] [%6]
17-
(signature (core.fn [?a] [(core.adt [[?a] [?a]])]))
18-
((core.make_adt 0) [%5] [%6]
19-
(signature (core.fn [?a] [(core.adt [[?a] [?a]])])))))))))
9+
(signature (core.fn [?a] [?a]))
10+
(cfg [%0] [%1]
11+
(signature (core.fn [?a] [?a]))
12+
(cfg [%2] [%3]
13+
(signature (core.ctrl [[?a]] [[?a]]))
14+
(block [%2] [%3 %2]
15+
(signature (core.ctrl [[?a]] [[?a] [?a]]))
16+
(dfg [%4] [%5]
17+
(signature (core.fn [?a] [(core.adt [[?a] [?a]])]))
18+
((core.make_adt 0) [%4] [%5]
19+
(signature (core.fn [?a] [(core.adt [[?a] [?a]])])))))))))
2020

2121
(define-func example.cfg_order
2222
(param ?a core.type)
@@ -26,15 +26,15 @@
2626
(cfg [%0] [%1]
2727
(signature (core.fn [?a] [?a]))
2828
(cfg [%2] [%4]
29-
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
29+
(signature (core.ctrl [[?a]] [[?a]]))
3030
(block [%3] [%4]
31-
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
31+
(signature (core.ctrl [[?a]] [[?a]]))
3232
(dfg [%5] [%6]
3333
(signature (core.fn [?a] [(core.adt [[?a]])]))
3434
((core.make_adt _ _ 0) [%5] [%6]
3535
(signature (core.fn [?a] [(core.adt [[?a]])])))))
3636
(block [%2] [%3]
37-
(signature (core.fn [(core.ctrl [?a])] [(core.ctrl [?a])]))
37+
(signature (core.ctrl [[?a]] [[?a]]))
3838
(dfg [%7] [%8]
3939
(signature (core.fn [?a] [(core.adt [[?a]])]))
4040
((core.make_adt _ _ 0) [%7] [%8]

‎hugr-model/tests/fixtures/model-entrypoint.edn

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@
2525
(cfg [] []
2626
(signature (core.fn [] []))
2727
(cfg [%entry] [%exit]
28-
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
28+
(signature (core.ctrl [[]] [[]]))
2929
(meta core.entrypoint)
3030
(block [%entry] [%exit]
31-
(signature (core.fn [(core.ctrl [])] [(core.ctrl [])]))
31+
(signature (core.ctrl [[]] [[]]))
3232
(dfg [] [%value]
3333
(signature (core.fn [] [(core.adt [[]])]))
3434
((core.make_adt _ _ 0) [] [%value]

0 commit comments

Comments
 (0)