Skip to content

[Optimization] Scope the graph-decomposition lowering fixpoint to circuit functions - #3216

Open
maliasadi wants to merge 11 commits into
mainfrom
decomp/opt_fixedpoint_lowering
Open

maliasadi wants to merge 11 commits into
mainfrom
decomp/opt_fixedpoint_lowering

Conversation

@maliasadi

@maliasadi maliasadi commented Sep 10, 2026

Copy link
Copy Markdown
Member

Context:
With the new changes to the graph-decomposition system, graph-decomposition applies the solver's chosen rules by iterating decompose-lowering, ctrl-lowering and adjoint-lowering to a fixpoint. Every pass in that loop ran over the whole module. Now that a program module carries the reachable rule closure alongside the QJIT-ed workflow, the rules dominate the module: in a one-gate test circuit (MyOp -> H, gate_set=["H"]) the module holds 125 func.funcs and ~3300 ops, of which the circuit is a handful. decomp:lowering-fixpoint was 16.7 ms of a 19.2 ms decomp:total in PR #3206

Description of the Change:

Adds DecompUtils::getDecompositionRoots that returns the circuits that the decompose-lowering should decompose/rewrite. Every func.func that is not itself a rule will fall back to the module itself when a module holds decomposable gates outside any function.

Benefits:

Per-iteration cost is now proportional to the circuit rather than to the available rule set. On the test above:

before:

  • decomp:lowering-fixpoint 16.7 ms
  • decomp:total 19.2 ms

in this PR:

  • decomp:lowering-fixpoint 2.1 ms
  • decomp:total: 4.2 ms

Possible Drawbacks:

Related GitHub Issues:
[sc-130390]

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.31%. Comparing base (12f40c4) to head (17fc947).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3216   +/-   ##
=======================================
  Coverage   95.31%   95.31%           
=======================================
  Files         177      177           
  Lines       20861    20861           
  Branches     2103     2103           
=======================================
  Hits        19884    19884           
  Misses        771      771           
  Partials      206      206           

☔ 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.

return WalkResult::skip();
}
if (isa<DecomposableGate>(op)) {
gateOutsideFunc = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why would a gate ever appear outside a function?

Comment on lines +63 to +64
module.walk<WalkOrder::PreOrder>([&](Operation *op) {
if (auto func = dyn_cast<func::FuncOp>(op)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you are just collecting all the functions I don't think you would even need a recursive call here, iterating over the module might be enough 🤔

if (failed(runPipeline(ctrlPm, module))) {
return signalPassFailure();
}
if (failed(lowerRoots(hasCtrlRegion, createCtrlLoweringPass))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we lowering adjoint/control after decomposing gates?

Comment on lines +265 to 268
size_t currentOpCount = countOps();
if (currentOpCount == previousOpCount) {
break;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Identical op count is not a guarantee for identical IR 🤔 MLIR already implemented fixed point iteration in their greedy pattern applicator, maybe the instead of invoking nested passes here we should use that

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