feat: Normalize CFGs #2591

acl-cqc · 2025-09-26T15:01:09Z

closes #945, closes #2556.

Add module normalize_cfgs and a new merge_basic_blocks (with improved API) within
Normalizations (in fn normalize_cfg and NormalizeCFGPass):
- Single block CFG (Entry -> Exit) becomes a DFG
- If Entry node has no internal predecessors and only one successor -> move contents outside/before CFG
- If Exit node has only one predecessor -> move contents of that node outside/after CFG (and make it become the exit node)
Deprecate the old merge_basic_blocks as the interface is annoying (panicking, using RootCheckable).
While about it, generalize InlineDFG over NodeType

Does not address: "inlining" arbitrary CFG into a BB (requires splitting the outer BB into parts before/after the inner CFG); emptying (but preserving) a branching entry block (if no predecessors).

…rep2

codecov · 2025-09-26T15:06:15Z

Codecov Report

❌ Patch coverage is 89.53688% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.07%. Comparing base (4fb0a5e) to head (34491eb).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-passes/src/normalize_cfgs.rs	90.43%	17 Missing and 38 partials ⚠️
hugr-passes/src/lib.rs	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2591      +/-   ##
==========================================
+ Coverage   82.99%   83.07%   +0.07%     
==========================================
  Files         254      255       +1     
  Lines       48040    48422     +382     
  Branches    43550    43932     +382     
==========================================
+ Hits        39873    40225     +352     
- Misses       6098     6103       +5     
- Partials     2069     2094      +25

Flag	Coverage Δ
python	`91.60% <ø> (ø)`
rust	`82.19% <89.53%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This reverts commit 3050976.

acl-cqc · 2025-09-30T08:50:32Z

hugr-passes/src/normalize_cfg.rs

+    /// The CFG was preserved, but the entry or exit blocks may have changed.
+    #[allow(missing_docs)]
+    CFGPreserved {
+        entry_changed: bool,


would like to add 'num blocks merged' here, I guess could do that by changing signature of merge_basic_blocks and then making the merge_bbs:: version (currently a re-export) just throw away this information

Perhaps it'd be more useful to return the Option<Node> of the extracted DFGs rather than bools here?

Done both, taking the opportunity to sort out merge_basic_blocks by giving the "new" version a sensible interface with an error, and so deprecating only the old (uncalled) panicking version.

You might want me to move the inline module in hugr-passes/src/lib.rs out into merge_bbs.rs again, can do.

acl-cqc · 2025-09-30T08:55:17Z

hugr-passes/src/normalize_cfg.rs

+    #[allow(clippy::match_result_ok)] // let Ok(...) without .ok() borrows `h`
+    if let Some(succ) = h.output_neighbours(entry).exactly_one().ok() {
+        if succ == exit {
+            // Any (basic-block) predecessors of `entry` are unreachable, so allow/ignore


This seemed a neat trick: less checks on our part, more optimization. However I'm starting to wonder: it requires its own test and makes ComposablePass::run much harder. Perhaps a better strategy would be to enhance DeadCodeElimination to remove unreachable blocks (currently it assumes all BBs are reachable if the CFG is!) and insist here.

Not quite as good as I hoped - one could still have entirely disconnected BBs that raise the same issue - but again, it seems more uniform. Raised #2597.

acl-cqc · 2025-09-30T08:57:02Z

hugr-passes/src/normalize_cfg.rs

+                .filter(|n| hugr.get_optype(*n).is_cfg())
+                .collect::<Vec<_>>();
+            // Process inner CFGs first, in case they are removed
+            // (if they are in an unreachable block when the Entry node has only the Exit as successor)


Sadly even this current behaviour means that you can get results saying what was done to a CFG that was later removed from the Hugr altogether as the whole thing was unreachable...see https://github.com/CQCL/hugr/pull/2591/files#r2390401557

aborgna-q

Agreed with removing the optimized special case, but the rest looks gtm

hugr-passes/src/lib.rs

hugr-passes/src/normalize_cfgs.rs

aborgna-q · 2025-09-30T09:41:46Z

hugr-passes/src/normalize_cfg.rs

+    /// The CFG was preserved, but the entry or exit blocks may have changed.
+    #[allow(missing_docs)]
+    CFGPreserved {
+        entry_changed: bool,


Perhaps it'd be more useful to return the Option<Node> of the extracted DFGs rather than bools here?

hugrbot · 2025-09-30T11:58:13Z

This PR contains breaking changes to the public Rust API.
Please deprecate the old API instead (if possible), or mark the PR with a ! to indicate a breaking change.

cargo-semver-checks summary


--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.44.0/src/lints/enum_missing.ron

Failed in:
enum hugr_passes::non_local::NonLocalEdgesError, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/non_local.rs:57

aborgna-q

LGTM

aborgna-q · 2025-09-30T12:45:00Z

hugr-passes/src/lib.rs

 pub mod untuple;

+/// Merge basic blocks. Subset of [normalize_cfgs], use the latter.
+#[deprecated(note = "Use normalize_cfgs", since = "0.15.1")]


Mixed up the version with tket's

Suggested change

#[deprecated(note = "Use normalize_cfgs", since = "0.15.1")]

#[deprecated(note = "Use normalize_cfgs", since = "0.23.0")]

lol, should have twigged that was low...15 was ages ago! This looks ahead/predicts the next number, right...

aborgna-q · 2025-09-30T12:45:25Z

hugr-passes/src/lib.rs

+    /// If the `entrypoint` of `cfg` is not an [OpType::CFG]
+    ///
+    /// [OpType::CFG]: hugr_core::ops::OpType::CFG
+    #[deprecated(note = "Use version in normalize_cfgs", since = "0.15.1")]


Mixed up the version with tket's

Suggested change

#[deprecated(note = "Use version in normalize_cfgs", since = "0.15.1")]

#[deprecated(note = "Use version in normalize_cfgs", since = "0.23.0")]

## 🤖 New release * `hugr-model`: 0.22.4 -> 0.23.0 (✓ API compatible changes) * `hugr-core`: 0.22.4 -> 0.23.0 (⚠ API breaking changes) * `hugr-llvm`: 0.22.4 -> 0.23.0 (✓ API compatible changes) * `hugr-passes`: 0.22.4 -> 0.23.0 (⚠ API breaking changes) * `hugr-persistent`: 0.2.3 -> 0.3.0 (✓ API compatible changes) * `hugr`: 0.22.4 -> 0.23.0 (✓ API compatible changes) * `hugr-cli`: 0.22.4 -> 0.23.0 (✓ API compatible changes) ### ⚠ `hugr-core` breaking changes ```text --- failure enum_variant_missing: pub enum variant removed or renamed --- Description: A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_variant_missing.ron Failed in: variant PackageEncodingError::ExtensionVersion, previously in file /tmp/.tmp8eoQo1/hugr-core/src/envelope/package_json.rs:89 --- failure function_parameter_count_changed: pub fn parameter count changed --- Description: A publicly-visible function now takes a different number of parameters. ref: https://doc.rust-lang.org/cargo/reference/semver.html#fn-change-arity impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/function_parameter_count_changed.ron Failed in: hugr_core::import::import_package now takes 3 parameters instead of 2, in /tmp/.tmpBTXSed/hugr/hugr-core/src/import.rs:187 --- failure struct_missing: pub struct removed or renamed --- Description: A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/struct_missing.ron Failed in: struct hugr_core::extension::prelude::PRELUDE_REGISTRY, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43 struct hugr_core::extension::PRELUDE_REGISTRY, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43 struct hugr_core::std_extensions::logic::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/logic.rs:134 struct hugr_core::extension::prelude::PRELUDE, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43 struct hugr_core::extension::PRELUDE, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43 struct hugr_core::std_extensions::ptr::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/ptr.rs:112 struct hugr_core::std_extensions::arithmetic::int_types::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_types.rs:209 struct hugr_core::std_extensions::arithmetic::conversions::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/conversions.rs:174 struct hugr_core::std_extensions::collections::array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/array.rs:93 struct hugr_core::std_extensions::arithmetic::int_types::INT_TYPES, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_types.rs:49 struct hugr_core::std_extensions::arithmetic::float_types::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/float_types.rs:104 struct hugr_core::std_extensions::collections::static_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/static_array.rs:142 struct hugr_core::std_extensions::collections::list::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/list.rs:289 struct hugr_core::std_extensions::STD_REG, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions.rs:35 struct hugr_core::std_extensions::collections::value_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/value_array.rs:99 struct hugr_core::std_extensions::collections::borrow_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/borrow_array.rs:290 struct hugr_core::std_extensions::arithmetic::float_ops::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/float_ops.rs:115 struct hugr_core::std_extensions::arithmetic::int_ops::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_ops.rs:254 ``` ### ⚠ `hugr-passes` breaking changes ```text --- failure enum_missing: pub enum removed or renamed --- Description: A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely. ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/enum_missing.ron Failed in: enum hugr_passes::non_local::NonLocalEdgesError, previously in file /tmp/.tmp8eoQo1/hugr-passes/src/non_local.rs:57 ``` <details><summary>Changelog</summary> ## `hugr-model` <blockquote> ## [0.23.0](hugr-model-v0.22.4...hugr-model-v0.23.0) - 2025-09-30 ### Bug Fixes - [**breaking**] Appease `cargo-audit` by replacing unmaintained dependencies ([#2572](#2572)) ### New Features - Documentation and error hints ([#2523](#2523)) </blockquote> ## `hugr-core` <blockquote> ## [0.23.0](hugr-core-v0.22.4...hugr-core-v0.23.0) - 2025-09-30 ### Bug Fixes - [**breaking**] Appease `cargo-audit` by replacing unmaintained dependencies ([#2572](#2572)) - *(core)* check extension versions on model import ([#2580](#2580)) - [**breaking**] test extension version compatibility on ModelWithExts ([#2587](#2587)) - *(core)* check used extension versions against resolved extensions ([#2588](#2588)) - [**breaking**] model import loads Package extensions ([#2590](#2590)) ### Miscellaneous Tasks - [**breaking**] Cleanup deprecated definitions ([#2594](#2594)) ### New Features - add trait+funcs for linking Hugrs explicitly by Node ([#2521](#2521)) - Documentation and error hints ([#2523](#2523)) - Allow creating DFG builders from existing hugrs ([#2562](#2562)) - add_input/output for arbitrary DFGBuilders ([#2564](#2564)) - [**breaking**] Return error instead of panicking in DFGWrapper::add_{in,out}put ([#2571](#2571)) - *(core)* inner acccesors for WithGenerator error ([#2583](#2583)) - Normalize CFGs ([#2591](#2591)) ### Refactor - [**breaking**] Replace lazy_static with std::sync::LazyLock ([#2567](#2567)) </blockquote> ## `hugr-llvm` <blockquote> ## [0.23.0](hugr-llvm-v0.22.4...hugr-llvm-v0.23.0) - 2025-09-30 ### Miscellaneous Tasks - [**breaking**] Cleanup deprecated definitions ([#2594](#2594)) ### Refactor - [**breaking**] Replace lazy_static with std::sync::LazyLock ([#2567](#2567)) ### Testing - Add framework for LLVM execution tests involving panics ([#2568](#2568)) </blockquote> ## `hugr-passes` <blockquote> ## [0.23.0](hugr-passes-v0.22.4...hugr-passes-v0.23.0) - 2025-09-30 ### Bug Fixes - DeadCodeElim keeps consumers of linear outputs ([#2560](#2560)) - [**breaking**] Appease `cargo-audit` by replacing unmaintained dependencies ([#2572](#2572)) ### Miscellaneous Tasks - [**breaking**] Cleanup deprecated definitions ([#2594](#2594)) ### New Features - [**breaking**] DeadCodeElimPass reports error on non-existent entry_points ([#2566](#2566)) - Normalize CFGs ([#2591](#2591)) ### Refactor - [**breaking**] Replace lazy_static with std::sync::LazyLock ([#2567](#2567)) </blockquote> ## `hugr-persistent` <blockquote> ## [0.3.0](hugr-persistent-v0.2.3...hugr-persistent-v0.3.0) - 2025-09-30 ### Miscellaneous Tasks - [**breaking**] Cleanup deprecated definitions ([#2594](#2594)) ### Refactor - [**breaking**] Replace lazy_static with std::sync::LazyLock ([#2567](#2567)) </blockquote> ## `hugr` <blockquote> ## [0.23.0](hugr-v0.22.4...hugr-v0.23.0) - 2025-09-30 ### Bug Fixes - DeadCodeElim keeps consumers of linear outputs ([#2560](#2560)) - [**breaking**] Appease `cargo-audit` by replacing unmaintained dependencies ([#2572](#2572)) - *(core)* check extension versions on model import ([#2580](#2580)) - [**breaking**] test extension version compatibility on ModelWithExts ([#2587](#2587)) - *(core)* check used extension versions against resolved extensions ([#2588](#2588)) - [**breaking**] model import loads Package extensions ([#2590](#2590)) ### Miscellaneous Tasks - [**breaking**] Cleanup deprecated definitions ([#2594](#2594)) ### New Features - [**breaking**] DeadCodeElimPass reports error on non-existent entry_points ([#2566](#2566)) - add trait+funcs for linking Hugrs explicitly by Node ([#2521](#2521)) - Documentation and error hints ([#2523](#2523)) - Allow creating DFG builders from existing hugrs ([#2562](#2562)) - add_input/output for arbitrary DFGBuilders ([#2564](#2564)) - [**breaking**] Return error instead of panicking in DFGWrapper::add_{in,out}put ([#2571](#2571)) - *(core)* inner acccesors for WithGenerator error ([#2583](#2583)) - Normalize CFGs ([#2591](#2591)) ### Refactor - [**breaking**] Replace lazy_static with std::sync::LazyLock ([#2567](#2567)) </blockquote> ## `hugr-cli` <blockquote> ## [0.22.3](hugr-cli-v0.22.2...hugr-cli-v0.22.3) - 2025-09-11 ### New Features - *(hugr-cli)* CliError::validate helper ([#2507](#2507)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

acl-cqc added 11 commits September 23, 2025 11:03

Generalize InlineDFG to any HugrNode

b9e2af9

Generalize merge_bbs to any HugrNode

e39db0b

factor out mk_rep2

fbfa4de

Option -> parametrize

26e3668

factor out add_unpack

a571810

Add normalize_cfgs and error/result enums, deprecate merge_basic_blocks

4777809

Combine two entry-elisions, plus some missing edges/ports; revert mk_…

d5ae400

…rep2

Some fixes, docs

c4de50a

Attempt first test == elision. Resulting structure is a PITA to inspect

ad58ea3

Do elision as separate case, w/ take_inputs

43ecfba

Rename cfg->h, outline cfg_node

271fe8a

acl-cqc added 13 commits September 26, 2025 17:12

Test adding some unreachable blocks

80c31a9

Inline unary_unit_sum, sort out imports

02d39eb

Test removing entry block

9242b4c

fix test (and business logic)

6f29698

update numbers of ports

1d4780b

Test removing block before exit

4a9de85

rename add_unpack => wire_unpack_first

32f5e32

common up unpack_before_output (via an extra clone of a typerow)

6f9a7ae

simplify test checks using strings

6f5e595

Convert triple-BB test to use normalize_cfgs

3251918

elide_cfg only tests w/ unreachable entry-predecessor

4fa1f40

docs

3593a66

InlineDFG defaults to Node

6ad9ff4

acl-cqc changed the title ~~feat: (WIP) Normalize CFGs~~ feat: Normalize CFGs Sep 29, 2025

fmt

d60928f

acl-cqc marked this pull request as ready for review September 29, 2025 09:08

acl-cqc requested a review from a team as a code owner September 29, 2025 09:08

acl-cqc requested a review from mark-koch September 29, 2025 09:08

acl-cqc added 5 commits September 30, 2025 09:14

Split apart normalize_cfg.rs, taking all tests with it

3050976

Revert "Split apart normalize_cfg.rs, taking all tests with it"

c2af7cb

This reverts commit 3050976.

Rename module, add deprecated mod merge_bbs with just re-export

9d617b9

fmt

ec0e2f6

Merge remote-tracking branch 'origin/main' into acl/norm_cfg

33086a5

acl-cqc commented Sep 30, 2025

View reviewed changes

acl-cqc requested a review from aborgna-q September 30, 2025 08:57

common up optype_mut's

dedd861

ss2165 mentioned this pull request Sep 30, 2025

chore: release #2554

Merged

aborgna-q approved these changes Sep 30, 2025

View reviewed changes

acl-cqc added 4 commits September 30, 2025 12:25

check Entry has no preds even if succ == exit; remove test

683bcfe

clippy: lifetimes needless after all?

476d276

fixup! check Entry

4006171

Report preserved exit_dfg/entry_dfg

d92aa25

acl-cqc added 3 commits September 30, 2025 13:19

rename module normalize_cfg{=>s}

e8582f6

report num_merged by separating merge_basic_blocks

c41411c

docs

2f92e90

acl-cqc requested a review from aborgna-q September 30, 2025 12:38

aborgna-q approved these changes Sep 30, 2025

View reviewed changes

Ooops, version number

34491eb

acl-cqc enabled auto-merge September 30, 2025 13:28

acl-cqc added this pull request to the merge queue Sep 30, 2025

Merged via the queue into main with commit 3e21fbf Sep 30, 2025
25 of 26 checks passed

acl-cqc deleted the acl/norm_cfg branch September 30, 2025 13:37

hugrbot mentioned this pull request Oct 1, 2025

chore: release #2605

Merged

	#[deprecated(note = "Use normalize_cfgs", since = "0.15.1")]
	#[deprecated(note = "Use normalize_cfgs", since = "0.23.0")]

	#[deprecated(note = "Use version in normalize_cfgs", since = "0.15.1")]
	#[deprecated(note = "Use version in normalize_cfgs", since = "0.23.0")]

feat: Normalize CFGs #2591

feat: Normalize CFGs #2591

Uh oh!

Conversation

acl-cqc commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aborgna-q left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugrbot commented Sep 30, 2025

Uh oh!

aborgna-q left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

acl-cqc commented Sep 26, 2025 •

edited

Loading

codecov bot commented Sep 26, 2025 •

edited

Loading