✨ Add unitary matrix to QCO dialect #1426
Open
Conversation
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
taminob
added
enhancement
8 tasks
Contributor
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds Eigen as a build dependency and implements per-op unitary-matrix accessors across the QCO dialect (TD/headers/ops), updates CMake/FetchContent and clang-tidy settings, and adds unit tests validating the new matrix-extraction APIs. Changes
Sequence Diagram(s)(silent — none generated) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @mlir/include/mlir/Dialect/QCO/IR/QCODialect.h:
- Around line 205-222: The permutate function currently computes undoPermutation
but never fills permutatedMatrix or uses swapMatrix; implement the permutation
by applying the provided permutation vector to reorder rows and/or columns of
inputMatrix (depending on intended semantics) into permutatedMatrix and compute
undoPermutation as already started; specifically, iterate over indices i from 0
to dim-1, set permutatedMatrix.row(i) = inputMatrix.row(permutation(i)) (or
column variant if columns should be permuted), keep
undoPermutation(permutation(i)) = i, and remove the unused swapMatrix or use
utils::getMatrixSWAP if a SWAP-based transformation is required; ensure
assertions remain and return {permutatedMatrix, undoPermutation} once populated.
- Around line 192-203: The helper tryGetParameterAsDouble currently creates a
DummyArityType to call
TargetAndParameterArityTrait::Impl<arith::ConstantOp>::getStaticParameter;
refactor by extracting getStaticParameter into a standalone utility function
(e.g., getStaticParameterForConstantOp or getStaticParameterFromAttr) and have
tryGetParameterAsDouble call that utility directly, removing the DummyArityType
alias; update the declaration/definition of the new utility in the same
header/namespace as TargetAndParameterArityTrait and replace usages of
DummyArityType::Impl<arith::ConstantOp>::getStaticParameter with the new
function.
- Line 35: The file currently includes an internal Eigen header path
("unsupported/Eigen/src/KroneckerProduct/KroneckerTensorProduct.h") which is
fragile; replace that internal src include with the public unsupported Eigen
header (for example use the public header under unsupported/Eigen, e.g.
<unsupported/Eigen/KroneckerProduct> or the corresponding public
KroneckerProduct header) so the include is stable across Eigen updates and
remove the direct reference to the src directory.
- Around line 230-236: The loop over region incorrectly returns early when a
cast to mlir::qco::UnitaryOpInterface succeeds: in the block containing the
dyn_cast to UnitaryOpInterface (variable unitaryOp) the code returns result
before calling unitaryOp.getUnitaryMatrix(), preventing any unitary processing;
fix by inverting the condition so you return only when the cast fails (i.e., if
(!unitaryOp) continue/return as intended) and then call
unitaryOp.getUnitaryMatrix() and handle/update result; adjust the loop around
the dyn_cast, getUnitaryMatrix(), and any result accumulation accordingly in the
code paths referencing UnitaryOpInterface and result.
In @mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:
- Line 277: The pattern in operations like GPhaseOp (and RXOp, RYOp, etc.)
unsafely calls tryGetParameterAsDouble(op, 0).value(), which throws if the
parameter is not a compile-time constant; change getUnitaryMatrix generation to
first check the optional (e.g., using has_value()/operator bool or
tryGetParameterAsDouble return check) and handle the dynamic-parameter case
instead of unwrapping: either assert/emit a clear error when a static value is
required, document that getUnitaryMatrix only supports static parameters, or
make getUnitaryMatrix return an error/Optional/MaybeMatrix for non-static
parameters; update the usages that call utils::getMatrixGPhase(...) (and
equivalent getMatrix* helpers) to accept the guarded/optional path so no
.value() is called without a prior check.
In @mlir/include/mlir/Dialect/Utils/MatrixUtils.h:
- Around line 289-292: The current indexing for placing the target block uses
matrix(dim - targetDim - j, dim - targetDim - i) which writes the block reversed
and in the wrong position; update the placement to use matrix(dim - targetDim +
j, dim - targetDim + i) so the targetMatrix(j, i) maps to the bottom-right block
(rows/cols [dim-targetDim, dim)), or replace the loop with Eigen's block API by
assigning matrix.bottomRightCorner(targetDim, targetDim) = targetMatrix for a
clearer, less error-prone fix; locate the assignment inside the function that
builds the controlled gate matrix and update that code path accordingly.
- Line 13: MatrixUtils.h currently includes QCODialect.h, creating a circular
include with QCODialect.h; remove the direct include and either forward-declare
the minimal QCO symbols you need (e.g., declare namespace mlir::qco { class
QCODialect; } or forward-declare specific types used) inside MatrixUtils.h, or
move the dialect-dependent utility functions out of MatrixUtils (or into a new
file) so MatrixUtils.h stays dialect-agnostic; update any implementations that
relied on the QCODialect include to include QCODialect.h in their .cpp or in the
new dialect-specific file instead.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.clang-tidyCMakeLists.txtmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/lib/Dialect/QCO/IR/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.
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.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
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.
📚 Learning: 2025-11-03T23:09:26.881Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.
Applied to files:
CMakeLists.txtmlir/lib/Dialect/QCO/IR/CMakeLists.txt
📚 Learning: 2026-01-04T23:01:47.734Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: mlir/CMakeLists.txt:13-15
Timestamp: 2026-01-04T23:01:47.734Z
Learning: In the munich-quantum-toolkit/core repository's MLIR directory (mlir/CMakeLists.txt), the MSVC compile definition `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` is necessary to suppress warnings triggered by MLIR/LLVM internal headers, not because the project code uses non-floating complex types. This suppression is needed to avoid warnings from third-party MLIR/LLVM header includes.
Applied to files:
CMakeLists.txtmlir/lib/Dialect/QCO/IR/CMakeLists.txt
📚 Learning: 2026-01-04T21:41:30.252Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: cmake/SetupMLIR.cmake:32-33
Timestamp: 2026-01-04T21:41:30.252Z
Learning: In the munich-quantum-toolkit/core repository, the project provides its own pre-built LLVM/MLIR distributions that are compiled with LLVM_ENABLE_RTTI=ON and LLVM_ENABLE_EH=ON. Users are directed to these distributions via the upgrade guide and installation instructions to avoid ABI compatibility issues. The mid-term goal is to eliminate RTTI and exception handling dependencies entirely.
Applied to files:
CMakeLists.txtmlir/lib/Dialect/QCO/IR/CMakeLists.txt
📚 Learning: 2025-12-14T16:51:52.504Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: .readthedocs.yaml:13-18
Timestamp: 2025-12-14T16:51:52.504Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, LLVM and MLIR toolchains are required for the documentation build because `uv run` includes a full build of the package, which compiles C++/MLIR extensions using scikit-build-core.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-12-05T15:54:56.385Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:19-20
Timestamp: 2025-12-05T15:54:56.385Z
Learning: The munich-quantum-toolkit/core-plugins-catalyst project does not support Windows builds, so MSVC-specific compiler options or cross-platform compatibility with MSVC is not required.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-01-04T23:54:33.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: src/qdmi/na/CMakeLists.txt:179-184
Timestamp: 2026-01-04T23:54:33.540Z
Learning: In the munich-quantum-toolkit/core repository, MSVC link-time code generation (LTCG) must be disabled for the NA device dynamic library target (mqt-core-qdmi-na-device-dyn) on Windows to avoid link errors and missing symbols. This issue affects both release-only builds and debug/release mixed builds, not just debug/release mixing scenarios.
Applied to files:
CMakeLists.txt
📚 Learning: 2025-12-05T15:57:39.701Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 3
File: lib/Conversion/CatalystQuantumToMQTOpt/CMakeLists.txt:22-25
Timestamp: 2025-12-05T15:57:39.701Z
Learning: The munich-quantum-toolkit projects (core and core-plugins-catalyst) use `file(GLOB_RECURSE ...)` patterns in CMakeLists.txt files to collect header files, following an established convention in the parent repository for consistency across the codebase.
Applied to files:
CMakeLists.txtmlir/lib/Dialect/QCO/IR/CMakeLists.txt
📚 Learning: 2025-12-28T17:14:53.890Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1403
File: src/qdmi/na/CMakeLists.txt:31-38
Timestamp: 2025-12-28T17:14:53.890Z
Learning: In the munich-quantum-toolkit/core repository, the NA device generator target (mqt-core-qdmi-na-device-gen) is intentionally propagated to MQT_CORE_TARGETS via list(APPEND) because it's publicly linked to the NA device library (the NA device uses a public function from the generator). The SC device generator is not propagated because it has no such dependency with the SC device library.
Applied to files:
CMakeLists.txt
📚 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/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-07T12:29:16.380Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-07T12:29:02.062Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-17T17:44:31.349Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-08T23:58:09.648Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-09T00:55:11.926Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
mlir/lib/Dialect/QCO/IR/CMakeLists.txt
📚 Learning: 2025-12-17T11:32:53.683Z
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:53.683Z
Learning: In the mlir directory of the munich-quantum-toolkit/core repository, the clang-tidy configuration (mlir/.clang-tidy) prefers marking free functions as `static` over placing them in anonymous namespaces. The configuration enables `llvm-prefer-static-over-anonymous-namespace` and disables `misc-use-anonymous-namespace`. Types/classes should still use anonymous namespaces, but free functions should use explicit `static` linkage.
Applied to files:
.clang-tidy
📚 Learning: 2025-12-22T01:25:21.609Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Applied to files:
.clang-tidy
📚 Learning: 2025-12-17T17:44:21.624Z
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:21.624Z
Learning: In mlir/include/mlir/Dialect/QCO/IR/QCOOps.td (QCO dialect), ensure GPhaseOp declares MemoryEffects<[MemWrite]> rather than Pure. This op has no results and is a zero-target operation, so using Pure would lead to its removal by dead-code elimination. By marking it with MemWrite, review ensures DCE preserves the operation because it has a meaningful effect on the global quantum state. This guidance applies when reviewing or updating QCO ops in this file (and broadly to similar zero-target ops with side effects in QCO).
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 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/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.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/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-08T14:55:43.899Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-100
Timestamp: 2025-12-08T14:55:43.899Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), GPhaseOp is a zero-target operation (global phase). When a CtrlOp wraps a GPhaseOp, it only has control qubits and no targets. The CtrlInlineGPhase canonicalization pattern correctly produces outputs only for the positive controls, not targets.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-11-01T15:57:31.153Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 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/QCO/IR/QCOOps.td
📚 Learning: 2025-12-08T12:44:05.883Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:60-70
Timestamp: 2025-12-08T12:44:05.883Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The RemoveTrivialCtrl pattern correctly only checks getNumPosControls() when determining if a CtrlOp should be removed.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.hmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-08T23:16:20.680Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-101
Timestamp: 2025-12-08T23:16:20.680Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The CtrlInlineGPhase canonicalization pattern correctly only checks getNumPosControls() and processes only positive controls when inlining a GPhaseOp.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.tdmlir/include/mlir/Dialect/Utils/MatrixUtils.h
🧬 Code graph analysis (1)
mlir/include/mlir/Dialect/Utils/MatrixUtils.h (1)
mlir/include/mlir/Dialect/QCO/IR/QCODialect.h (2)
mlir(58-179)mlir(190-222)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🐍 Test (macos-15) / 🐍 macos-15
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Debug) / 🍎 macos-15 clang Debug
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🐍 Lint / 🚨 Lint
🔇 Additional comments (11)
.clang-tidy (1)
77-78: LGTM!Appropriate configuration to exclude Eigen headers from
misc-include-cleaneranalysis. This prevents clang-tidy noise from third-party header-only library includes.CMakeLists.txt (2)
107-110: Verify target architecture detection for cross-compilation.The condition uses
CMAKE_HOST_SYSTEM_PROCESSORwhich detects the build machine's architecture, not the target architecture. For cross-compilation scenarios (e.g., building ARM64 binaries on x64), this should useCMAKE_SYSTEM_PROCESSORinstead.🔧 Suggested fix
- if(WIN32 AND ${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL ARM64) + if(WIN32 AND CMAKE_SYSTEM_PROCESSOR STREQUAL "ARM64")
101-106: LGTM - Eigen dependency setup.Fetching Eigen 3.4.1 via FetchContent with shallow clone is appropriate. Version 3.4.1 is the latest stable patch in the 3.x series with no known security advisories.
mlir/lib/Dialect/QCO/IR/CMakeLists.txt (1)
24-28: LGTM!Adding
Eigen3::Eigenas a PUBLIC dependency is correct since the QCO dialect headers expose Eigen types (e.g.,Eigen::MatrixXcdin the interface).mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.td (3)
93-103: LGTM!Updated convenience methods correctly use the
$_opproxy for accessing operation properties.
113-123: Verify error message string construction.The error message concatenation
"Operation '" + $_op.getBaseSymbol() + "' has no..."may not compile correctly sincegetBaseSymbol()returnsStringRef, which doesn't directly supportoperator+withconst char*. Consider usingllvm::Twineorstd::stringconversion.🔧 Suggested fix
[{ if constexpr (requires { $_op.getUnitaryMatrixDefinition(); }) { return $_op.getUnitaryMatrixDefinition(); } else { - llvm::reportFatalUsageError("Operation '" + $_op.getBaseSymbol() + "' has no unitary matrix definition!"); + llvm::reportFatalUsageError(llvm::Twine("Operation '") + $_op.getBaseSymbol() + "' has no unitary matrix definition!"); } }]
126-131: Verify whether$_opis substituted inextraTraitClassDeclaration.In MLIR's TableGen,
$_opsubstitution is standard inInterfaceMethodbodies, but its availability inextraTraitClassDeclarationis unclear. This context receives C++ code that gets injected into the trait class, where methods would typically access the operation viathis. If$_opis not substituted here, the code will fail to compile. Consider usingstatic_cast<ConcreteType*>(this)->getUnitaryMatrixDefinition()or verifying that TableGen performs substitution in trait class declarations.mlir/include/mlir/Dialect/QCO/IR/QCOOps.td (2)
241-250: LGTM - Well-designed TableGen infrastructure for matrix definitions.The
MatrixDefinitionLambdaandFixedSizeMatrixDefinitionLambdaclasses provide a clean abstraction for generating type-safe matrix definition lambdas. The conditional code generation (line 243) correctly handles operations without matrix definitions.
1057-1066: Verify CtrlOp matrix generation correctness.The matrix definition relies on
getBlockMatrixandgetCtrlBody, both of which have implementation issues flagged inQCODialect.h. Ensure those are resolved for this to work correctly.mlir/include/mlir/Dialect/Utils/MatrixUtils.h (2)
26-163: LGTM - Single-qubit gate matrices.The matrix definitions follow standard quantum computing conventions and use proper complex number representations with
std::complex_literals.
165-272: LGTM - Two-qubit gate matrices.The two-qubit gate matrix implementations (SWAP, iSWAP, DCX, ECR, and parameterized gates) are correctly defined.
taminob
force-pushed
the
taminob/mlir-qco-unitary-matrix-definition
branch
from
4a4823a to
bb53604
Compare
burgholzer
requested changes
Jan 11, 2026
Member
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Hey @taminob 👋🏼
Consider this a quick first batch of feedback on the current implementation. Take everything with a grain of salt; I know that this is only an incomplete draft so far. Some of the comments might be obvious to you anyway and I am sure you planned to address some of them yourself.
Overall, this is heading in the right direction. Thanks for pushing this forward. The trait solution is starting to look rather nice.
I think the main considerations are:
- cleaning up the setup + a proper merge with main
- optimizing the implementation of the
MatrixUtilsmodule - figuring out the "
constexprsituation" or how to more directly provide compile-time-known matrices.
And, obviously, this will need some more tests eventually, but we'll get there 😌
CMakeLists.txt
Outdated
Comment on lines
122
to
132
|
|
||
| FetchContent_Declare( | ||
| Eigen | ||
| GIT_REPOSITORY https://gitlab.com/libeigen/eigen.git | ||
| GIT_TAG 3.4.1 | ||
| GIT_SHALLOW TRUE) | ||
| FetchContent_MakeAvailable(Eigen) | ||
| if(WIN32 AND ${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL ARM64) | ||
| message(STATUS "Enabling non-optimal vectorization in Eigen to avoid alignment issues") | ||
| add_compile_definitions(EIGEN_DONT_ALIGN_STATICALLY) | ||
| endif() |
Member
There was a problem hiding this comment.
This should be moved to the cmake/ExternalDependencies file like all other external dependencies.
Also: the latest release of Eigen is 5.0.1 which is two major versions newer than what is being used here. Please use the latest version.
Also try to follow the remaining external dependencies and add a commit SHA instead of a tag; with the appropriate comment.
Lastly: do we really need the EIGEN_DONT_ALIGN_STATICALLY here? could you remove it temporarily so that we see the CI failure (if any).
Collaborator
Author
There was a problem hiding this comment.
Done, let's see if the Windows ARM CI fails
| */ | ||
| template <size_t T, size_t P> class TargetAndParameterArityTrait { | ||
| template <size_t T, size_t P, typename UnitaryMatrixType, | ||
| UnitaryMatrixType (*MatrixDefinition)(UnitaryOpInterface)> |
Member
There was a problem hiding this comment.
UnitaryMatrixType (*MatrixDefinition)(UnitaryOpInterface)>
could this be a llvm::function_ref?
Collaborator
Author
There was a problem hiding this comment.
I'll check it out, but it can't be a std::function because it is a template parameter
Collaborator
Author
There was a problem hiding this comment.
Doesn't seem to be possible - the allowed types for a template parameter are pretty restrictive
Member
There was a problem hiding this comment.
👍🏼 just searched a little bit on the internet and it seems that the template parameter (like it is) is the preferred solution for this anyway.
Maybe briefly looking at https://iifx.dev/en/articles/457550510/boosting-c-performance-from-std-function-to-template-parameters could be helpful. The Callable&& callback pattern paired with std::forward seems like a reasonable approach.
Collaborator
Author
There was a problem hiding this comment.
I think your link is about something completely different. In this PR, a new template instance is actually created for each function because not the "callable" type is part of the template parameters but the "callable" itself (function pointer). So basically, each operation has its own, custom trait.
In an earlier version, I tried to avoid this by having a separate trait UnitaryMatrixTrait, but I wasn't able to make it work with two native traits in a single tablegen def and I wanted to avoid having to specify for each operation the number of targets twice. Or do you think that's the lesser evil?
Collaborator
Author
There was a problem hiding this comment.
Since we have the concrete instance of the operation in the interface, we can also simply add the method declaration in extraClassDeclaration and then add the implementation in a corresponding source file (which seems to already exist for most (maybe all) operations). Would simplify the code generation by a lot, but would also require touching multiple files for adding new operations (although we'll probably rarely add new gates, right?). Should I rewrite the PR to use that approach?
It's definitely easier to understand than the current solution, although it's less cool and doesn't use the capabilities of tablegen as nicely :)
Member
There was a problem hiding this comment.
I think we have source code files for every single operation already, so this feels like an easy choice.
I get your point about the nice capabilities of tablegen ;o)
I don't think that having to touch multiple files isn't too bad. As you said, we'll rarely add new gates anyway.
And the whole purpose of the dedicated Operation files is to place code that is related to the individual operations there.
Overall, this seems like a good way forward.
Collaborator
Author
There was a problem hiding this comment.
I implemented a draft for this, do you want to check if that's better? I guess it definitely is cleaner than the previous approach and also offers more flexibility (only wrap in std::optional if it is not a static matrix).
Do we also need this feature for the QC dialect? Because then it could actually be benefitial to have the MatrixUtils.h file (although I don't want to move it back again for each operation :( )
Member
There was a problem hiding this comment.
I'll check.
I don't think we particularly need the matrix features for the QC dialect for now. It's fine to keep them isolated to the QCO dialect for now.
Member
There was a problem hiding this comment.
Okay. Just briefly looked through it and this seems to look very clean.
I need to play with the code a little bit once the PR is in a pretty much final state, but this is definitely going in the right direction and the code rabbit comments should point you in the right direction already.
Comment on lines
146
to
147
| const auto* op = this->getConstOperation(); | ||
| return MatrixDefinition(llvm::dyn_cast<UnitaryOpInterface>(op)); |
Member
There was a problem hiding this comment.
Based on https://mlir.llvm.org/docs/Rationale/UsageOfConst/, I would not bother too much with const and the respective casting.
Makes the code here a bit simpler.
Collaborator
Author
There was a problem hiding this comment.
Ah, thanks for the link - I read in the MLIR documentation somewhere that the const should be generated if applicable. It's a bit annoying, because e.g. FuncOp::getOps<OpType> returned a const mlir::Operation* in a test case for me, requiring a workaround via FuncOp::getOperations().
Member
There was a problem hiding this comment.
I think I might have encountered similar needs for workarounds due to const. The better solution in almost all those cases was to remove the const, which seems to fit well with the general recommendation from the link.
Collaborator
Author
There was a problem hiding this comment.
For me, using loops instead of the -> operator on the iterators actually resolved the issue (although I'm not sure why)
8 tasks
burgholzer
added a commit
that referenced
this pull request
Jan 12, 2026
## Description This PR reduces the current code duplication of `getStaticParameter` which is used in both `QC` and `QCO`. It additionally adds the conversion to a standard C++ double type since there is no other use of the function in the code base so far. An alternative to the currently returned `std::optional` would be a `llvm::FailureOr<double>` which is simply a subclass of `std::optional`. However, it is in my opinion slightly more annoying to use since it requires `llvm::succeeded` and `llvm::failed` to check the result instead of the implicit conversion to bool available in `std::optional`. This is meant to make it slightly safer to use, but makes the code also more verbose and less convenient which is why I decided against using it. `std::optional` is an established standard C++ tool and people usually know how to handle it correctly which cannot be said for `llvm::FailureOr<double>`. Required for #1426 ## Checklist: <!--- This checklist serves as a reminder of a couple of things that ensure your pull request will be merged swiftly. --> - [ ] 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. --------- Signed-off-by: Tamino Bauknecht <[email protected]> Co-authored-by: Lukas Burgholzer <[email protected]>
taminob
force-pushed
the
taminob/mlir-qco-unitary-matrix-definition
branch
from
bb53604 to
d95f625
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td (1)
1007-1037: AddgetUnitaryMatrix()declaration to BarrierOp.
BarrierOpimplementsUnitaryOpInterfacebut lacks the requiredgetUnitaryMatrix()method, unlike all other unitary operations. The interface will fail at runtime if this method is invoked. For a variadic-qubit barrier that applies identity transformation, declare it to returnstd::optional<Eigen::MatrixXcd>.
🤖 Fix all issues with AI agents
In @cmake/ExternalDependencies.cmake:
- Around line 25-32: The FetchContent call for Eigen inside the
BUILD_MQT_CORE_MLIR guard immediately invokes FetchContent_MakeAvailable(Eigen)
instead of appending Eigen to the FETCH_PACKAGES list like other deps, which
breaks the deferred fetch pattern and skips system fallback; either revert to
the deferred pattern (append "Eigen" to FETCH_PACKAGES and remove the direct
FetchContent_MakeAvailable call) or, if Eigen must be available early, add a
concise comment explaining why immediate availability is required; also add
FIND_PACKAGE_ARGS ${EIGEN_VERSION} to the FetchContent_Declare for Eigen so the
CMake flow can attempt a system Eigen fallback using the declared version via
the EIGEN_VERSION variable.
In @CMakeLists.txt:
- Line 26: Remove the redundant include(FetchContent) from CMakeLists.txt
because FetchContent is already included by ExternalDependencies.cmake; keep
only the include of ExternalDependencies.cmake and delete the standalone
include(FetchContent) line, then run a quick CMake configure to verify no
regressions.
In @mlir/include/mlir/Dialect/QCO/IR/QCODialect.h:
- Around line 169-171: The function getControlledOp performs an unsafe
llvm::cast<CtrlOp>(op) which will crash if op is not a CtrlOp; change it to
perform a safe llvm::dyn_cast<CtrlOp>(op) and check the result (return a
null/optional or handle the error) or add a clear assertion message documenting
the precondition; alternatively, if you want to keep the unchecked behavior,
rename getControlledOp to something explicit like getBodyUnitaryFromCtrl to
signal it requires a CtrlOp. Ensure references to CtrlOp, UnitaryOpInterface,
and the cast are updated accordingly and that callers handle the
null/failed-case when using dyn_cast.
In @mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.td:
- Line 31: The TODO "TODO: fix const correctness?" in QCOInterfaces.td must be
resolved or tracked: either remove the comment and create a tracked issue, or
audit and fix const-correctness in the TableGen interface definitions in this
file (QCOInterfaces.td) — ensure methods, properties, and function-like
declarations use correct const qualifiers in their signatures and return types
consistent with the expected immutability, update any TableGen records or
interface methods that wrongly omit const, and if you choose tracking, replace
the TODO with a brief comment containing an issue/PR number referencing the
created tracker.
- Around line 147-150: The visitor used in the std::visit call does not
explicitly handle std::nullopt_t and relies on implicit conversions for Eigen
types; update the visitor lambda (the auto&& visitor used with std::visit on
matrix) to have explicit overloads: one overload taking std::nullopt_t that
returns std::optional<MatrixType>{std::nullopt} and another template/overload
for Eigen matrix-like types that constructs and returns
std::optional<MatrixType>{ MatrixType(m) } (i.e., explicitly convert m into
MatrixType rather than returning m directly); then use this visitor in the
existing std::visit call so nullopt is unambiguous and Eigen conversions are
explicit.
- Around line 151-155: The else branch uses std::get<MatrixType>(matrix) which
will throw std::bad_variant_access with no contextual info; replace it with a
safe check using std::get_if<MatrixType>(&matrix) (or wrap std::get in a
try/catch) so you can emit a clear error that includes the expected MatrixType
and the actual variant alternative (or call llvm::report_fatal_error with a
descriptive message). Update the return path in the same block that currently
references matrix/MatrixType to use the safe getter and construct a helpful
error message on mismatch.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.clang-tidyCMakeLists.txtcmake/ExternalDependencies.cmakemlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
🧰 Additional context used
🧠 Learnings (39)
📓 Common learnings
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.
📚 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/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-10T18:49:44.352Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-10T16:28:41.975Z
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]
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-07T12:29:16.380Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-12T12:05:47.463Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-17T17:44:31.349Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-10T16:07:55.896Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-07T12:29:02.062Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-08T23:44:44.157Z
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:44.157Z
Learning: In MLIR code (mlir/ directories), avoid using const qualifiers on core MLIR data types like Value, Type, Attribute, Operation*, Block*, Region*, etc. in function parameters and signatures. MLIR's design rationale explicitly discourages const for these types. See https://mlir.llvm.org/docs/Rationale/UsageOfConst/ for details.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-10T17:31:22.110Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:816-862
Timestamp: 2026-01-10T17:31:22.110Z
Learning: In mlir/lib/Conversion/QCOToQC/QCOToQC.cpp, when creating scf::IfOp with `rewriter.create<scf::IfOp>(loc, types, condition, false)`, passing `false` for the withElseRegion parameter creates an else region that exists but contains no blocks. Therefore, inlineRegionBefore can be called on the else region without checking if it exists, and there's no need to erase an auto-created empty block from the else region (unlike the then region, which does get an auto-created empty block that must be erased).
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-14T17:02:02.997Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp:187-210
Timestamp: 2025-12-14T17:02:02.997Z
Learning: In the Flux dialect builder (mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp), the coding style relies on implicit conversion from Value to ValueRange in ctrl() calls (e.g., `ctrl(control, {}, ...)` instead of `ctrl(ValueRange{control}, ValueRange{}, ...)`). This pattern is used consistently throughout all macro-generated methods and should be maintained for consistency.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2025-12-08T12:44:05.883Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:60-70
Timestamp: 2025-12-08T12:44:05.883Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The RemoveTrivialCtrl pattern correctly only checks getNumPosControls() when determining if a CtrlOp should be removed.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.td
📚 Learning: 2025-12-08T23:58:09.648Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 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/include/mlir/Dialect/QCO/IR/QCOInterfaces.td
📚 Learning: 2025-12-09T00:55:11.926Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.tdmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.td
📚 Learning: 2025-11-03T23:09:26.881Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.
Applied to files:
cmake/ExternalDependencies.cmakeCMakeLists.txt
📚 Learning: 2026-01-04T23:01:47.734Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: mlir/CMakeLists.txt:13-15
Timestamp: 2026-01-04T23:01:47.734Z
Learning: In the munich-quantum-toolkit/core repository's MLIR directory (mlir/CMakeLists.txt), the MSVC compile definition `_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING` is necessary to suppress warnings triggered by MLIR/LLVM internal headers, not because the project code uses non-floating complex types. This suppression is needed to avoid warnings from third-party MLIR/LLVM header includes.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-14T16:51:52.504Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 23
File: .readthedocs.yaml:13-18
Timestamp: 2025-12-14T16:51:52.504Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, LLVM and MLIR toolchains are required for the documentation build because `uv run` includes a full build of the package, which compiles C++/MLIR extensions using scikit-build-core.
Applied to files:
cmake/ExternalDependencies.cmake
📚 Learning: 2025-12-17T11:32:53.683Z
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:53.683Z
Learning: In the mlir directory of the munich-quantum-toolkit/core repository, the clang-tidy configuration (mlir/.clang-tidy) prefers marking free functions as `static` over placing them in anonymous namespaces. The configuration enables `llvm-prefer-static-over-anonymous-namespace` and disables `misc-use-anonymous-namespace`. Types/classes should still use anonymous namespaces, but free functions should use explicit `static` linkage.
Applied to files:
.clang-tidy
📚 Learning: 2025-12-22T01:25:21.609Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1383
File: bindings/ir/register_permutation.cpp:153-171
Timestamp: 2025-12-22T01:25:21.609Z
Learning: In the munich-quantum-toolkit/core repository, when using nanobind iterator factory functions like `make_key_iterator` and `make_iterator`, the unqualified form (without explicit `nb::` prefix) is preferred. The clang-tidy configuration suggests removal of explicit namespace qualification, relying on ADL (Argument-Dependent Lookup) to resolve these functions correctly.
Applied to files:
.clang-tidymlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-08T22:56:09.502Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1435
File: docs/mlir/QC.md:1-11
Timestamp: 2026-01-08T22:56:09.502Z
Learning: In the munich-quantum-toolkit/core repository, MLIR dialect documentation files (e.g., MLIRQCDialect.md, MLIRQCInterfaces.md) are automatically generated during the documentation build via the ReadTheDocs `pre_build` step and do not need to be committed to the repository.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-10T15:54:56.832Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:891-923
Timestamp: 2026-01-10T15:54:56.832Z
Learning: In MLIR conversion code (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf::WhileOp construction only creates empty regions, not blocks. After creating a WhileOp with `rewriter.create<scf::WhileOp>(...)`, the first calls to `rewriter.createBlock(&whileOp.getBefore(), ...)` and `rewriter.createBlock(&whileOp.getAfter(), ...)` create the initial blocks in those empty regions, so no block erasure is needed. This differs from scf::IfOp, which auto-creates an empty block that must be erased when using `inlineRegionBefore()`.
<!-- <review_comment_addressed>
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCODialect.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/include/mlir/Dialect/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-07T09:10:31.836Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1355
File: src/qdmi/sc/Device.cpp:97-102
Timestamp: 2025-12-07T09:10:31.836Z
Learning: In the munich-quantum-toolkit/core repository, duplication of QDMI-related macros (such as IS_INVALID_ARGUMENT) across device implementations (e.g., in src/qdmi/sc/Device.cpp and src/qdmi/dd/Device.cpp) is acceptable as a temporary measure. The preferred long-term solution is to upstream these macros to the QDMI repository rather than creating local shared headers, so they can be reused across all dependent projects.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCODialect.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/include/mlir/Dialect/QCO/IR/QCODialect.h
📚 Learning: 2026-01-12T13:02:42.152Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:841-887
Timestamp: 2026-01-12T13:02:42.152Z
Learning: In the QCO dialect (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf.if operations with qubit operands must satisfy the invariant that both then and else branches yield the same physical qubits (as quantum resources), even though they may be represented by different SSA values along the qubit evolution chain. This property allows the ConvertQCOScfIfOp pattern to safely use only the then branch's yield operands for replacement, as all SSA values map to the same qc.qubit reference after type conversion to memory semantics. The else branch is guaranteed to exist in valid QCO programs.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCODialect.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/QCO/IR/QCODialect.hmlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-17T17:44:21.624Z
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:21.624Z
Learning: In mlir/include/mlir/Dialect/QCO/IR/QCOOps.td (QCO dialect), ensure GPhaseOp declares MemoryEffects<[MemWrite]> rather than Pure. This op has no results and is a zero-target operation, so using Pure would lead to its removal by dead-code elimination. By marking it with MemWrite, review ensures DCE preserves the operation because it has a meaningful effect on the global quantum state. This guidance applies when reviewing or updating QCO ops in this file (and broadly to similar zero-target ops with side effects in QCO).
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-08T14:55:43.899Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-100
Timestamp: 2025-12-08T14:55:43.899Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), GPhaseOp is a zero-target operation (global phase). When a CtrlOp wraps a GPhaseOp, it only has control qubits and no targets. The CtrlInlineGPhase canonicalization pattern correctly produces outputs only for the positive controls, not targets.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-11-01T15:57:31.153Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-04T06:59:40.314Z
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.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 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/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2026-01-04T21:41:30.252Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: cmake/SetupMLIR.cmake:32-33
Timestamp: 2026-01-04T21:41:30.252Z
Learning: In the munich-quantum-toolkit/core repository, the project provides its own pre-built LLVM/MLIR distributions that are compiled with LLVM_ENABLE_RTTI=ON and LLVM_ENABLE_EH=ON. Users are directed to these distributions via the upgrade guide and installation instructions to avoid ABI compatibility issues. The mid-term goal is to eliminate RTTI and exception handling dependencies entirely.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 Learning: 2025-12-08T11:32:57.308Z
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1375
File: python/mqt/core/plugins/qiskit/converters.py:71-78
Timestamp: 2025-12-08T11:32:57.308Z
Learning: In the munich-quantum-toolkit/core repository, helper functions like `_raise_error` are used to satisfy Ruff's TRY301 rule when raising exceptions in nested contexts (e.g., within try/except blocks). This pattern is necessary given the project's strict linting configuration.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
📚 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/QCO/IR/QCOOps.td
📚 Learning: 2025-11-05T09:23:46.540Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1287
File: src/qdmi/dd/Device.cpp:492-521
Timestamp: 2025-11-05T09:23:46.540Z
Learning: In the munich-quantum-toolkit/core repository, the `qasm3::Importer::imports()` function is backwards compatible with OpenQASM 2 programs. Therefore, it can be used to parse both QASM2 and QASM3 program formats without requiring separate importers for each version.
Applied to files:
mlir/include/mlir/Dialect/QCO/IR/QCOOps.td
🔇 Additional comments (9)
.clang-tidy (1)
77-78: LGTM!Excluding Eigen headers from
misc-include-cleaneris appropriate. Eigen is a header-only template library where include-cleaner can produce false positives due to implicit template instantiation patterns.mlir/include/mlir/Dialect/QCO/IR/QCODialect.h (1)
147-154: LGTM!The
getConstOperation()helper correctly works around MLIR's lack of a const overload forgetOperation(). The use of the dereference operator to obtain a const pointer is well-documented.mlir/include/mlir/Dialect/QCO/IR/QCOOps.td (7)
241-254: LGTM!The trait syntax change to use TableGen's
#operator is cleaner than!strconcat. The new trait aliases provide good coverage for standard gate arities.
274-277: LGTM!The 1×1 matrix return type is appropriate for global phase (effectively a scalar phase factor), and
std::optionalcorrectly handles the case whenthetais a dynamic SSA value.
301-304: LGTM!Static
Eigen::Matrix2cdreturn type is correct for non-parameterized single-qubit gates whose unitary matrices are compile-time constants.
555-558: LGTM!Non-static
std::optional<Eigen::Matrix2cd>return type correctly handles parameterized gates where the unitary depends on runtime angle values. The optional handles cases when parameters are dynamic SSA values rather than constants.
755-758: LGTM!Static
Eigen::Matrix4cdis correct for non-parameterized two-qubit gates.
848-851: LGTM!Consistent with the parameterized gate pattern—non-static
std::optional<Eigen::Matrix4cd>handles dynamic angle parameters for two-qubit rotation gates.
1125-1128: LGTM!
std::optional<Eigen::MatrixXcd>is the correct return type forCtrlOp—the matrix size depends on the number of controls (which is variable), requiring dynamic allocation. Theoptionalhandles cases where the inner unitary's parameters are not compile-time constants.
| #pragma clang diagnostic pop | ||
| #endif | ||
|
|
||
| #include <Eigen/Core> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n mlir/include/mlir/Dialect/QCO/IR/QCODialect.h | sed -n '20,35p'Repository: munich-quantum-toolkit/core
Length of output: 590
🌐 Web query:
Eigen unsupported module stability KroneckerProduct API guarantees
💡 Result:
Short answer: There are no formal stability/ABI/API guarantees for Eigen's "unsupported" modules (including KroneckerProduct). They are labeled "unsupported" and provided "as is"; they may change, be moved into supported code, or be modified between releases, so you should not rely on long‑term API/ABI stability without pinning a specific Eigen version or vendoring the code. Use eval()/copies as documented when assigning results. [1][2][3]
Sources:
- Eigen unsupported modules (general policy: "provided 'as is' without any support"). [1]
- KroneckerProduct module docs (experimental; usage notes such as use .eval()). [2]
- Community discussion noting no guarantees about ABI/API stability of unsupported modules. [3]
🏁 Script executed:
git log --oneline -1Repository: munich-quantum-toolkit/core
Length of output: 99
🏁 Script executed:
cat -n mlir/include/mlir/Dialect/QCO/IR/QCODialect.h | sed -n '58,78p'Repository: munich-quantum-toolkit/core
Length of output: 1287
🏁 Script executed:
cat -n mlir/include/mlir/Dialect/QCO/IR/QCODialect.h | sed -n '147,175p'Repository: munich-quantum-toolkit/core
Length of output: 1060
🏁 Script executed:
git log --format=%B -1Repository: munich-quantum-toolkit/core
Length of output: 91
🏁 Script executed:
git log --all --grep="unsupported\|KroneckerProduct" --oneline | head -20Repository: munich-quantum-toolkit/core
Length of output: 593
Eigen unsupported module lacks stability guarantees.
The unsupported/Eigen/KroneckerProduct header is from Eigen's unsupported module, which is provided "as is" without formal ABI/API stability guarantees. These modules may change, be modified, or moved between releases. Ensure the team is aligned on pinning a specific Eigen version or has a plan for handling potential API changes before merging.
Also applies to: 32-32
Comment on lines
169
to
171
| [[nodiscard]] inline UnitaryOpInterface getControlledOp(UnitaryOpInterface op) { | ||
| return llvm::cast<CtrlOp>(op).getBodyUnitary(); | ||
| } |
Contributor
There was a problem hiding this comment.
Unsafe cast without validation.
llvm::cast<CtrlOp>(op) will assert/crash if op is not a CtrlOp. The function name getControlledOp doesn't clearly indicate this precondition. Consider either:
- Using
llvm::dyn_castwith null check, or - Renaming to clarify the precondition (e.g.,
getBodyUnitaryFromCtrl)
Option 1: Safe cast with validation
[[nodiscard]] inline UnitaryOpInterface getControlledOp(UnitaryOpInterface op) {
+ auto ctrlOp = llvm::dyn_cast<CtrlOp>(op.getOperation());
+ if (!ctrlOp) {
+ return nullptr;
+ }
+ return ctrlOp.getBodyUnitary();
- return llvm::cast<CtrlOp>(op).getBodyUnitary();
}Option 2: Add assertion with clear message
[[nodiscard]] inline UnitaryOpInterface getControlledOp(UnitaryOpInterface op) {
+ assert(llvm::isa<CtrlOp>(op.getOperation()) &&
+ "getControlledOp requires a CtrlOp");
return llvm::cast<CtrlOp>(op).getBodyUnitary();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [[nodiscard]] inline UnitaryOpInterface getControlledOp(UnitaryOpInterface op) { | |
| return llvm::cast<CtrlOp>(op).getBodyUnitary(); | |
| } | |
| [[nodiscard]] inline UnitaryOpInterface getControlledOp(UnitaryOpInterface op) { | |
| assert(llvm::isa<CtrlOp>(op) && | |
| "getControlledOp requires a CtrlOp"); | |
| return llvm::cast<CtrlOp>(op).getBodyUnitary(); | |
| } |
🤖 Prompt for AI Agents
In @mlir/include/mlir/Dialect/QCO/IR/QCODialect.h around lines 169 - 171, The
function getControlledOp performs an unsafe llvm::cast<CtrlOp>(op) which will
crash if op is not a CtrlOp; change it to perform a safe
llvm::dyn_cast<CtrlOp>(op) and check the result (return a null/optional or
handle the error) or add a clear assertion message documenting the precondition;
alternatively, if you want to keep the unchecked behavior, rename
getControlledOp to something explicit like getBodyUnitaryFromCtrl to signal it
requires a CtrlOp. Ensure references to CtrlOp, UnitaryOpInterface, and the cast
are updated accordingly and that callers handle the null/failed-case when using
dyn_cast.
Comment on lines
147
to
150
| if constexpr (std::is_same_v<std::remove_cvref_t<MatrixType>, Eigen::MatrixXcd>) { | ||
| // cast to Eigen::MatrixXcd via visitor pattern | ||
| auto&& visitor = [](auto&& m) -> std::optional<MatrixType> { return m; }; | ||
| return std::visit(visitor, matrix); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Visitor does not handle std::nullopt_t correctly.
When the variant holds std::nullopt_t, the visitor attempts to construct std::optional<MatrixType> from std::nullopt_t. While this works (implicit conversion to std::nullopt), the intent could be clearer. More critically, returning the matrix m directly for Eigen types relies on implicit conversion to Eigen::MatrixXcd, which works for fixed-size matrices but the control flow could be more explicit.
Consider explicit handling of nullopt
- auto&& visitor = [](auto&& m) -> std::optional<MatrixType> { return m; };
- return std::visit(visitor, matrix);
+ return std::visit([](auto&& m) -> std::optional<MatrixType> {
+ if constexpr (std::is_same_v<std::remove_cvref_t<decltype(m)>, std::nullopt_t>) {
+ return std::nullopt;
+ } else {
+ return MatrixType(m);
+ }
+ }, matrix);🤖 Prompt for AI Agents
In @mlir/include/mlir/Dialect/QCO/IR/QCOInterfaces.td around lines 147 - 150,
The visitor used in the std::visit call does not explicitly handle
std::nullopt_t and relies on implicit conversions for Eigen types; update the
visitor lambda (the auto&& visitor used with std::visit on matrix) to have
explicit overloads: one overload taking std::nullopt_t that returns
std::optional<MatrixType>{std::nullopt} and another template/overload for Eigen
matrix-like types that constructs and returns std::optional<MatrixType>{
MatrixType(m) } (i.e., explicitly convert m into MatrixType rather than
returning m directly); then use this visitor in the existing std::visit call so
nullopt is unambiguous and Eigen conversions are explicit.
5e8d0d0 to
01b3e0b
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
Cpp-Linter Report
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp:
- Line 40: The codebase still mixes std::numbers and llvm::numbers across
StandardGates implementations causing inconsistent behavior; update all uses of
std::numbers::pi to llvm::numbers::pi in the remaining gate implementations
(UOp.cpp — 3 places, ROp.cpp — 3 places, XXPlusYYOp.cpp — 2 places, TOp.cpp — 1
place, TdgOp.cpp — 1 place), and ensure any comparisons like std::abs(*lambda -
std::numbers::pi) use llvm::numbers::pi (and keep std::abs or switch to
llvm::abs if desired) so the migration is complete and consistent with U2Op.cpp,
HOp.cpp, and ECROp.cpp.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
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: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
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: 1287
File: test/qdmi/dd/CMakeLists.txt:9-21
Timestamp: 2025-11-03T23:09:26.881Z
Learning: The CMake functions `generate_device_defs_executable` and `generate_prefixed_qdmi_headers` used in QDMI device test CMakeLists.txt files are provided by the external QDMI library (fetched via FetchContent from https://github.com/Munich-Quantum-Software-Stack/qdmi.git), specifically in the cmake/PrefixHandling.cmake module of the QDMI repository.
📚 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/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2026-01-10T16:07:55.896Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
📚 Learning: 2026-01-12T12:05:47.463Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
📚 Learning: 2026-01-10T16:28:41.975Z
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]
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2026-01-07T12:29:16.380Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2026-01-10T18:49:44.352Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2026-01-07T12:29:02.062Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
📚 Learning: 2026-01-12T13:02:42.152Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:841-887
Timestamp: 2026-01-12T13:02:42.152Z
Learning: In the QCO dialect (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf.if operations with qubit operands must satisfy the invariant that both then and else branches yield the same physical qubits (as quantum resources), even though they may be represented by different SSA values along the qubit evolution chain. This property allows the ConvertQCOScfIfOp pattern to safely use only the then branch's yield operands for replacement, as all SSA values map to the same qc.qubit reference after type conversion to memory semantics. The else branch is guaranteed to exist in valid QCO programs.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
📚 Learning: 2025-12-17T17:44:31.349Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2025-12-08T23:44:39.930Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2025-12-17T11:32:45.843Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.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/QCO/IR/Operations/StandardGates/ECROp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
📚 Learning: 2025-12-08T12:44:05.883Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:60-70
Timestamp: 2025-12-08T12:44:05.883Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The RemoveTrivialCtrl pattern correctly only checks getNumPosControls() when determining if a CtrlOp should be removed.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
📚 Learning: 2025-11-01T15:57:31.153Z
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.
Applied to files:
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
🧬 Code graph analysis (2)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cpp (6)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (2)
getUnitaryMatrix(112-126)getUnitaryMatrix(112-112)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/GPhaseOp.cpp (2)
getUnitaryMatrix(59-65)getUnitaryMatrix(60-60)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/IdOp.cpp (2)
getUnitaryMatrix(44-46)getUnitaryMatrix(44-44)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/POp.cpp (2)
getUnitaryMatrix(68-73)getUnitaryMatrix(68-68)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ROp.cpp (2)
getUnitaryMatrix(88-99)getUnitaryMatrix(88-88)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RXOp.cpp (2)
getUnitaryMatrix(66-75)getUnitaryMatrix(66-66)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (3)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cpp (4)
op(32-35)op(32-33)getUnitaryMatrix(45-48)getUnitaryMatrix(45-45)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/UOp.cpp (4)
op(35-49)op(35-36)getUnitaryMatrix(115-132)getUnitaryMatrix(115-115)mlir/lib/Dialect/QCO/IR/Operations/StandardGates/IdOp.cpp (2)
getUnitaryMatrix(44-46)getUnitaryMatrix(44-44)
🪛 Cppcheck (2.19.0)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cpp
[information] 14-14: Include file
(missingIncludeSystem)
[information] 15-15: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp
[information] 14-14: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp
[information] 15-15: Include file
(missingIncludeSystem)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (3)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cpp (1)
44-48: LGTM! Correct Hadamard matrix implementation.The unitary matrix correctly represents H = 1/√2 × [[1, 1], [1, -1]]. Using
constexprforxis appropriate sincellvm::numbers::sqrt2is a compile-time constant.mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp (1)
44-54: LGTM! Correct ECR gate matrix implementation.The 4×4 unitary matrix for the two-qubit ECR gate is correctly constructed with clear variable naming (
m0for zero,m1for real 1/√2,mifor imaginary i/√2).mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (1)
112-126: LGTM! Correct U2 gate matrix implementation.The unitary matrix correctly implements U2(φ,λ) = 1/√2 × [[1, -e^(iλ)], [e^(iφ), e^(i(φ+λ))]]. Using
e^(i(λ+π))form01is mathematically equivalent to-e^(iλ). Returningstd::optionalis appropriate for gates with runtime parameters.
| const auto lambda = valueToDouble(op.getLambda()); | ||
| if (!phi || std::abs(*phi) > TOLERANCE || !lambda || | ||
| std::abs(*lambda - std::numbers::pi) > TOLERANCE) { | ||
| std::abs(*lambda - llvm::numbers::pi) > TOLERANCE) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find remaining usages of std::numbers in QCO StandardGates to verify migration consistency
echo "=== Files still using std::numbers in StandardGates ==="
rg -n 'std::numbers' mlir/lib/Dialect/QCO/IR/Operations/StandardGates/
echo ""
echo "=== Files using llvm::numbers in StandardGates ==="
rg -n 'llvm::numbers' mlir/lib/Dialect/QCO/IR/Operations/StandardGates/
echo ""
echo "=== Summary of all gate implementation files ==="
fd -e '.cpp' . mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ | sortRepository: munich-quantum-toolkit/core
Length of output: 4874
Complete the migration from std::numbers to llvm::numbers across all StandardGates implementations.
The migration is incomplete. While U2Op.cpp, HOp.cpp, and ECROp.cpp have been updated to use llvm::numbers, the following files still use std::numbers:
- UOp.cpp (3 usages)
- ROp.cpp (3 usages)
- XXPlusYYOp.cpp (2 usages)
- TOp.cpp (1 usage)
- TdgOp.cpp (1 usage)
If std::numbers causes CI failures on Windows/macOS, this fix must be applied consistently across all affected gate implementations to prevent partial issues.
🤖 Prompt for AI Agents
In @mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp at line 40, The
codebase still mixes std::numbers and llvm::numbers across StandardGates
implementations causing inconsistent behavior; update all uses of
std::numbers::pi to llvm::numbers::pi in the remaining gate implementations
(UOp.cpp — 3 places, ROp.cpp — 3 places, XXPlusYYOp.cpp — 2 places, TOp.cpp — 1
place, TdgOp.cpp — 1 place), and ensure any comparisons like std::abs(*lambda -
std::numbers::pi) use llvm::numbers::pi (and keep std::abs or switch to
llvm::abs if desired) so the migration is complete and consistent with U2Op.cpp,
HOp.cpp, and ECROp.cpp.
This reverts commit 6caa04b.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/HOp.cpp`:
- Around line 44-47: Add an explicit include for the <numbers> header used by
std::numbers::sqrt2: modify the top of the file so that HOp.cpp includes
<numbers> (matching the pattern used by TOp/TdgOp/ROp/UOp/U2Op/XXPlusYYOp) so
HOp::getUnitaryMatrix can safely use std::numbers::sqrt2 without relying on
transitive includes.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RZXOp.cpp`:
- Around line 69-82: The RZXOp::getUnitaryMatrix implementation has inverted
off-diagonal signs: with ms defined as 0 - i*sin(θ/2) the lower-block entries
use ms but should use -ms; update the returned Eigen::Matrix4cd so rows 2 and 3
use mc, -ms and -ms, mc respectively (i.e., replace the two occurrences of ms in
the last two rows with -ms) to match the reference GateMatrixDefinitions.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/TdgOp.cpp`:
- Around line 58-63: The TdgOp::getUnitaryMatrix implementation is correct but
can be simplified by inlining the trivial real constants; replace the local
temporaries m00 and m01 with 1.0 and 0.0 directly in the Eigen::Matrix2cd
construction while keeping the computed m11 (std::polar(...)) as-is, so the
matrix is constructed as {{1.0, 0.0}, {0.0, m11}}.
In `@mlir/unittests/Dialect/QCO/IR/test_unitary_matrix.cpp`:
- Around line 11-28: The test uses Eigen types (Eigen::Matrix2cd,
Eigen::Matrix4cd, Eigen::MatrixXcd) but does not explicitly include Eigen; add
an explicit Eigen header include (for example Eigen/Dense) to the top of the
test's include list so those types are declared directly and the dependency is
obvious and robust.
In `@mlir/unittests/Dialect/QCO/IR/test_unitary_op_interface.cpp`:
- Around line 64-87: The helper functions getFirstOp (templated OpType, uses
ModuleOp and func::FuncOp) and static toString(ModuleOp) are unused dead code;
either remove both functions from test_unitary_op_interface.cpp or add one or
more tests that call getFirstOp to fetch an operation from a ModuleOp (using a
concrete OpType) and/or call toString(moduleOp) to assert expected printed text.
Update or remove any forward declarations accordingly and run the unit tests to
ensure no unused-symbol warnings remain.
- Around line 111-119: The loop uses an int index i and directly calls
expectedValues.at(i++), which can cause signed/unsigned mismatch and an
out-of-bounds exception if there are more unitary ops than expected; change the
index to size_t i = 0 and add a bounds guard before accessing expectedValues
(e.g., assert/EXPECT_LT(i, expectedValues.size()) or break/FAIL with a clear
message) so when iterating funcOp.getOps() and casting with
llvm::dyn_cast<qco::UnitaryOpInterface>(op) you never call
expectedValues.at(i++) beyond its size while still testing
unitaryOp.getUnitaryMatrix<Eigen::Matrix2cd>() against expectedValues[i].
- Around line 35-91: Extract the duplicated test setup into a shared test
fixture header (e.g., QCOTestFixture.h): move SetUp(), buildQCOIR(),
getFirstOp<>, toString(), and the MLIRContext unique_ptr member into a base
class (e.g., QCOTestFixture) and keep the DialectRegistry/append/load logic
inside SetUp; update QcoUnitaryOpInterfaceTest to inherit from QCOTestFixture
and remove the duplicated implementations; ensure the header declares the class,
includes necessary MLIR headers, has include guards or pragma once, and that
tests include this header instead of duplicating code.
♻️ Duplicate comments (1)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (1)
113-127: LGTM! Correct U2 gate unitary matrix.The implementation correctly constructs the U2(φ, λ) gate matrix using the standard definition with
1/√2normalization and appropriate phase factors. The use ofstd::polarfor phase components is consistent with other gate implementations in this PR.Note: The
std::numbersvsllvm::numbersconsistency concern was previously flagged in an earlier review.
Comment on lines
35
to
71
| class QcoUnitaryOpInterfaceTest : public testing::Test { | ||
| protected: | ||
| void SetUp() override { | ||
| DialectRegistry registry; | ||
| registry | ||
| .insert<qco::QCODialect, arith::ArithDialect, cf::ControlFlowDialect, | ||
| func::FuncDialect, scf::SCFDialect, LLVM::LLVMDialect>(); | ||
|
|
||
| context = std::make_unique<MLIRContext>(); | ||
| context->appendDialectRegistry(registry); | ||
| context->loadAllAvailableDialects(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Build expected QCO IR programmatically and run canonicalization | ||
| */ | ||
| [[nodiscard]] OwningOpRef<ModuleOp> buildQCOIR( | ||
| const std::function<void(qco::QCOProgramBuilder&)>& buildFunc) const { | ||
| qco::QCOProgramBuilder builder(context.get()); | ||
| builder.initialize(); | ||
| buildFunc(builder); | ||
| return builder.finalize(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Get first operation of given type in a module containing a function | ||
| * as its first operation. | ||
| */ | ||
| template <typename OpType> | ||
| [[nodiscard]] OpType getFirstOp(ModuleOp moduleOp) { | ||
| auto funcOp = llvm::dyn_cast<func::FuncOp>( | ||
| moduleOp.getBody()->getOperations().front()); | ||
| if (!funcOp) { | ||
| return nullptr; | ||
| } | ||
|
|
||
| auto ops = funcOp.getOps<OpType>(); | ||
| if (ops.empty()) { | ||
| return nullptr; | ||
| } | ||
|
|
||
| return *ops.begin(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Get text representation of given module. | ||
| */ | ||
| [[nodiscard]] static std::string toString(ModuleOp moduleOp) { | ||
| std::string buffer; | ||
| llvm::raw_string_ostream serializeStream{buffer}; | ||
| moduleOp->print(serializeStream); | ||
| return serializeStream.str(); | ||
| } | ||
|
|
||
| private: | ||
| std::unique_ptr<MLIRContext> context; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting shared test fixture to reduce duplication.
The SetUp, buildQCOIR, getFirstOp, and toString methods are duplicated from test_unitary_matrix.cpp. Consider extracting these into a shared test utility header (e.g., QCOTestFixture.h) to improve maintainability.
🧰 Tools
🪛 Cppcheck (2.19.0)
[style] 64-64: The function 'getFirstOp' is never used.
(unusedFunction)
[style] 82-82: The function 'toString' is never used.
(unusedFunction)
🤖 Prompt for AI Agents
In `@mlir/unittests/Dialect/QCO/IR/test_unitary_op_interface.cpp` around lines 35
- 91, Extract the duplicated test setup into a shared test fixture header (e.g.,
QCOTestFixture.h): move SetUp(), buildQCOIR(), getFirstOp<>, toString(), and the
MLIRContext unique_ptr member into a base class (e.g., QCOTestFixture) and keep
the DialectRegistry/append/load logic inside SetUp; update
QcoUnitaryOpInterfaceTest to inherit from QCOTestFixture and remove the
duplicated implementations; ensure the header declares the class, includes
necessary MLIR headers, has include guards or pragma once, and that tests
include this header instead of duplicating code.
6f5d755 to
00d29dd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/ECROp.cpp`:
- Line 14: Add an explicit `#include` <numbers> to ECROp.cpp because the code uses
std::numbers::sqrt2 (e.g., in the ECROp handling where sqrt2 is referenced);
this removes reliance on transitive includes and matches other gates (HOp, TOp,
TdgOp, U2Op) that already include <numbers>. Ensure the new include is placed
with the other standard headers at the top of the file.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/GPhaseOp.cpp`:
- Around line 14-21: The implementation in GPhaseOp.cpp returns
Eigen::Matrix<std::complex<double>, 1, 1> but omits the required Eigen header;
add `#include` <Eigen/Core> to the includes at the top of
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/GPhaseOp.cpp so the
Eigen::Matrix type and related traits are available (ensure the new include is
added alongside the existing standard and MLIR headers).
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/POp.cpp`:
- Around line 15-16: Remove the explicit `#include` <Eigen/Core> from POp.cpp to
match other gate implementations (e.g., RYYOp.cpp, RZZOp.cpp) that rely on the
transitive include provided by QCODialect.h; update POp.cpp to rely on
QCODialect.h for Eigen types and verify the file still compiles, and if
compilation fails instead add the Eigen include to the central QCODialect.h so
all gate implementations consistently get Eigen definitions.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RZXOp.cpp`:
- Around line 15-22: The RZXOp::getUnitaryMatrix implementation returns
Eigen::Matrix4cd but the Eigen header is missing; add the include for
<Eigen/Core> to the includes at the top of RZXOp.cpp (alongside <cmath>,
<complex>, and the mlir includes) so Eigen types compile properly; ensure the
include appears before any code that uses Eigen types and matches the pattern
used in iSWAPOp.cpp and RXXOp.cpp.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/TOp.cpp`:
- Around line 56-62: TOp::getUnitaryMatrix currently assigns m00 and m01 as
named temporaries; simplify by inlining those constants directly into the
Eigen::Matrix2cd constructor and keep the computed phase as-is (the std::polar
call for m11). Update the body of TOp::getUnitaryMatrix to construct and return
Eigen::Matrix2cd{{1.0, 0.0}, {0.0, std::polar(1.0, std::numbers::pi / 4.0)}} so
it matches the concise style used by POp::getUnitaryMatrix.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/XXPlusYYOp.cpp`:
- Around line 14-24: The file
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/XXPlusYYOp.cpp uses
Eigen::Matrix4cd but doesn't include <Eigen/Core>; add `#include` <Eigen/Core>
near the other includes (before any code that references Eigen types) so the
return type Eigen::Matrix4cd and any Eigen operations compile; apply the same
fix to other files in this PR that return or use Eigen types.
In `@mlir/unittests/Dialect/QCO/IR/test_unitary_op_interface.cpp`:
- Around line 88-91: The code incorrectly calls llvm::dyn_cast on
moduleOps.begin() (an iterator) instead of the operation it points to; change
the use in the test to dereference the iterator first and dyn_cast that value
(i.e., use *moduleOps.begin() or call .front()/.begin()->get() as appropriate)
so that llvm::dyn_cast<func::FuncOp> receives an Operation*, and keep references
to moduleOp, moduleOps, getBody(), getOperations(), moduleOps.begin(), and
func::FuncOp to locate and update the cast site.
♻️ Duplicate comments (3)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (1)
118-122: Thestd::numbersusage is inconsistent with other files that usellvm::numbers.This file uses
std::numbers::sqrt2andstd::numbers::piin the newgetUnitaryMatrix()function, which is consistent with the existing canonicalization patterns in this file but inconsistent with other StandardGates files that have migrated tollvm::numbers. The previous review already flagged this migration issue across the codebase.mlir/unittests/Dialect/QCO/IR/test_unitary_op_interface.cpp (1)
62-67: Remove unusedtoStringhelper method.This method is defined but never used in this test file.
mlir/unittests/Dialect/QCO/IR/test_unitary_matrix.cpp (1)
11-29: LGTM!The explicit
#include <Eigen/Core>at line 14 properly declares the Eigen dependency. The include ordering follows conventions (local headers, then system headers). The cppcheck "missingInclude" hints are false positives from the static analyzer being unable to resolve include paths.
Comment on lines
+15
to
+16
| #include <Eigen/Core> | ||
| #include <complex> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing the explicit <Eigen/Core> include for consistency.
Other gate implementations in this PR (e.g., RYYOp.cpp, RZZOp.cpp) rely on the transitive include from QCODialect.h rather than including <Eigen/Core> directly. For consistency across the codebase, consider removing this explicit include.
♻️ Suggested change
`#include` "mlir/Dialect/QCO/IR/QCODialect.h"
`#include` "mlir/Dialect/QCO/QCOUtils.h"
`#include` "mlir/Dialect/Utils/Utils.h"
-#include <Eigen/Core>
`#include` <complex>🧰 Tools
🪛 Cppcheck (2.19.0)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
🤖 Prompt for AI Agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/POp.cpp` around lines 15 -
16, Remove the explicit `#include` <Eigen/Core> from POp.cpp to match other gate
implementations (e.g., RYYOp.cpp, RZZOp.cpp) that rely on the transitive include
provided by QCODialect.h; update POp.cpp to rely on QCODialect.h for Eigen types
and verify the file still compiles, and if compilation fails instead add the
Eigen include to the central QCODialect.h so all gate implementations
consistently get Eigen definitions.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RYYOp.cpp`:
- Around line 15-17: You have a redundant utils:: qualifier despite the file
already importing the namespace with using namespace mlir::utils; — locate all
calls to utils::valueToDouble (and any other utils::-prefixed symbols) in
RYYOp.cpp (e.g., valueToDouble usages within functions constructing or parsing
the RYYOp) and remove the leading utils:: qualifier so they call valueToDouble
directly; ensure all occurrences (including those around the
parse/build/print/compute parts previously flagged) are updated for consistent
style.
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RZZOp.cpp`:
- Around line 15-16: The file imports mlir::utils (via a using or namespace
alias) but still uses the redundant qualifier utils::valueToDouble and similar
calls; update RZZOp.cpp to remove the redundant utils:: prefix and call
valueToDouble(...) (and any other utils::foo calls) unqualified to match the
style used in RYYOp.cpp, ensuring all occurrences (e.g., valueToDouble and
related helpers) are updated consistently.
♻️ Duplicate comments (1)
mlir/lib/Dialect/QCO/IR/Operations/StandardGates/U2Op.cpp (1)
114-128: U2 matrix formula looks correct; consider flattening the nested optionals.
Minor readability:phi/lambdacan be extracted first and early-return on missing values.
Also, this introduces additionalstd::numbersusage—if the project is standardizing onllvm::numbers, align this implementation too.
Comment on lines
+15
to
+17
| #include <Eigen/Core> | ||
| #include <cmath> | ||
| #include <complex> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid redundant utils:: qualifier (minor consistency nit).
At Line 28 you already have using namespace mlir::utils;, but Line 73 still calls utils::valueToDouble(...). Consider picking one style for consistency.
Proposed diff
- if (auto theta = utils::valueToDouble(getTheta())) {
+ if (auto theta = valueToDouble(getTheta())) {Also applies to: 23-23, 70-83
🧰 Tools
🪛 Cppcheck (2.19.0)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
🤖 Prompt for AI Agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RYYOp.cpp` around lines 15 -
17, You have a redundant utils:: qualifier despite the file already importing
the namespace with using namespace mlir::utils; — locate all calls to
utils::valueToDouble (and any other utils::-prefixed symbols) in RYYOp.cpp
(e.g., valueToDouble usages within functions constructing or parsing the RYYOp)
and remove the leading utils:: qualifier so they call valueToDouble directly;
ensure all occurrences (including those around the parse/build/print/compute
parts previously flagged) are updated for consistent style.
Comment on lines
+15
to
+16
| #include <Eigen/Core> | ||
| #include <complex> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid redundant utils:: qualifier (minor consistency nit).
Same consistency point as in RYYOp.cpp: Line 27 imports mlir::utils, but Line 72 still calls utils::valueToDouble(...).
Proposed diff
- if (auto theta = utils::valueToDouble(getTheta())) {
+ if (auto theta = valueToDouble(getTheta())) {Also applies to: 22-22, 69-82
🧰 Tools
🪛 Cppcheck (2.19.0)
[information] 15-15: Include file
(missingIncludeSystem)
[information] 16-16: Include file
(missingIncludeSystem)
🤖 Prompt for AI Agents
In `@mlir/lib/Dialect/QCO/IR/Operations/StandardGates/RZZOp.cpp` around lines 15 -
16, The file imports mlir::utils (via a using or namespace alias) but still uses
the redundant qualifier utils::valueToDouble and similar calls; update RZZOp.cpp
to remove the redundant utils:: prefix and call valueToDouble(...) (and any
other utils::foo calls) unqualified to match the style used in RYYOp.cpp,
ensuring all occurrences (e.g., valueToDouble and related helpers) are updated
consistently.
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR implements the feature of the unitary matrix definition for
UnitaryOpInterfacewhich has been removed from the original dialect rewrite PR in favor of this follow-up.It aims to allow access to the matrix definition of the standard gates via a fast, stack-allocated matrix returned by the function
getUnitaryMatrixDefinition()in the trait.A dynamic, heap-based matrix (size
Eigen::Dynamic) is returned by theUnitaryOpInterface. This requires a copy at the moment and further evaluations are required if all matrices should be moved to the heap or if there is a way to make use of the draft functiongetFastUnitaryMatrix()which uses a template parameter, but is currently also only accessible via a trait and thus needs a cast to the correct type first.The changes require the newly introduced
Eigenlibrary for its linear algebra type.For now, it uses the unsupported kronecker product feature which needs to be discussed before merging. "Unsupported" features in Eigen are community-contributed and are not an indication of how mature the code is. However, the maintainers do not provide any API guarantees.
Checklist: