Conversation
#962 ("Stop emitting non-local edges") was a temporary hack: guppy threaded every live value through each nested region's input/output signature instead of emitting non-local (inter-graph) edges, because lower parts of the stack could not lower them. That hack was meant to last only until hugr#1234, which has since been fixed via the LocalizeEdges pass (hugr#2237), and the paired backend selene-hugr-qis-compiler 0.4.2 now lowers non-local edges successfully (0.2.10 hard-errored with "Found N nonlocal edges"). This reverts the two semantic changes from #962: - cfg_compiler.choose_vars_for_tuple_sum: only non-copyable vars are passed into the Conditional; copyable values are read directly from the enclosing DFG via non-local edges rather than threaded through the case signatures. - linearity_checker._check_comprehension: only BORROW-kind outer places are threaded through the comprehension loop; copyable used places rely on non-local edges. (The `used_outer_places` field name from #962 is kept; only which places populate it changes.) Partial win on the or-chain short-circuit pattern (hugr#3169): non-local edges are restored for the Conditional/TailLoop cases, cutting O0 `alloca` count ~2.3-3.9x and end-to-end opt-2 compile time ~1.6-2.3x. The dominant O(W^2) cost remains the CFG basic-block signature threading in `compile_bb`, which #962 never touched; de-threading that (emitting Dom/non-local edges for CFG-live copyable values) is a separate follow-up. Ref: Quantinuum/hugr#3169 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Bencher Report| Project | guppylang |
| Branch | ss2165-revert-962-poc-current-stack |
| Testbed | Linux |
Click to view all benchmark results
| Benchmark | hugr_bytes | Benchmark Result bytes x 1e3 (Result Δ%) | Upper Boundary bytes x 1e3 (Limit %) | hugr_nodes | Benchmark Result nodes (Result Δ%) | Upper Boundary nodes (Limit %) |
|---|---|---|---|---|---|---|
| tests/benchmarks/test_big_array.py::test_big_array_compile | 📈 view plot 🚷 view threshold | 66.19 x 1e3(-0.32%)Baseline: 66.41 x 1e3 | 67.07 x 1e3 (98.69%) | 📈 view plot 🚷 view threshold | 4,583.00(-0.26%)Baseline: 4,595.00 | 4,640.95 (98.75%) |
| tests/benchmarks/test_ctrl_flow.py::test_many_ctrl_flow_compile | 📈 view plot 🚷 view threshold | 28.65 x 1e3(-0.42%)Baseline: 28.77 x 1e3 | 29.06 x 1e3 (98.59%) | 📈 view plot 🚷 view threshold | 1,297.00(-0.31%)Baseline: 1,301.00 | 1,314.01 (98.71%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_benchmark_compile | 📈 view plot 🚷 view threshold | 7.80 x 1e3(-0.52%)Baseline: 7.84 x 1e3 | 7.92 x 1e3 (98.49%) | 📈 view plot 🚷 view threshold | 306.00(-1.29%)Baseline: 310.00 | 313.10 (97.73%) |
| tests/benchmarks/test_queue_push_pop.py::test_queue_push_pop_benchmark_compile | 📈 view plot 🚷 view threshold | 10.65 x 1e3(-1.10%)Baseline: 10.77 x 1e3 | 10.88 x 1e3 (97.92%) | 📈 view plot 🚷 view threshold | 407.00(-0.97%)Baseline: 411.00 | 415.11 (98.05%) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2090 +/- ##
=======================================
Coverage 93.21% 93.21%
=======================================
Files 152 152
Lines 14675 14676 +1
=======================================
+ Hits 13679 13680 +1
Misses 996 996 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
acl-cqc
reviewed
Sep 9, 2026
| with dfg.builder.add_conditional(unit_sum, *all_vars_wires) as conditional: | ||
| # Non-copyable types must be passed into the conditional since we can't use | ||
| # inter-graph (non-local) edges to feed them in implicitly. Copyable values are | ||
| # read directly from the enclosing DFG via non-local edges instead of being |
Contributor
There was a problem hiding this comment.
There is an alternative here. Rather than building (inside BB0, that then branches to either BB1 or BB2) a conditional containing Case1 and Case2, we generate two new basic blocks BB1' and BB2', that unconditionally branch to BB1 and BB2. Each of these takes the same inputs, i.e. all of the outputs of BB0 (the union of inputs required by BB1 and BB2 - could still use nonlocal (Dom) edges as both new blocks are dominated by their unique predecessor BB0), and discards those not needed. BB0 then branches to either BB1' or BB2', without needing the Conditional, SumType, or big Tag. BB(1/2)' can also be skipped if BB(1/2) uses all the inputs itself, i.e. generalizing the existing check before the call to choose_vars_for_unit_sum:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts the temporary #962 hack now that qis-compiler (0.4.x) lowers non-local edges again — see Quantinuum/hugr#3169.
Non-copyable places go back on non-local edges instead of being packed through the
Conditional/TailLoopsignatures. Partial win on its own (alloca −2.3–3.9x, opt-2 compile −1.6–2.3x on anor-chain); the CFG block-signature O(W²) is handled in the PR stacked on top.