Skip to content

perf(?): Basic blocks not conditionals when successors need different subsets of outputs - #2311

Draft
acl-cqc wants to merge 2 commits into
mainfrom
acl/adaptor_blocks
Draft

acl-cqc wants to merge 2 commits into
mainfrom
acl/adaptor_blocks

Conversation

@acl-cqc

@acl-cqc acl-cqc commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Not sure whether this is refactor (I mean it changes the Hugr), perf (effect on performance unclear) or feat (it changes the Hugr to reduce the amount of weird stuff??)....

But, presently, when a block in a CFG has successors that need different subsets of the outputs, we build a big SumType abstracting over the differences (actually just over all the nonlinear outputs, not just those ones that are different), to use as the branch predicate; and the predecessor block gains a Conditional which takes all of the nonlinear outputs and then builds one or other variant of that SumType with only the desired values.

Instead, insert "adaptor blocks" inbetween the predecessor and each successor which uses only some of the predecessor's outputs. Successors using all the outputs need no adaptor (when all successors use all the outputs, this was previously special-cased to avoid the Conditional, but this now falls out naturally / generally).

The nasty bit is we have to pass the adaptors up out of compile_bb so we can route the edges correctly in compile_cfg, but even that is just an extra return value, did not need to add anything to e.g. class BB.

@acl-cqc acl-cqc changed the title perf: Basic blocks not conditionals when successors need different subsets of outputs perf(?): Basic blocks not conditionals when successors need different subsets of outputs Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Projectguppylang
Branchacl/adaptor_blocks
TestbedLinux
Click to view all benchmark results
Benchmarkhugr_bytesBenchmark Result
bytes x 1e3
(Result Δ%)
Upper Boundary
bytes x 1e3
(Limit %)
hugr_nodesBenchmark Result
nodes
(Result Δ%)
Upper Boundary
nodes
(Limit %)
tests/benchmarks/test_big_array.py::test_big_array_compile📈 view plot
🚷 view threshold
60.29 x 1e3
(-2.92%)Baseline: 62.10 x 1e3
62.72 x 1e3
(96.12%)
📈 view plot
🚷 view threshold
4,514.00
(-1.55%)Baseline: 4,585.00
4,630.85
(97.48%)
tests/benchmarks/test_ctrl_flow.py::test_many_ctrl_flow_compile📈 view plot
🚷 view threshold
26.41 x 1e3
(-4.85%)Baseline: 27.76 x 1e3
28.03 x 1e3
(94.20%)
📈 view plot
🚷 view threshold
1,182.00
(-9.15%)Baseline: 1,301.00
1,314.01
(89.95%)
tests/benchmarks/test_queue_push_pop.py::test_queue_push_benchmark_compile📈 view plot
🚷 view threshold
6.97 x 1e3
(-3.81%)Baseline: 7.24 x 1e3
7.32 x 1e3
(95.24%)
📈 view plot
🚷 view threshold
286.00
(-7.74%)Baseline: 310.00
313.10
(91.34%)
tests/benchmarks/test_queue_push_pop.py::test_queue_push_pop_benchmark_compile📈 view plot
🚷 view threshold
9.29 x 1e3
(-5.79%)Baseline: 9.86 x 1e3
9.96 x 1e3
(93.28%)
📈 view plot
🚷 view threshold
378.00
(-8.03%)Baseline: 411.00
415.11
(91.06%)
🐰 View full continuous benchmarking report in Bencher

p.id: p for row in bb.sig.output_rows for p in row
}
outputs = sort_vars(list(var_map.values()))
# Can we use 'Dom` edges for droppable outputs? I think no - each output is

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean possibly we could but it would be kinda orthogonal to this

# used by some BB as an actual input, so we must pass them as explicit outputs.
for i, r in enumerate(bb.sig.output_rows):
if len(r) == len(var_map):
assert {p.id for p in r} == {p.id for p in var_map.values()}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe drop this? the len check seems like it should be both cheap and precise

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (e60c3ef) to head (496f3c2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2311      +/-   ##
==========================================
- Coverage   93.37%   93.34%   -0.03%     
==========================================
  Files         165      165              
  Lines       16202    16202              
==========================================
- Hits        15128    15124       -4     
- Misses       1074     1078       +4     

☔ View full report in Codecov by Harness.
📢 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.

@codspeed-hq

codspeed-hq Bot commented Sep 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 11 untouched benchmarks


Comparing acl/adaptor_blocks (496f3c2) with main (c386494)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants