-
-
Notifications
You must be signed in to change notification settings - Fork 47
✨Implement Unitized Routing Passes #1301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The ProblemThe problem is the following. Given a circuit representation of a quantum program, there is no canonical way of representing these as an MLIR program. For example, module {
%q0_0 = mqtopt.qubit 0
%q1_0 = mqtopt.qubit 1
%q2_0 = mqtopt.qubit 2
%q3_0 = mqtopt.qubit 3
%q4_0 = mqtopt.qubit 4
%q4_0 = mqtopt.qubit 5
%q2_1, %q1_1 = mqtopt.x %q2_0 ctrl %q1_0
%q5_1, %q4_1 = mqtopt.x %q5_0 ctrl %q4_0
%q1_2, %q0_1 = mqtopt.x %q1_1 ctrl %q0_0
%q3_1, %q2_2 = mqtopt.x %q3_0 ctrl %q2_1
}and module {
%q0_0 = mqtopt.qubit 0
%q1_0 = mqtopt.qubit 1
%q2_0 = mqtopt.qubit 2
%q3_0 = mqtopt.qubit 3
%q4_0 = mqtopt.qubit 4
%q4_0 = mqtopt.qubit 5
%q2_1, %q1_1 = mqtopt.x %q2_0 ctrl %q1_0
%q1_2, %q0_1 = mqtopt.x %q1_1 ctrl %q0_0
%q3_1, %q2_2 = mqtopt.x %q3_0 ctrl %q2_1
%q5_1, %q4_1 = mqtopt.x %q5_0 ctrl %q4_0 // moved below.
}represent the same circuit: From a circuit perspective it is pretty easy to see that the
|
|
I must still be missing the point here somehow. You have something like: where Then what you want to create is So you want to create a SWAP using the qubit values But it's highly likely that I am missing something here. |
Yes and no. What you are describing works just fine. That's not the problem. I think I'm just really bad at explaining the problem right now (that was also my conclusion from our last meeting). This pull request is an attempt to practice explaining it. Let me reiterate: Assume we have the circuit discussed above again: If we iterate not the textual IR (top-down walk) but the circuit (via def-use chain) layer-by-layer we would visit the ops in the following order: Consequently we update the current mapping of SSA Values (qubits) the same way. This is handled by the /// ---- Layer 0 ----
layout.updateFor(<gate-0>)
layout.updateFor(<gate-1>)
/// ---- Layer 1 ----
layout.updateFor(<gate-2>)
layout.updateFor(<gate-3>)Now let's assume ...
%q2_1, %q1_1 = mqtopt.x %q2_0 ctrl %q1_0
/// Layer 1
%q5_2, %q3_1 = mqtopt.swap() %q5_1, %q3_0 // Inserted SWAP based on the current SSA values given by the layout.
%q1_2, %q0_1 = mqtopt.x %q1_1 ctrl %q0_0
%q3_2 %q2_2 = mqtopt.x %q3_1 ctrl %q2_1
%q5_1, %q4_1 = mqtopt.x %q5_0 ctrl %q4_0But wait! In the textual order |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Intermediate ThoughtsSolutionIterate the circuit from a circuit perspective via the def-use chain (with the help of the Alternatively one could avoid collecting the ops and iterate the IR top-down layer-by-layer with a SSA Dominance ViolationHere is an example of such a SSA violation after routing. module {
%q0_0 = mqtopt.qubit 0
%q1_0 = mqtopt.qubit 1
%q2_0 = mqtopt.qubit 2
%q3_0 = mqtopt.qubit 3
%q4_0 = mqtopt.qubit 4
%q5_0 = mqtopt.qubit 5
%q2_1, %q1_1 = mqtopt.x %q2_0 ctrl %q1_0
%q5_2, %q3_1 = mqtopt.swap() %q5_1, %q3_0 // Uses %q5_1 which is below in the IR: SSA Violation 🚨
%q1_2, %q0_1 = mqtopt.x %q1_1 ctrl %q0_0
%q3_2 %q2_2 = mqtopt.x %q3_1 ctrl %q2_1
%q5_1, %q4_1 = mqtopt.x %q5_0 ctrl %q4_0
}Fix SSA Dominance Violation1) MLIR Built-InThere is an easy solution for this built into MLIR:
There is just one problem. This is really slow for larger programs.
There is an open pull request for this issue. However, the code base (and the implementation of the topological sort) has changed quite a bit. So this might be "fixed" already. 2) Reorder operations manuallySince the built-in repair function, i.e. By doing so we can move all unitaries of the current layer in front of the anchor of the next. Benchmarks show that this should be the way to go.
The only downside of this approach is that one has to be very careful when dealing with constructs such as |
burgholzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MatthiasReumann! This looks really clean and neatly organised. I just have a bunch of smaller comments that came up while reading through the code. I hope they make sense.
Some are truly micro optimizations, so they might not be that important. But I think none of the comments should take long to incorporate.
While you are at it: would you mind merging in the current main branch and adding a changelog entry for this (or add the PR to an existing changelog entry)?
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
Outdated
Show resolved
Hide resolved
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/QubitIndex.h
Outdated
Show resolved
Hide resolved
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h
Outdated
Show resolved
Hide resolved
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.h
Outdated
Show resolved
Hide resolved
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp (1)
144-150: Avoid potential null dereference when restoring atscf::YieldOpIf a
scf::YieldOpis the first op in its block (e.g., empty loop/if bodies),op->getPrevNode()isnullptr, andsetInsertionPointAfter(nullptr)is invalid. Inserting before the yield is sufficient and avoids the edge case.- .Case<scf::YieldOp>([&](scf::YieldOp op) { - if (unit.restore()) { - rewriter.setInsertionPointAfter(op->getPrevNode()); - insertSWAPs(op.getLoc(), llvm::reverse(history), - unit.layout(), rewriter); - } - }); + .Case<scf::YieldOp>([&](scf::YieldOp op) { + if (unit.restore()) { + rewriter.setInsertionPoint(op); + insertSWAPs(op.getLoc(), llvm::reverse(history), + unit.layout(), rewriter); + } + });mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cpp (1)
48-50: Update pass description to mention layout restoration checksThe implementation now also enforces that units marked with
restore()end with the same layout (atscf::YieldOp), not just that two‑qubit gates are executable. Consider updating the comment to reflect both responsibilities:-/** - * @brief This pass verifies that all two-qubit gates are executable on the - * target architecture. - */ +/** + * @brief This pass verifies that all two-qubit gates are executable on the + * target architecture and that layouts are restored for units marked with + * restore(). + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md(2 hunks)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h(1 hunks)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h(2 hunks)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h(11 hunks)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.h(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp(3 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp(2 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cpp(3 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h:202-214
Timestamp: 2025-10-07T15:58:19.247Z
Learning: In the QMAP routing algorithm (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h), the heuristic function h() intentionally uses a non-admissible heuristic that sums per-gate shortest-path distances. This is a deliberate design choice from the QMAP paper, where admissibility is dropped because locally optimal solutions can negatively affect the overall routing quality. This approach prioritizes global solution quality over guaranteed local optimality.
📚 Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.
Applied to files:
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.hmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/DeadGateEliminationPattern.cpp:42-45
Timestamp: 2025-10-09T13:28:29.237Z
Learning: In the MQTOpt MLIR dialect, linear types enforce single-use semantics where each qubit value can only be consumed once, preventing duplicate deallocations and making RAUW operations safe when replacing output qubits with corresponding input qubits in transformation patterns.
Applied to files:
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.hmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.hmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-10-09T13:14:10.178Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:219-221
Timestamp: 2025-10-09T13:14:10.178Z
Learning: The MQT Core project (munich-quantum-toolkit/core repository) uses the C++20 standard, not C++17. C++20 features such as abbreviated function templates (e.g., `const auto&` parameters) are supported and valid in this codebase.
Applied to files:
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h
📚 Learning: 2025-10-07T15:58:19.247Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h:202-214
Timestamp: 2025-10-07T15:58:19.247Z
Learning: In the QMAP routing algorithm (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h), the heuristic function h() intentionally uses a non-admissible heuristic that sums per-gate shortest-path distances. This is a deliberate design choice from the QMAP paper, where admissibility is dropped because locally optimal solutions can negatively affect the overall routing quality. This approach prioritizes global solution quality over guaranteed local optimality.
Applied to files:
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.hmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cppmlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-11-27T08:52:06.428Z
Learnt from: lsschmid
Repo: munich-quantum-toolkit/qmap PR: 832
File: src/hybridmap/HybridNeutralAtomMapper.cpp:1379-1383
Timestamp: 2025-11-27T08:52:06.428Z
Learning: In src/hybridmap/HybridNeutralAtomMapper.cpp, the getBestMovePos function throws std::runtime_error when no move position is found (finalBestPos.coords is empty). This behavior is acceptable for current use cases since tight architectures are not a concern. A future improvement would be to fall back to SWAP-based strategies instead of throwing.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp
🧬 Code graph analysis (7)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/RoutingVerificationPass.cpp (3)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (6)
isEntryPoint(40-51)isEntryPoint(40-40)isTwoQubitGate(53-56)isTwoQubitGate(53-53)isExecutable(144-145)getIns(58-58)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp (10)
fromEntryPointFunction(135-142)fromEntryPointFunction(135-136)op(52-52)op(52-52)op(55-59)op(55-55)op(62-75)op(62-63)next(275-298)next(275-275)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp (4)
fromEntryPointFunction(32-40)fromEntryPointFunction(33-34)next(57-82)next(57-57)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.h (1)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (7)
nodiscard(33-114)nodiscard(57-61)nodiscard(68-72)nodiscard(135-140)nodiscard(159-163)nodiscard(182-186)nodiscard(205-207)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp (2)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (6)
isEntryPoint(40-51)isEntryPoint(40-40)isTwoQubitGate(53-56)isTwoQubitGate(53-53)isExecutable(144-145)getIns(58-58)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp (4)
fromEntryPointFunction(32-40)fromEntryPointFunction(33-34)next(57-82)next(57-57)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h (2)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h (1)
nodiscard(53-65)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (8)
nodiscard(33-114)nodiscard(57-61)nodiscard(68-72)nodiscard(135-140)nodiscard(159-163)nodiscard(182-186)nodiscard(205-207)swap(231-243)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp (2)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp (12)
fromEntryPointFunction(135-142)fromEntryPointFunction(135-136)op(52-52)op(52-52)op(55-59)op(55-55)op(62-75)op(62-63)layout(137-137)next(275-298)next(275-275)forLayout(283-283)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.h (1)
SequentialUnit(25-44)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (3)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h (1)
nodiscard(53-65)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (7)
nodiscard(33-114)nodiscard(57-61)nodiscard(68-72)nodiscard(135-140)nodiscard(159-163)nodiscard(182-186)nodiscard(205-207)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Architecture.cpp (3)
nodiscard(24-40)nodiscard(42-49)nodiscard(83-86)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (2)
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h (1)
nodiscard(53-65)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Architecture.cpp (3)
nodiscard(24-40)nodiscard(42-49)nodiscard(83-86)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: 🚦 Check
🔇 Additional comments (19)
CHANGELOG.md (1)
23-23: Changelog entry and PR link look consistentDescription, emoji, PR reference, and author formatting match the existing convention; no further changes needed.
Also applies to: 270-270
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Architecture.h (1)
15-15: QubitIndex alias integration looks goodIntroducing
using QubitIndex = uint32_t;here and using it inCouplingSet,NeighbourVector, and public APIs is consistent and improves type clarity without changing behavior.Also applies to: 20-20, 26-30
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/PlacementPass.cpp (1)
54-63: Localized parent-depth constants keep stack semantics clearDefining
FOR_PARENT_DEPTHandIF_PARENT_DEPTHnext tohandleFor/handleIfmatches the existing stack behavior (one and two duplicates, respectively) and makes the intent self-contained; the updated pass brief is also more accurate.Also applies to: 434-435
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Unit.h (1)
19-40: Unit abstraction is minimal and appropriateEncapsulating
Layout, owningRegion*, and therestoreflag inUnitcleanly supports bothSequentialUnitandLayeredUnit; no changes needed here.mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp (1)
83-188: A-based layered routing and dominance repair look consistent*The use of
LayeredUnitplus per-layer sliding windows, local SWAP insertion atlayer.anchor, andmoveOpBeforereordering to repair SSA dominance (combined with history‑based restoration atscf::YieldOp) is coherent and aligns with the unitized design; no functional issues stand out.mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp (1)
121-121:numSwapsis properly declared and requires no changes
numSwapsis correctly defined as aStatisticinPasses.tdfor theNaiveRoutingPassSCpass class. The TableGen-generated code via the#include "mlir/Dialect/MQTOpt/Transforms/Passes.h.inc"macro automatically generates the necessarySTATISTICdeclaration. Adding a manualSTATISTICdefinition as suggested would be redundant and incorrect for MLIR TableGen-based passes.mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp (3)
32-40: LGTM!The
fromEntryPointFunctionimplementation correctly mirrors the pattern used inLayeredUnit::fromEntryPointFunction(see relevant snippets at lines 134-141), ensuring consistency across unit types.
42-55: LGTM!The constructor correctly scans the region for control-flow dividers (
RegionBranchOpInterface) and sets boundaries appropriately.
57-82: LGTM!The
next()method correctly handlesscf::ForOpandscf::IfOpcontrol-flow constructs, with proper layout copying/moving semantics that match theLayeredUnit::next()pattern shown in the relevant code snippets.mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (3)
32-37: LGTM!The migration from
llvm::StringLiteraltomlir::StringLiteralis appropriate sincemlir::StringLiteralis a type alias forllvm::StringLiteralin MLIR's support headers.
98-115: LGTM!The
createSwaphelper is well-structured with clear parameter documentation. The implementation correctly creates a SWAP gate with empty control qubits and parameters.
144-150: LGTM!The precondition assertion for
isTwoQubitGate(op)has been added as suggested in past reviews, ensuring callers are aware of the requirement.mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h (2)
14-24: LGTM!The include of
<mlir/IR/PatternMatch.h>resolves the previously flagged issue aboutPatternRewriternot being declared. While a forward declaration would be more lightweight, the direct include is a valid and simpler approach.
125-148: The crossed use-replacement logic in SWAP insertion is correct.Lines 139-142 implement proper SWAP semantics where
in0 → out1andin1 → out0. This reflects that a SWAP(A, B) gate produces outputs where the quantum state originally inin0is now in output position 1, and vice versa. The subsequentlayout.swap()andremapQubitValue()calls correctly maintain the layout state by updating program index associations while preserving hardware location tracking.mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (5)
109-114: LGTM!Adding
getNumQubits()toThinLayoutis a clean addition that provides access to the layout size without exposing internal vectors.
264-269: LGTM!The use of
llvm::zip_equalcorrectly relies on the 1-1 correspondence guarantee between input and output qubits inUnitaryInterfaceoperations, as documented in the retrieved learnings.
277-286: LGTM!The
remapoverloads forResetOpandMeasureOpcorrectly handle single-qubit remapping.
338-344: LGTM!The
mlir::qualification onDenseMapandSmallVectoris consistent with the style changes throughout the file.
321-327:remapIfResultsassumes IfOp results are ordered by hardware indices.The method zips
getHardwareQubits()(indexed 0...n) withop->getResults().take_front(nqubits), requiring that IfOp results correspond to hardware indices in order. Ensure yield operands in both branches are ordered to match hardware qubit indices when constructing the IfOp.
burgholzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last round of feedback (I promise).
This is almost ready to go in.
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Common.h
Outdated
Show resolved
Hide resolved
mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Router.h
Outdated
Show resolved
Hide resolved
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp
Outdated
Show resolved
Hide resolved
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Lukas Burgholzer <[email protected]> Signed-off-by: matthias <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp (1)
142-147: Avoid potential null dereference by inserting beforescf::YieldOp.
op->getPrevNode()may benullptrwhen the yield is the first op in its block; callingsetInsertionPointAfter(nullptr)is unsafe. Insert the restore SWAPs directly before the yield viasetInsertionPoint(op), which is also the cleaner pattern discussed earlier. Based on learnings, prefersetInsertionPoint(op)here.- .Case<scf::YieldOp>([&](scf::YieldOp op) { - if (unit.restore()) { - rewriter.setInsertionPointAfter(op->getPrevNode()); - insertSWAPs(op.getLoc(), llvm::reverse(history), - unit.layout(), rewriter); - } - }); + .Case<scf::YieldOp>([&](scf::YieldOp op) { + if (unit.restore()) { + rewriter.setInsertionPoint(op); + insertSWAPs(op.getLoc(), llvm::reverse(history), + unit.layout(), rewriter); + } + });Please confirm this matches your intended placement of restore-SWAPs (immediately before the yield).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(2 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp(1 hunks)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.294Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
📚 Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-04T06:59:40.294Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:84-85
Timestamp: 2025-12-04T06:59:40.294Z
Learning: In the MQTOpt MLIR routing passes (NaiveRoutingPassSC, AStarRoutingPassSC), the input IR is guaranteed to contain only 1-qubit and 2-qubit gates. All 3+-qubit gates must be decomposed before routing; otherwise the input IR is invalid. This invariant allows skipTwoQubitBlock in LayeredUnit.cpp to safely assert wires.size() == 2.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-10-07T15:58:19.247Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h:202-214
Timestamp: 2025-10-07T15:58:19.247Z
Learning: In the QMAP routing algorithm (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Planner.h), the heuristic function h() intentionally uses a non-admissible heuristic that sums per-gate shortest-path distances. This is a deliberate design choice from the QMAP paper, where admissibility is dropped because locally optimal solutions can negatively affect the overall routing quality. This approach prioritizes global solution quality over guaranteed local optimality.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cppmlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-10-09T13:20:11.483Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/test/Dialect/MQTOpt/Transforms/lift-measurements.mlir:269-288
Timestamp: 2025-10-09T13:20:11.483Z
Learning: In the MQT MLIR dialect, the `rz` gate should not be included in the `DIAGONAL_GATES` set for the `ReplaceBasisStateControlsWithIfPattern` because its operator matrix does not have the required shape | 1 0 | / | 0 x | for the targets-as-controls optimization. It is only included in `LiftMeasurementsAboveGatesPatterns` where the matrix structure requirement differs.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
🧬 Code graph analysis (1)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp (3)
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/Common.cpp (5)
isEntryPoint(40-51)isEntryPoint(40-40)isTwoQubitGate(53-56)isTwoQubitGate(53-53)isExecutable(144-145)mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/SequentialUnit.cpp (4)
fromEntryPointFunction(32-40)fromEntryPointFunction(33-34)next(57-82)next(57-57)mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h (1)
swap(232-244)
🪛 GitHub Actions: CI
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp
[error] 161-161: use of undeclared identifier 'in1'; did you mean 'in0'?
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
[error] 136-136: use of undeclared identifier 'in1'; did you mean 'in0'?
🔇 Additional comments (1)
CHANGELOG.md (1)
23-23: Changelog entry is well-formatted and accurate.The entry follows the established format conventions: emoji, clear description, PR reference, and author attribution with no issue reference. The PR link is correctly positioned in descending merge-time order. The description accurately reflects the PR's introduction of Unit-based routing abstractions.
Also applies to: 270-270
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/AStarRoutingPass.cpp
Show resolved
Hide resolved
mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp
Show resolved
Hide resolved
burgholzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy, the Rabbit is happy, CI is happy.
Let's go. Nice work 🙏🏼
Description
This pull request implements an "unit"-ized approach to route quantum-classical programs. Furthermore, it solves the issue that the def-use and textual (program) operation order is not that compatible.
Unit
An Unit divides a quantum-classical program into routable sections:
Each unit is routed separately. This PR introduces two modes of traversal for such units:
SequentialUnit: Traverses the ops in the unit sequentially (Naive Routing)LayeredUnit: Traverses the ops layer-by-layer (A* Routing)Changes
--route-naive-sc) from A* routing pass (--route-astar-sc): This makes sense from a logical as well as a pass option perspective. The pass options for the A* routing pass belong to the A* routing pass. If we were to implement another method, let's say SABRE, its options would again reside in a separate pass.SequentialUnitandLayeredUnit.SequentialUnitalso for verificationChecklist: