✨ Add Rotation Gate Merging using Quaternions

[J4MMlE]

Description

This PR extends the rotation merging pass in the MQTOpt dialect to support quaternion-based gate fusion. This is the first step toward closing #1029.

The existing rotation merge pass only merges consecutive rotation gates of the same type (e.g., rx + rx or ry + ry) by adding their angles.
This PR introduces quaternion-based merging, which can merge rotation gates of different types (currently only single qubit gates rx, ry, rz, u).

Quaternions are widely used to represent rotations in three-dimensional space and naturally map to qubit gate rotations around the Bloch sphere. The implementation:

1. Converts rotation gates to quaternion representation
2. Multiplies quaternions using the Hamilton product
3. Converts the resulting quaternion back to a u gate. (This could also be done differently in the future, and directly decompose to the correct base gates by using the decomposition from ✨ Implement single-qubit gate decomposition pass #1182)

Since this optimization may only be beneficial on certain quantum architectures, it is disabled by default. It can be invoked using:

mqt-cc --mlir-merge-rotation-gates <input.mlir>

Checklist:

• The pull request only contains commits that are focused and relevant to this change.
• I have added appropriate tests that cover the new/changed functionality.
• I have updated the documentation to reflect these changes.
• I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals.
  - [ ] I have added migration instructions to the upgrade guide (if needed).
• The changes follow the project's style guidelines and introduce no new warnings.
• The changes are fully tested and pass the CI checks.
• I have reviewed my own code changes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[burgholzer]

Hey @J4MMlE 👋🏻
This is great to see!

How much of an ask would it be to directly base this pass on the QCO dialect and its infrastructure?
We'd like to stop adding features to the old dialects and instead only add them to the new ones as much as possible, so that we can remove the old dialects rather sooner than later.
This is not a must, but it would be highly appreciated.

[DRovara]

Thanks a lot @J4MMlE for the effort! The code already looks super clean in my opinion and I like the way you have implemented everything.

I do have quite a few minor comments, but they are largely just related to comments in the code (docstrings/typos).

What I did not look at in much detail is the CMake setup. I guess it would make sense if @burgholzer (after your vacation, of course) could look into that - although probably it would be most efficient to do that only once the tests are ready, too (btw, I think the point from my top-level comment below might also be interesting to consider for you).

Anyways, @J4MMlE, once you have addressed the comments and added the tests, feel free to re-request my review and I will look at the code again. Thanks a lot in the meantime!

[DRovara]

I just remembered one more comment I wanted to make that I forgot:

You talk about how you no longer have to explicitly filter for control qubits due to the new dialect structure.
However, right now, I believe your pass would not work at all with controlled gates anymore - I'm not sure if that's intended.

Imagine the following pseudo-code:

%q0_0, %q1_0 = alloc(2)
[...]
%q0_1, %q1_1 = qco.ctrl(%q0_0), (%q1c_0 = %q1_0) {
  %q1c_1 = qco.u(...) %q1c_0
  qco.yield %q1c_1 
}
%q0_2, %q1_2 = qco.ctrl(%q0_1), (%q1c2_0 = %q1_1) {
  %q1c2_1 = qco.u(...) %q1c2_0
  qco.yield %q1c2_1 
}

Here, the first u gate has qco.yield as its user. However, in short, the snippet above corresponds to:

controlled(q0) u(...) q1
controlled(q0) u(...) q1

so in theory, this can definitely be merged.

Now, I don't know if this is a flaw of the pass (maybe this situation should be checked explicitly), a flaw of the dialect implementation (maybe QCO should provide a way to get the actual successor gate, rather than the yield which we don't care much about), or if it's just out of scope for this pass.

My personal gut feeling is that it would be a nice helper method to implement for the QCO UnitaryOpInterface.

[DRovara]

Thanks a lot @J4MMlE for the work. It looks super good already (for the record, I still haven't looked at the CMake configuration, I'll leave that to someone else).
The tests also look really really clean, I like that a lot.

I did have a minor concern regarding the numerical correctness, maybe you could check that out real quick? Either I am wrong or something is not quite working correctly.

Once my comments are addressed, feel free to remove the "Draft" status from this PR and request a review from Lukas.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added quaternion-based rotation-gate merging with an opt-in CLI flag.

• Tests

  • Added comprehensive unit tests covering merge behavior, numerical correctness, edge cases, and greedy merging.

• Chores

  • Updated build and test targets and updated the changelog.

Walkthrough

Adds a quaternion-based rotation-gate merging pass, exposes a compiler flag and CLI option to enable it, conditionally integrates the pass into the compiler pipeline, updates build/test targets, and adds comprehensive unit tests and changelog entries.

Changes

Cohort / File(s)	Summary
Config & Pass Declarations mlir/include/mlir/Compiler/CompilerPipeline.h, mlir/include/mlir/Dialect/QCO/Transforms/Passes.td, mlir/include/mlir/Dialect/QCO/Transforms/Passes.h	Adds bool mergeRotationGates = false to QuantumCompilerConfig, declares a new MergeRotationGates TableGen pass, and removes a small comment block.
Optimization Implementation mlir/lib/Dialect/QCO/Transforms/Optimizations/QuaternionMergeRotationGates.cpp	New quaternion-based pattern rewrite pass implementing detection and fusion of consecutive RX/RY/RZ/U gates into U gates using quaternion math, Hamilton product, gimbal-lock handling, angle clamping, and GreedyPatternRewriteDriver integration.
Pipeline & CLI Integration mlir/lib/Compiler/CompilerPipeline.cpp, mlir/tools/mqt-cc/mqt-cc.cpp	Includes QCO pass header, conditionally inserts and runs MergeRotationGates in Stage 5 when config_.mergeRotationGates is true; adds CLI flag --mlir-merge-rotation-gates and wires it into QuantumCompilerConfig.
Build System mlir/lib/Compiler/CMakeLists.txt, mlir/lib/Dialect/QCO/Transforms/CMakeLists.txt	Adds MLIRQCOTransforms to compiler target public link libs and simplifies the link specification for the QCO transforms target.
Unit Tests & Test CMake mlir/unittests/Compiler/test_compiler_pipeline.cpp, mlir/unittests/Dialect/QCO/Transforms/CMakeLists.txt, mlir/unittests/Dialect/QCO/Transforms/Mapping/CMakeLists.txt, mlir/unittests/Dialect/QCO/Transforms/Optimizations/CMakeLists.txt, mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quaternion_merge.cpp	Adds pipeline test parameter for merge flag, a new comprehensive unit-test file validating many merge/non-merge scenarios and numerical edge cases, and test CMake additions to build/run the new test target.
Changelog CHANGELOG.md	Adds PR reference #1407 and contributor attribution.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (mqt-cc)
    participant Pipeline as CompilerPipeline
    participant PM as PassManager
    participant QCO as MLIRQCOTransforms

    CLI->>Pipeline: set QuantumCompilerConfig.mergeRotationGates=true/false
    Pipeline->>PM: build Stage 5 passes
    alt mergeRotationGates == true
        Pipeline->>PM: add MergeRotationGates pass
    end
    PM->>QCO: execute MergeRotationGates (pattern rewrites)
    QCO-->>PM: success / failure
    PM-->>Pipeline: continue optimization stages

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• 🗺️ MLIR Dialect Redesign #1264: Adds/wires a QCO rotation-gate merging pass and related pipeline integration; closely related to this pass and pipeline changes.

Suggested reviewers

• burgholzer

Poem

🐰 In quaternion hops I bound and weave,
Merging spins where rotations cleave.
Hamilton hums beneath my paws,
I stitch two gates into one cause.
One U remains — behold my gnaws ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add Rotation Gate Merging using Quaternions' accurately and concisely describes the main feature added in this PR—implementing quaternion-based rotation gate fusion for single-qubit gates.
Description check	✅ Passed	The PR description comprehensively covers the change objectives, implementation approach, usage instructions, testing, and documentation updates. It includes a completed checklist with most items checked, addressing the template requirements.
Docstring Coverage	✅ Passed	Docstring coverage is 85.47% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🤖 Fix all issues with AI agents

In `@mlir/lib/Dialect/QCO/Transforms/CMakeLists.txt`:
- Line 11: The global add_compile_options(-fexceptions) affects all targets;
instead remove that line and add a scoped compile option on the
MLIRQCOTransforms target by calling target_compile_options(MLIRQCOTransforms
PRIVATE -fexceptions) after the MLIRQCOTransforms target is created (e.g., after
the add_library/add_mlir_dialect or similar target definition) so only
MLIRQCOTransforms gets -fexceptions.
- Line 13: Remove the stray debug output in the CMakeLists by deleting or
guarding the message(STATUS "MLIR_DIALECT_LIBS contains: ${dialect_libs}") line;
either remove that message entirely from
mlir/lib/Dialect/QCO/Transforms/CMakeLists.txt or wrap it behind a project-level
debug option (e.g., only call message when a DEBUG_CMAKE or similar variable is
enabled) so normal builds no longer emit the dialect_libs status line.

In `@mlir/lib/Dialect/QCO/Transforms/QuaternionMergeRotationGates.cpp`:
- Around line 363-370: Fix the comment typo on the line above the SelectOp
creations: change "weather" to "whether" in the sentence "// choose correct
alpha and gamma weather safe or not" so it reads "// choose correct alpha and
gamma whether safe or not" (locate the comment immediately preceding the
creation of alpha and gamma via rewriter.create<mlir::arith::SelectOp>).

In `@mlir/unittests/Dialect/QCO/Transforms/CMakeLists.txt`:
- Around line 11-20: Remove or replace the TODO in the target_link_libraries
block for mqt-core-mlir-dialect-qco-transforms-test: either delete the "# TODO
figure out correct dependencies" line or update it to a concrete status/note
(e.g., "Verified dependencies for CI" or list missing deps) so it no longer
suggests unfinished work; verify the linked targets (GTest::gtest_main,
MLIRQCOProgramBuilder, MLIRQCOTransforms, MLIRIR, MLIRPass, MLIRSupport,
LLVMSupport) are intentionally present before committing the change.

In `@mlir/unittests/Dialect/QCO/Transforms/test_qco_quaternion_merge.cpp`:
- Around line 543-577: Move the SCF dialect load into the test fixture SetUp:
add context.loadDialect<scf::SCFDialect>(); inside the
QCOQuaternionMergeTest::SetUp() implementation so all tests consistently load
SCFDialect, and remove the duplicate context.loadDialect<scf::SCFDialect>();
call from the multipleUseInIf test (which currently calls it inside that test
body).
- Around line 136-153: The buildRotations function reads gate.angles[0..2] for
UOp without checking length, risking OOB; update buildRotations to validate the
RotationGate angles vector before calling builder.u by asserting or returning an
error when gate.opName == UOp::getOperationName() and gate.angles.size() < 3 (or
providing safe defaults), e.g. add a size check for gate.angles >= 3 and emit a
clear assertion/log message that includes the offending gate so the caller can
diagnose; reference buildRotations, RotationGate, UOp, gate.angles, and
builder.u when locating the change.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@mlir/lib/Dialect/QCO/Transforms/QuaternionMergeRotationGates.cpp`:
- Around line 136-144: Add a trailing llvm_unreachable() immediately after the
switch that handles RotationAxis to guard against future enum extensions and
silence non-exhaustive-switch warnings; locate the switch on axis (the one
returning quaternion literals for RotationAxis::X/Y/Z in
QuaternionMergeRotationGates.cpp) and append llvm_unreachable("Unhandled
RotationAxis") (and include the proper header <llvm/Support/ErrorHandling.h> if
not already included).
- Around line 309-316: The computed acos input bTemp3 (constructed from ww, zz,
bTemp1, bTemp2) can drift outside [-1,1]; clamp it to the valid domain before
calling mlir::math::AcosOp to avoid NaN. Replace the direct use of bTemp3 in the
AcosOp with a clamped value produced via mlir::arith::MinFOp and
mlir::arith::MaxFOp (or a combination) to bound it to [-1.0, 1.0] using the
existing constants (one, negOne or create negOne if needed); then pass that
clamped value to the creation of beta (rewriter.create<mlir::math::AcosOp>(loc,
clampedValue)). Ensure the new ops reference the same loc and use rewriter so
the IR stays consistent.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@mlir/lib/Dialect/QCO/Transforms/QuaternionMergeRotationGates.cpp`:
- Around line 374-375: The alpha/gamma extraction in
QuaternionMergeRotationGates.cpp can produce angles outside [-PI, PI]; implement
normalization for the variables alpha and gamma (e.g., in the code path that
computes these values inside the QuaternionMergeRotationGates transform/pass) by
mapping them into the principal range [-M_PI, M_PI] using a modular reduction
(fmod/remainder) and ±2*PI adjustment so mathematically equivalent angles are
canonical; update any uses of alpha/gamma in emit/replace logic within the
QuaternionMergeRotationGates routine to use the normalized values.

In `@mlir/unittests/Dialect/QCO/Transforms/test_qco_quaternion_merge.cpp`:
- Around line 702-715: Update the test comment in TEST_F(QCOQuaternionMergeTest,
numericalRotationIdentity) to match the assertion in the test
(expectUGateParams(0, TAU, 0)) rather than claiming U(0, 0, 0) or U(PI, -PI, 0);
reference the actual expected output U(0, TAU, 0) (with TAU = 2π) in the comment
or alternatively change the assertion to expect U(0, 0, 0) if that is the
intended canonical form—locate the test by the name numericalRotationIdentity
and the call expectUGateParams(0, TAU, 0) to make the consistent update.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

TODO: angle normalization is deferred.

The alpha/gamma values can exceed [-π, π] after extraction. This could produce unexpected but mathematically equivalent gate parameters. Worth tracking as a follow-up if downstream consumers expect normalized angles.

Would you like me to open an issue to track the angle normalization TODO?

🤖 Prompt for AI Agents

In `@mlir/lib/Dialect/QCO/Transforms/QuaternionMergeRotationGates.cpp` around
lines 374 - 375, The alpha/gamma extraction in QuaternionMergeRotationGates.cpp
can produce angles outside [-PI, PI]; implement normalization for the variables
alpha and gamma (e.g., in the code path that computes these values inside the
QuaternionMergeRotationGates transform/pass) by mapping them into the principal
range [-M_PI, M_PI] using a modular reduction (fmod/remainder) and ±2*PI
adjustment so mathematically equivalent angles are canonical; update any uses of
alpha/gamma in emit/replace logic within the QuaternionMergeRotationGates
routine to use the normalized values.

[burgholzer]

Normalization seems reasonable here. I believe Qiskit has some information on the periodicity of the U operation in its code. If I remember correctly, it wasn't as simple as 2pi periodicity.

[coderabbitai]

🧩 Analysis chain

🌐 Web query:

Qiskit U gate periodicity parameters theta phi lambda

💡 Result:

In Qiskit, U(θ, φ, λ) (aka the old U3) is defined by the matrix (up to a documented global-phase convention difference vs OpenQASM 2) (quantum.cloud.ibm.com):
[
U(θ,φ,λ)=
\begin{pmatrix}
\cos(\tfrac{θ}{2}) & -e^{iλ}\sin(\tfrac{θ}{2})\
e^{iφ}\sin(\tfrac{θ}{2}) & e^{i(φ+λ)}\cos(\tfrac{θ}{2})
\end{pmatrix}
]
and corresponds to a (R_z(φ)R_y(θ)R_z(λ)) Euler decomposition. (quantum.cloud.ibm.com)

Periodicity (exact matrix equality):

• (φ \sim φ + 2\pi k) (period (2\pi)) because only (e^{iφ}) appears. (quantum.cloud.ibm.com)
• (λ \sim λ + 2\pi k) (period (2\pi)) because only (e^{iλ}) appears. (quantum.cloud.ibm.com)
• (θ) is (4\pi)-periodic as a matrix because (\cos(θ/2),\sin(θ/2)) are (4\pi)-periodic. (quantum.cloud.ibm.com)

Periodicity up to global phase (physically equivalent operation):

• (θ \sim θ + 2\pi) because (U(θ+2\pi,φ,λ) = -U(θ,φ,λ)=e^{i\pi}U(θ,φ,λ)) (just a global phase). (quantum.cloud.ibm.com)

Citations:

• 1: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai
• 2: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai
• 3: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai
• 4: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai
• 5: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai
• 6: https://quantum.cloud.ibm.com/docs/api/qiskit/qiskit.circuit.library.U3Gate?utm_source=openai

---

@burgholzer You're absolutely right! The U gate parameters have different periodicities: φ (phi) and λ (lambda) have 2π periodicity, while θ (theta) has 4π periodicity for exact matrix equality, though θ is 2π-periodic up to global phase.

In this implementation:

• beta (the Y-rotation angle, corresponding to θ) is extracted via acos, which naturally returns values in [0, π], so it doesn't exceed the principal range
• alpha and gamma (the Z-rotation angles, corresponding to φ and λ) have straightforward 2π periodicity and can indeed exceed [-π, π] after the atan2-based extraction

So the TODO comment is appropriately scoped—normalizing alpha and gamma to [-π, π] with modulo 2π is the right approach. The beta parameter is already well-behaved thanks to the acos domain.

---

🧠 Learnings used

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: burgholzer
Repo: munich-quantum-toolkit/core PR: 1330
File: mlir/lib/Dialect/QC/IR/Modifiers/InvOp.cpp:126-139
Timestamp: 2026-02-09T15:19:47.622Z
Learning: In the QC dialect (mlir/lib/Dialect/QC/IR/Modifiers/InvOp.cpp), InvOp::verify intentionally restricts the body region to exactly two operations (one unitary + yield). Multi-op region inversion is by-design deferred to future work, potentially via a dedicated Unitary Operation for grouping gates.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.

---

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.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:1729-1763
Timestamp: 2026-01-10T16:28:41.975Z
Learning: In the QCToQCO conversion pass (mlir/lib/Conversion/QCToQCO/QCToQCO.cpp), the dynamic legality checks for func operations (func::CallOp, func::FuncOp, func::ReturnOp) and scf operations assume that operations use either all classical types or all qubit types, never mixed. Therefore, checking for the presence of qc::QubitType in operands or arguments is sufficient to determine if conversion is needed—there is no need to check both operands and results separately.
<!-- </add_learning]

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Operations/StandardGates/BarrierOp.cpp:45-54
Timestamp: 2025-12-09T00:55:11.926Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux), qubits follow single-user/linear-type semantics where each qubit value can only be consumed once, similar to MQTOpt. This invariant makes it safe to dereference getUsers().begin() in canonicalization patterns like MergeSubsequentBarrier in BarrierOp.cpp, as there will be at most one user per qubit output.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:80-100
Timestamp: 2025-12-08T23:58:09.648Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), quartz.ctrl uses reference semantics and does not return values, unlike flux.ctrl which uses value semantics and returns transformed qubits. When inlining a GPhaseOp in the CtrlInlineGPhase pattern, it's correct to create POp operations for positive controls and erase the CtrlOp without collecting or replacing result values.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1070-1085
Timestamp: 2026-01-07T12:29:16.380Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the ConvertQCOFuncFuncOp pattern assumes that when a func.func operation is matched for conversion, all of its arguments are qco.qubit types (never mixed qubit/classical). The pattern unconditionally converts all arguments to qc::QubitType based on this assumption.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:259-259
Timestamp: 2025-12-17T17:44:31.349Z
Learning: In the QCO dialect (mlir/include/mlir/Dialect/QCO/IR/QCOOps.td), GPhaseOp intentionally uses `MemoryEffects<[MemWrite]>` instead of `Pure` to prevent the remove-dead-values pass from eliminating it. Since GPhaseOp is a zero-target operation with no result values, it would otherwise be removed by DCE, even though it has a meaningful effect on the global quantum state.

---

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.

---

Learnt from: taminob
Repo: munich-quantum-toolkit/core PR: 1206
File: mlir/include/mlir/Passes/Decomposition/GateSequence.h:58-68
Timestamp: 2026-02-06T22:14:18.231Z
Learning: In mlir/include/mlir/Passes/Decomposition/GateSequence.h, the complexity() method intentionally includes a conditional check for hasGlobalPhase() and adds helpers::getComplexity(qc::GPhase, 0) even though it currently returns 0. This is a forward-compatibility design pattern to allow future modification of the complexity calculation for global phase gates without requiring updates to the rest of the codebase.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:988-1024
Timestamp: 2026-01-10T16:07:55.896Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the SCF operation conversion patterns (ConvertQCOScfYieldOp, ConvertQCOScfConditionOp, ConvertQCOScfIfOp, ConvertQCOScfWhileOp, ConvertQCOScfForOp) assume that all operands are qubit types (qco.qubit or qc.qubit), never mixed qubit/classical types. The conversion is scoped to handle all-qubit SCF operations only.

---

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.314Z
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.

[J4MMlE]

I couldn't find any explicit 2PI-periodicity handling in Qiskit. @burgholzer can you point me somewhere if I'm missing it?

Our direct quaternion-to-Euler extraction produces alpha/gamma as sums/differences of two atan2 values, which can land in (-2PI, 2PI]. Wrapping to [-PI, PI] is cosmetic since the U gate is 2PI-periodic in both.

One other normalization which was left out intentionally was the quaternion re-normalization after the Hamilton product (to make sure we are on the unit sphere again). This is unnecessary here because we never chain Hamilton products directly - each merge produces a U-Gate with Euler angles, and the next merge reconstructs a fresh quaternion from cos/sin, which is always unit length.

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[burgholzer]

@J4MMlE @coderabbitai I searched and found where I got this from originally: https://openqasm.com/versions/3.0/language/gates.html#id1

This definition of U has a different global phase from previous versions of the OpenQASM spec. Unfortunately the original definitions were 4π rather than 2π periodic in the θ parameter. A gate U_old(0, ϕ, θ) q; under the previous definition corresponds to U(0, ϕ, θ) q; gphase(-0/2); with the present definition.

So it's a difference in the definition of the U gate.
We are still using the OpenQASM 2 definition at the moment, which is 4pi periodic in the theta parameter.
It may be worth considering updating our definition used throughout MQT Core at some point. However, this has several consequences on equivalences that we currently bake into the library in various places (e.g., U(π/2, 0, π) being a Hadamard gate; which is no longer strictly true with the new definition).
As such, we won't be making changes to the U gate definition as part of this PR. Hence, the theta parameter has to be considered as 4pi periodic.

[coderabbitai]

@burgholzer, thanks for the clarification and the OpenQASM spec link! That's an important distinction to keep in mind.

To summarize for the normalization TODO:

• beta (θ): The acos-based extraction already yields values in [0, π], which is safely within the 4π-periodic range of MQT Core's OpenQASM 2 U gate definition — no normalization needed here.
• alpha/gamma (φ, λ): These are 2π-periodic in both definitions and can exceed [-π, π] as sums/differences of two atan2 values. The TODO for wrapping them to [-π, π] (or [0, 2π)) is still valid but purely cosmetic/canonical.

I'll record this U gate definition distinction as a learning.

---

✏️ Learnings added

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1407
File: mlir/lib/Dialect/QCO/Transforms/Optimizations/QuaternionMergeRotationGates.cpp:370-371
Timestamp: 2026-03-12T15:27:48.347Z
Learning: In MQT Core, the U gate uses the OpenQASM 2 definition, where the theta (θ) parameter is 4π periodic (not 2π periodic as in OpenQASM 3). The OpenQASM 3 definition differs by a global phase factor. Updating to the OpenQASM 3 definition is currently out of scope because it would break existing equivalences baked into the codebase (e.g., U(π/2, 0, π) being a Hadamard gate). This applies throughout MQT Core MLIR and the broader library. The phi (φ) and lambda (λ) parameters remain 2π periodic in both definitions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

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: burgholzer
Repo: munich-quantum-toolkit/core PR: 1330
File: mlir/lib/Dialect/QC/IR/Modifiers/InvOp.cpp:126-139
Timestamp: 2026-02-09T15:19:47.622Z
Learning: In the QC dialect (mlir/lib/Dialect/QC/IR/Modifiers/InvOp.cpp), InvOp::verify intentionally restricts the body region to exactly two operations (one unitary + yield). Multi-op region inversion is by-design deferred to future work, potentially via a dedicated Unitary Operation for grouping gates.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.

---

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.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:1729-1763
Timestamp: 2026-01-10T16:28:41.975Z
Learning: In the QCToQCO conversion pass (mlir/lib/Conversion/QCToQCO/QCToQCO.cpp), the dynamic legality checks for func operations (func::CallOp, func::FuncOp, func::ReturnOp) and scf operations assume that operations use either all classical types or all qubit types, never mixed. Therefore, checking for the presence of qc::QubitType in operands or arguments is sufficient to determine if conversion is needed—there is no need to check both operands and results separately.
<!-- </add_learning]

---

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1550
File: mlir/include/mlir/Dialect/QCO/QCOUtils.h:88-90
Timestamp: 2026-03-11T15:43:26.203Z
Learning: In `mlir/include/mlir/Dialect/QCO/QCOUtils.h`, all utility template functions (e.g., `removeInversePairTwoTargetZeroParameter`, `removeTwoTargetZeroParameterPairWithSwappedTargets`) unconditionally dereference `op.getOutputQubit(N).user_begin()` without a `hasOneUse()` guard. This is safe because QCO dialect qubits are linear types that must be used exactly once by definition, guaranteeing a valid single user always exists. Adding `hasOneUse()` guards is unnecessary and should not be flagged in review.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Operations/StandardGates/BarrierOp.cpp:45-54
Timestamp: 2025-12-09T00:55:11.926Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux), qubits follow single-user/linear-type semantics where each qubit value can only be consumed once, similar to MQTOpt. This invariant makes it safe to dereference getUsers().begin() in canonicalization patterns like MergeSubsequentBarrier in BarrierOp.cpp, as there will be at most one user per qubit output.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:80-100
Timestamp: 2025-12-08T23:58:09.648Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), quartz.ctrl uses reference semantics and does not return values, unlike flux.ctrl which uses value semantics and returns transformed qubits. When inlining a GPhaseOp in the CtrlInlineGPhase pattern, it's correct to create POp operations for positive controls and erase the CtrlOp without collecting or replacing result values.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1070-1085
Timestamp: 2026-01-07T12:29:16.380Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the ConvertQCOFuncFuncOp pattern assumes that when a func.func operation is matched for conversion, all of its arguments are qco.qubit types (never mixed qubit/classical). The pattern unconditionally converts all arguments to qc::QubitType based on this assumption.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:259-259
Timestamp: 2025-12-17T17:44:31.349Z
Learning: In the QCO dialect (mlir/include/mlir/Dialect/QCO/IR/QCOOps.td), GPhaseOp intentionally uses `MemoryEffects<[MemWrite]>` instead of `Pure` to prevent the remove-dead-values pass from eliminating it. Since GPhaseOp is a zero-target operation with no result values, it would otherwise be removed by DCE, even though it has a meaningful effect on the global quantum state.

---

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1283
File: src/qir/runtime/QIR.cpp:196-201
Timestamp: 2025-11-01T15:57:31.153Z
Learning: In the QIR runtime (src/qir/runtime/QIR.cpp), the PRX gate (__quantum__qis__prx__body) is an alias for the R gate (Phased X-Rotation) and should call runtime.apply<qc::R>(theta, phi, qubit), not runtime.apply<qc::RX>() which is a single-parameter rotation gate.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:988-1024
Timestamp: 2026-01-10T16:07:55.896Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the SCF operation conversion patterns (ConvertQCOScfYieldOp, ConvertQCOScfConditionOp, ConvertQCOScfIfOp, ConvertQCOScfWhileOp, ConvertQCOScfForOp) assume that all operands are qubit types (qco.qubit or qc.qubit), never mixed qubit/classical types. The conversion is scoped to handle all-qubit SCF operations only.

---

Learnt from: taminob
Repo: munich-quantum-toolkit/core PR: 1206
File: mlir/include/mlir/Passes/Decomposition/GateSequence.h:58-68
Timestamp: 2026-02-06T22:14:18.231Z
Learning: In mlir/include/mlir/Passes/Decomposition/GateSequence.h, the complexity() method intentionally includes a conditional check for hasGlobalPhase() and adds helpers::getComplexity(qc::GPhase, 0) even though it currently returns 0. This is a forward-compatibility design pattern to allow future modification of the complexity calculation for global phase gates without requiring updates to the rest of the codebase.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1479
File: mlir/lib/Conversion/JeffToQCO/JeffToQCO.cpp:991-1003
Timestamp: 2026-03-06T18:16:42.923Z
Learning: In the JeffToQCO conversion pass (mlir/lib/Conversion/JeffToQCO/JeffToQCO.cpp), do not add func.func/func.call/func.return conversion patterns in the initial version. The design assumes any QCO module produced by this pass contains a single function with no qubit-typed inputs that returns an integer, so qubit-typed helper functions or call-site conversions are not needed in this pass. This is a known temporary limitation — support for qubit-typed helper functions and multi-function modules is expected to be added in a future version.

---

Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1040-1050
Timestamp: 2026-01-07T12:29:02.062Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), ConvertQCOFuncCallOp assumes that if a func::CallOp has qubit results, then all arguments and results are qubits (no mixed classical/quantum types). The conversion is scoped to handle all-qubit function calls only.

---

Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Operations/StandardGates/PhaseOp.cpp:0-0
Timestamp: 2025-12-08T23:44:39.930Z
Learning: In MLIR code under any mlir/ directory, avoid using const qualifiers on core MLIR types in function parameters/signatures (e.g., Value, Type, Attribute, Operation*, Block*, Region*, etc.). This aligns with MLIR's design rationale and should be applied to C++ source files (e.g., .cpp) within mlir/; see https://mlir.llvm.org/docs/Rationale/UsageOfConst/ for details.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:83-86
Timestamp: 2025-12-17T11:32:45.843Z
Learning: In the mlir portion of munich-quantum-toolkit/core, prefer marking free functions as static (static linkage) over placing them in anonymous namespaces, per the clang-tidy rule llvm-prefer-static-over-anonymous-namespace. Do not apply this to type/class definitions; they may continue to use anonymous namespaces. This guideline should be checked across C++ source files under mlir/ (e.g., any free function in LayeredUnit.cpp) to ensure free functions have static linkage, while types/classes can remain in anonymous namespaces.

---

Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1479
File: mlir/lib/Conversion/QCOToJeff/QCOToJeff.cpp:1367-1370
Timestamp: 2026-03-09T02:58:00.657Z
Learning: Guideline: In MLIR conversion code, using rewriter.replaceOpWithNewOp<OtherOp>(oldOp, ...) inserts the new operation before oldOp. For terminator replacements needing a specific end-of-block position, use the idiom: rewriter.setInsertionPointToEnd(block); OtherOp::create(rewriter, loc, ...); rewriter.eraseOp(oldOp). Alternatively, avoid transient two-terminator state by using: rewriter.setInsertionPoint(oldOp); OtherOp::create(rewriter, loc, ...); rewriter.eraseOp(oldOp). This applies broadly to MLIR conversion passes and should be checked for correct insertion behavior when rewriting ops.

[J4MMlE]

Ok @burgholzer how should i deal with this? I think the wrapping into [0, 2π) would still be nice and get rid of the other TODO in the tests (no normalization of results required).

[burgholzer]

for alpha and gamma wrapping into [0, 2pi) or [-pi, pi) seems reasonable. for beta, this is not possible based on the discussion above; but would be possible whenever we switch the definition of the U gate.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mlir/unittests/Compiler/test_compiler_pipeline.cpp`:
- Around line 210-211: The current EXPECT_NE(recordWith.afterOptimization,
recordWithout.afterOptimization) can be true without proving the quaternion
merge actually ran; instead parse the IR snapshots and assert a merge-specific
signal: verify that the run with mergeRotationGates enabled
(recordWith.afterOptimization) contains the merged qco::UOp and that the run
with mergeRotationGates disabled (recordWithout.afterOptimization) still
contains the original qco::RZOp and/or qco::RXOp. Update the test to parse both
afterOptimization strings (or IR representations) and use presence/absence
checks for qco::UOp vs qco::RZOp/qco::RXOp to prove the merge pass executed (see
Stage 5 in CompilerPipeline.cpp / mergeRotationGates for context).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fc16913f-9a02-496c-94e0-38e29f507b14

📥 Commits

Reviewing files that changed from the base of the PR and between cfe9eda and 1a22b2b.

📒 Files selected for processing (1)

• mlir/unittests/Compiler/test_compiler_pipeline.cpp

[burgholzer]

Converting this to a draft while I work on it. Don't want to trigger CodeRabbit to often.

[burgholzer]

Okay. I falsely assumed that this PR was ready for review and only later noticed that most of the comments from the previous review are still open.

Nevertheless, I cleaned up the PR a little bit and addressed some of my own comments along with some of the CodeRabbit comments.
I have also converted this PR back to a draft to avoid it being spammed with CodeRabbit comments.

@J4MMlE please take it from here and address the rest of the review feedback. Please re-request a review once you are done.

[burgholzer]

@J4MMlE based on your comment in #1407 (comment) I believe it makes sense to implement this here.