✨ Add non-linear control flow conversions between QC and QCO dialects #1396
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
added 24 commits
December 11, 2025 15:43
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
denialhaag
added
enhancement
added 10 commits
December 22, 2025 15:06
Contributor
📝 WalkthroughWalkthroughAdds SCF, Func, and Arith builder APIs to QC/QCO, implements bidirectional conversion patterns for SCF/Func between QC and QCO, and introduces per-region qubit tracking and stateful, region-aware lowering in QC→QCO and QC←QCO conversions. Changes
Sequence Diagram(s)sequenceDiagram
rect rgb(240,248,255)
participant QC_Module as QC Module
participant QCToQCO as QCToQCO Pass
participant LoweringState as LoweringState (per-region)
participant QCO_Module as QCO Module
QC_Module->>QCToQCO: run pass
QCToQCO->>LoweringState: collectUniqueQubits(module)
Note right of LoweringState: build per-region qubitMap & regionMap
QCToQCO->>QCToQCO: register StatefulOpConversionPattern(s)
QCToQCO->>QCO_Module: apply patterns -> produce QCO ops (region-aware mapping)
end
rect rgb(255,250,240)
participant QCOToQC as QCOToQC Pass
QCO_Module->>QCOToQC: run pass
QCOToQC->>QCOToQC: use patterns for scf/func -> QC ops
QCOToQC->>QC_Module: emit QC Module
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 14
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/unittests/CMakeLists.txtmlir/unittests/conversion/CMakeLists.txtmlir/unittests/conversion/test_conversion.cpp
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
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.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:80-100
Timestamp: 2025-12-08T23:58:09.648Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), quartz.ctrl uses reference semantics and does not return values, unlike flux.ctrl which uses value semantics and returns transformed qubits. When inlining a GPhaseOp in the CtrlInlineGPhase pattern, it's correct to create POp operations for positive controls and erase the CtrlOp without collecting or replacing result values.
Learnt from: 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.
📚 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:
mlir/unittests/conversion/CMakeLists.txtmlir/unittests/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:
mlir/unittests/conversion/CMakeLists.txtmlir/unittests/CMakeLists.txt
📚 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/unittests/conversion/test_conversion.cppmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.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/unittests/conversion/test_conversion.cppmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.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/Conversion/QCOToQC/QCOToQC.cppmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-10T08:09:54.528Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1246
File: bindings/CMakeLists.txt:0-0
Timestamp: 2025-10-10T08:09:54.528Z
Learning: In the Munich Quantum Toolkit (MQT) Core project, scikit-build-core is configured with `wheel.install-dir = "mqt/core"` in pyproject.toml, which automatically prefixes all CMake `DESTINATION` paths with `mqt/core/` during wheel installation. Therefore, CMake install destinations are relative to the `mqt/core` package namespace, not `site-packages`.
Applied to files:
mlir/unittests/CMakeLists.txt
📚 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/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/Builder/QCOProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/Builder/QCOProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-10T02:20:01.189Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.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/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
🧬 Code graph analysis (4)
mlir/unittests/conversion/test_conversion.cpp (4)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
mlir(29-163)mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
mlir(30-171)mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (4)
measure(127-131)measure(127-127)measure(133-140)measure(133-133)mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (4)
measure(156-167)measure(156-156)measure(169-183)measure(169-169)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (19)
QCProgramBuilder(35-39)scfFor(451-461)scfFor(452-453)scfWhile(463-475)scfWhile(464-465)scfIf(477-499)scfIf(478-480)scfCondition(501-504)scfCondition(501-501)funcReturn(516-519)funcReturn(516-516)funcCall(510-514)funcCall(510-511)funcFunc(521-539)funcFunc(521-523)arithConstantIndex(545-549)arithConstantIndex(545-545)arithConstantBool(551-556)arithConstantBool(551-551)
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (25)
scfFor(604-632)scfFor(604-606)scfWhile(634-683)scfWhile(634-637)scfIf(685-722)scfIf(685-688)QCOProgramBuilder(38-42)scfCondition(724-728)scfCondition(724-725)scfYield(730-733)scfYield(730-730)funcReturn(748-751)funcReturn(748-748)funcCall(739-746)funcCall(739-739)funcFunc(752-775)funcFunc(752-754)arithConstantIndex(781-785)arithConstantIndex(781-781)arithConstantBool(787-792)arithConstantBool(787-787)validateQubitValue(132-140)validateQubitValue(132-132)updateQubitTracking(142-150)updateQubitTracking(142-143)
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp (2)
mlir/lib/Conversion/QCOToQC/QCOToQC.cpp (18)
op(297-306)op(298-299)op(327-333)op(328-329)op(354-360)op(355-356)op(392-410)op(393-394)op(437-450)op(438-439)op(737-750)op(738-739)op(773-790)op(774-775)op(808-813)op(809-810)patterns(1147-1147)typeConverter(1148-1148)mlir/lib/Conversion/QCToQIR/QCToQIR.cpp (2)
ctx(1154-1267)typeConverter(1158-1158)
🪛 Cppcheck (2.19.0)
mlir/unittests/conversion/test_conversion.cpp
[information] 11-11: Include file
(missingInclude)
[information] 12-12: Include file
(missingInclude)
[information] 13-13: Include file
(missingInclude)
[information] 14-14: Include file
(missingInclude)
[information] 15-15: Include file
(missingInclude)
[information] 16-16: Include file
(missingInclude)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
[information] 22-22: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
🔇 Additional comments (17)
mlir/unittests/conversion/CMakeLists.txt (1)
1-29: LGTM!The CMakeLists configuration follows the established project conventions, including the
file(GLOB_RECURSE ...)pattern for collecting sources. The library dependencies are appropriate for the conversion tests, and the 60-second discovery timeout is reasonable. Based on learnings, this pattern is consistent with the parent repository.mlir/unittests/conversion/test_conversion.cpp (2)
77-128: Test coverage looks comprehensive for the basic SCF conversion cases.The test correctly verifies bidirectional conversion for
scf.forwith qubit operations. The string-based comparison approach is pragmatic for verifying structural equivalence, though it may be sensitive to IR formatting changes.
336-368: VerifyscfIftest correctly handles both branches using the same input qubit.In the expected QCO output (lines 340-360), both
thenBodyandelseBodylambdas useq1directly rather than the input qubits from their respective regions. This appears intentional for the memory-to-value semantic transformation, but verify this matches the actual conversion behavior where each branch independently transforms the same qubit reference.mlir/unittests/CMakeLists.txt (1)
12-18: LGTM!The integration of the new conversion test subdirectory and dependency follows the existing pattern for other test targets.
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (1)
447-504: LGTM! SCF operation builders are well-implemented.The SCF builders correctly implement memory-semantics control flow without iter_args. The
scfForandscfIfmethods properly handle yield insertion, andscfWhilecorrectly expects the before body to end with a condition (not a yield).mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
871-1072: Overall: New SCF, Func, and Arith API additions look well-designed.The API follows the existing patterns in the builder, with proper documentation and consistent method chaining. The separation of concerns between SCF control flow, function operations, and arithmetic constants is clean.
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (3)
685-722:scfIfregisters input qubits as valid in both then/else regions - verify this is intentional.Lines 702-705 add the same
qubitsto boththenRegionandelseRegionvalid sets. This allows both branches to reference the same input qubits, which aligns with QCO's value semantics where each branch independently transforms the inputs. However, this differs from typical SSA where each region would have its own block arguments.The current approach works because:
- Both branches operate on the same input qubits
- Each branch's operations consume and produce new values independently
- The results from either branch are unified at the
scf.ifoutputConfirm this matches the intended conversion semantics.
600-632: LGTM!scfForcorrectly manages region-local qubit tracking.The implementation properly:
- Creates block arguments for iter_args
- Registers them as valid in the body region
- Updates tracking at the parent region level after the loop completes
132-140: The region-aware qubit tracking implementation is correct. Block arguments passed across region boundaries via SCF operations are explicitly registered in their destination regions, making the validation logic sound. The use ofqubit.getParentRegion()accurately identifies which region owns each qubit value.mlir/lib/Conversion/QCOToQC/QCOToQC.cpp (1)
837-863: The conversion safely accessesnewIf.getThenRegion().front()after inlining. MLIR's verification ensures all blocks end with terminator operations, and inlineRegionBefore moves blocks from one region before a given position in another region. Whenscf::IfOpis constructed, its regions are initialized with at least one block. After inliningop.getThenRegion()(which is guaranteed non-empty), the destination region is populated with those blocks. The code then erases the empty initialization block and safely accesses the inlined yield terminator—a pattern consistent throughout the MLIR codebase.mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (3)
1031-1166: LGTM! Well-structured SCF operation builder APIs.The new SCF operation builders (
scfFor,scfWhile,scfIf,scfCondition,scfYield) are well-documented with clear examples and appropriate callback signatures for building nested regions. The region-aware qubit tracking design properly supports value semantics across control-flow boundaries.
1167-1266: LGTM! Function and arithmetic operation builder APIs.The
funcReturn,funcCall,funcFunc,arithConstantIndex, andarithConstantBoolmethods are consistent with the existing builder pattern and well-documented.
1289-1289: No action needed. ThefuncRegionpointer is already properly initialized tonullptrin the constructor's member initializer list (line 40 of QCOProgramBuilder.cpp). The pointer is then assigned a valid value in theinitialize()method before use, preventing undefined behavior.Likely an incorrect or invalid review comment.
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp (4)
1604-1623: In-place mutation pattern may conflict with conversion framework.The pattern mutates the existing
func::FuncOpin-place (changing block argument types on line 1614 and function type on line 1621) rather than creating a new operation. This approach:
- May interfere with the conversion framework's ability to rollback on failure
- Assumes all arguments are qubit types (same limitation as
CallOp)Consider using
rewriter.replaceOpWithNewOp<func::FuncOp>(...)with converted types, or ensure this pattern is registered with appropriate priority to run last.
74-85: LGTM! Region-aware lowering state design.The updated
LoweringStatewith per-regionqubitMapandregionMapfor tracking QC qubits per operation is a sound design for handling nested control flow during conversion.
1241-1321: LGTM!scf.ifconversion with proper region handling.The pattern correctly:
- Creates a new
scf.ifwith proper result types- Moves/inlines regions from the old operation
- Generates the else yield for empty else regions
- Updates qubit maps for both then/else regions and the parent region
1700-1727: LGTM! Dynamic legality predicates.The use of the
needChangeattribute for dynamic legality of SCF ops and direct type checking for func ops provides clean control over which operations need conversion.
Contributor
Author
|
@denialhaag @ystade Also the issue with the |
Member
Thanks a lot for the update! I will try to get to this on a train ride in the next few days. Already some thoughts:
For now, enjoy the winter holidays! 🙂 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp (1)
74-85: Comment refers to “Flux SSA values” instead of QCOMinor doc nit: the
LoweringState::qubitMapcomment still says “latest Flux SSA values”, but this pass maps QC qubits to QCO values. Updating the wording to “QCO SSA values” would avoid confusion.
🤖 Fix all issues with AI agents
In @mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h:
- Around line 897-929: The doc example for QCProgramBuilder::scfWhile is
incorrect: it shows using non-existent QCProgramBuilder helpers (yield(),
condition()) and uses OpBuilder& in the callbacks but calls QC ops as if on
QCProgramBuilder; update the example so the lambda bodies are accurate — either
call QCProgramBuilder methods on the outer builder (e.g., use
builder.scfCondition(...) / builder.yield equivalents) or show the correct
OpBuilder usage by creating explicit ops (e.g., b.create<scf::ConditionOp>(...)
and b.create<scf::YieldOp>(...)) and ensure the parameter type in the prose
matches the signature of scfWhile(const std::function<void(OpBuilder&)>&,...).
- Around line 1011-1035: The documentation example for the funcFunc method
contains an incorrect dialect type in the MLIR snippet. In the example showing
the generated MLIR code for funcFunc, the function parameter is declared with
type !qco.qubit, but since this is the QC builder, it should be !qc.qubit to
match the dialect and be consistent with the operation type used in the function
body. Update the example in the funcFunc documentation to change the argument
type from !qco.qubit to !qc.qubit.
In @mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h:
- Around line 1035-1266: The doc examples for scfFor, scfWhile, scfIf (and other
SCF/FUNC entries) are inconsistent with the declared callback signatures (e.g.,
scfFor(..., const std::function<ValueRange(OpBuilder&, Value, ValueRange)>&
body), scfWhile(..., std::function<ValueRange(OpBuilder&, ValueRange)>,
scfIf(..., std::function<ValueRange(OpBuilder&)>) ) and the fact that the
callbacks receive an OpBuilder& (not a QCOProgramBuilder with helpers like .h or
.scfYield). Update each example to match the actual signatures: use the correct
lambda parameter types/names (OpBuilder& plus Value/ValueRange where declared),
reference the lambda’s ValueRange parameter (not an outer initArgs), and either
call functions on the outer QCOProgramBuilder instance or show how to obtain QCO
helpers from the provided OpBuilder (or explicitly state that QCO-specific
helpers are invoked via the QCOProgramBuilder wrapper), so the example bodies
compile against scfFor, scfWhile, scfIf, scfCondition, scfYield and funcFunc
signatures.
In @mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:
- Around line 1040-1050: ConvertQCOFuncCallOp's matchAndRewrite creates a
func::CallOp with no results then calls rewriter.replaceOp(op,
adaptor.getOperands()), which incorrectly assumes all operands are qubits and
maps them 1:1 to results; instead, identify which operand/result types are
qubits vs classical (using the op's result types and
adaptor.getOperands()/op.getArgTypes()), build a vector of replacement values
that contains only the operand values corresponding to qubit results in the same
positional order as those qubit results, keep/classical results untouched,
create the new func::CallOp with the correct (non-qubit) result types, and call
rewriter.replaceOp(op, replacements) so only qubit results are dropped and
classical results are preserved.
- Around line 1070-1085: ConvertQCOFuncFuncOp currently replaces every argument
and drops results; instead, in matchAndRewrite inspect the original function
type (use op.getFunctionType() or op.getType()), iterate its input types and
build a new argumentTypes vector that maps only the qco qubit type to
qc::QubitType while preserving non-qubit types, then update only those
BlockArguments that correspond to qubit inputs (do not unconditionally set all
blockArg.setType), similarly iterate original result types and construct a
preserved/new result vector that rewrites only qubit results to qc::QubitType
and keeps classical results unchanged, and finally call
rewriter.getFunctionType(newInputs, newResults) and
op.setFunctionType(newFuncType); keep the changes localized in
ConvertQCOFuncFuncOp::matchAndRewrite and avoid dropping non-qubit result types.
- Around line 816-862: The current ConvertQCOScfIfOp::matchAndRewrite replaces
the original scf.if with values yielded from the then-region, breaking SSA
dominance and dropping else-branch results; instead, allocate or obtain a single
outer-scoped !qc.qubit value (or convert the scf.if result type to !qc.qubit and
keep the scf.if result) that is visible outside the if, write/assign the
branch-local qubit into that shared value in both then and else regions (replace
the branch scf.yield to yield nothing or to yield the shared reference), and
finally replace the original op’s uses with that outer/shared !qc.qubit value;
update logic in matchAndRewrite around newIf, the region inlining, scf::YieldOp
handling, and the rewriter.replaceOp call so you never forward a region-local
value to outer uses and you correctly handle both branches’ results.
In @mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:
- Around line 1584-1612: ConvertQCFuncCallOp currently treats every
operand/result as a qubit; change it to handle mixed classical/qubit values:
iterate op->getOperands() and for each operand check if its type is
qc::QubitType (use isa/qc::QubitType or equivalent) — if qubit push
qubitMap[operand] into the new argument list, otherwise push the original
operand unchanged; build the result type vector by mapping only qubit-typed
result types to qco::QubitType and preserving non-qubit result types as-is;
create the func::CallOp with that mixed arg list and mixed result types;
finally, when updating getState().qubitMap for the new callOp results, iterate
original op->getResults() paired with callOp->getResults() but only update
qubitMap for pairs where the original result was a qc::QubitType so classical
results are not inserted or overwritten.
- Around line 119-187: collectUniqueQubits currently only inspects
region.front().getOperations() and thus ignores non-entry blocks; either enforce
the single-block assumption or iterate all blocks: replace the single-block
usage by either asserting region.hasOneBlock() with a clear message, or loop
over region.getBlocks()/for (auto &block : region) and traverse
block.getOperations() so all operations' operands/results are collected and
state->regionMap and the "needChange" attrs are correctly computed for
constructs like func::FuncOp and scf ops.
In @mlir/unittests/conversion/test_conversion.cpp:
- Around line 124-127: The test uses string-serialized IR equality
(getOutputString -> outputString / checkString and ASSERT_EQ) which is brittle
to non-semantic formatting changes; update the test by either adding a brief
comment above the ASSERT_EQ explaining this limitation and why string comparison
is used, or replace the string comparison with a semantic IR comparison helper
(e.g., parse both strings back to an MLIR module and compare
canonicalized/dialect-normalized IR or use an existing module-equality utility)
for getOutputString usages around these assertions to make the checks robust.
- Around line 85-90: Tests repeat downcasts like
static_cast<mlir::qc::QCProgramBuilder&>(b) and
static_cast<qco::QCOProgramBuilder&>(b) with NOLINTs; introduce small helper
lambdas (e.g., asQC and asQCO that take OpBuilder& and return the appropriate
builder reference) and replace the repeated static_cast(...) calls (used before
.h, .x, etc.) with asQC(b).h(q0), asQC(b).x(q0), or asQCO(b).<op>(...) to remove
boilerplate while preserving behavior and NOLINT suppression.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cppmlir/unittests/conversion/test_conversion.cpp
🧰 Additional context used
🧠 Learnings (18)
📚 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/unittests/conversion/test_conversion.cppmlir/lib/Conversion/QCOToQC/QCOToQC.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/unittests/conversion/test_conversion.cppmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.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/unittests/conversion/test_conversion.cppmlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.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/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.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/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2026-01-04T22:25:13.956Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: src/qir/runner/Runner.cpp:70-76
Timestamp: 2026-01-04T22:25:13.956Z
Learning: In src/qir/runner/Runner.cpp, the REGISTER_SYMBOL macro intentionally performs dual registration (both llvm::sys::DynamicLibrary::AddSymbol and adding to manualSymbols vector) to ensure symbol resolution works correctly when compiling with shared libraries enabled, even though it may appear redundant.
Applied to files:
mlir/lib/Conversion/QCOToQC/QCOToQC.cppmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCOToQC/QCOToQC.cpp
📚 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/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.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/Builder/QCOProgramBuilder.hmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/Builder/QCOProgramBuilder.hmlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-09T04:30:29.071Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h:72-140
Timestamp: 2025-10-09T04:30:29.071Z
Learning: In the CrawlScheduler (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Scheduler.h), the persistent `layoutCopy` is updated across all lookahead layers via `remapQubitValue` calls. After each two-qubit gate is scheduled in a layer, its input qubits are remapped to output qubits, ensuring the next layer's crawl starts from those outputs. This progressive remapping through SSA values prevents the same operation from being scheduled in multiple lookahead layers.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-10-07T15:30:42.946Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1237
File: mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h:219-231
Timestamp: 2025-10-07T15:30:42.946Z
Learning: In the Layout class for MLIR quantum routing (mlir/include/mlir/Dialect/MQTOpt/Transforms/Transpilation/Layout.h), the swap method intentionally does NOT swap the hw fields in QubitInfo. This is correct because SSA values represent quantum states at fixed hardware locations, and only their program index associations change during a SWAP gate. The hw field indicates where an SSA value physically resides and remains constant.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 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/lib/Conversion/QCToQCO/QCToQCO.cpp
📚 Learning: 2025-12-10T02:20:01.189Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Applied to files:
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
🧬 Code graph analysis (3)
mlir/unittests/conversion/test_conversion.cpp (6)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
mlir(29-163)mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
mlir(30-171)mlir/lib/Conversion/QCOToQC/QCOToQC.cpp (1)
context(1141-1218)mlir/lib/Conversion/QCToQCO/QCToQCO.cpp (1)
context(1718-1785)mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (4)
measure(127-131)measure(127-127)measure(133-140)measure(133-133)mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (4)
measure(156-167)measure(156-156)measure(169-183)measure(169-169)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (19)
QCProgramBuilder(35-39)scfFor(451-461)scfFor(452-453)scfWhile(463-475)scfWhile(464-465)scfIf(477-499)scfIf(478-480)scfCondition(501-504)scfCondition(501-501)funcReturn(516-519)funcReturn(516-516)funcCall(510-514)funcCall(510-511)funcFunc(521-539)funcFunc(521-523)arithConstantIndex(545-549)arithConstantIndex(545-545)arithConstantBool(551-556)arithConstantBool(551-551)
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (25)
scfFor(604-632)scfFor(604-606)scfWhile(634-683)scfWhile(634-637)scfIf(685-722)scfIf(685-688)QCOProgramBuilder(38-42)scfCondition(724-728)scfCondition(724-725)scfYield(730-733)scfYield(730-730)funcReturn(748-751)funcReturn(748-748)funcCall(739-746)funcCall(739-739)funcFunc(752-775)funcFunc(752-754)arithConstantIndex(781-785)arithConstantIndex(781-781)arithConstantBool(787-792)arithConstantBool(787-787)validateQubitValue(132-140)validateQubitValue(132-132)updateQubitTracking(142-150)updateQubitTracking(142-143)
🪛 Cppcheck (2.19.0)
mlir/unittests/conversion/test_conversion.cpp
[information] 11-11: Include file
(missingInclude)
[information] 12-12: Include file
(missingInclude)
[information] 13-13: Include file
(missingInclude)
[information] 14-14: Include file
(missingInclude)
[information] 15-15: Include file
(missingInclude)
[information] 16-16: Include file
(missingInclude)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
[information] 28-28: Include file
(missingIncludeSystem)
[information] 29-29: Include file
(missingIncludeSystem)
[information] 30-30: Include file
(missingIncludeSystem)
[information] 31-31: Include file
(missingIncludeSystem)
[information] 32-32: Include file
(missingIncludeSystem)
[information] 33-33: Include file
(missingIncludeSystem)
mlir/lib/Conversion/QCOToQC/QCOToQC.cpp
[information] 16-16: Include file
(missingIncludeSystem)
[information] 17-17: Include file
(missingIncludeSystem)
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
mlir/lib/Conversion/QCToQCO/QCToQCO.cpp
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: 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). (16)
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🐍 Test (ubuntu-24.04-arm) / 🐍 ubuntu-24.04-arm
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🐍 Test (ubuntu-24.04) / 🐍 ubuntu-24.04
- GitHub Check: 🐍 Test (macos-15-intel) / 🐍 macos-15-intel
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🐍 Test (windows-2025) / 🐍 windows-2025
- GitHub Check: 🐍 Test (macos-15) / 🐍 macos-15
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Debug) / 🍎 macos-15 clang Debug
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🐍 Lint / 🚨 Lint
- GitHub Check: 🇨 Coverage / 📈 Coverage
🔇 Additional comments (3)
mlir/unittests/conversion/test_conversion.cpp (3)
1-36: LGTM!The copyright header and includes are appropriate. The
using namespace mlir;directive in a test .cpp file is acceptable and improves readability.
37-75: LGTM!The test fixture and helper function are well-designed. Note that
getOutputStringcorrectly uses a reference parameter, addressing the past review comment. The static linkage on the free function follows the project's coding guidelines.
77-521: Test logic correctly models dialect semantics.The tests accurately capture the semantic differences between QC and QCO dialects:
- QC: Imperative style with in-place qubit mutation
- QCO: SSA form with explicit qubit threading through regions
The conversion expectations are correct for all tested operations (scf.for, scf.while, scf.if, func.func).
Comment on lines
+897
to
+929
| /** | ||
| * @brief Constructs a scf.while operation without return values | ||
| * | ||
| * @param beforeBody Function that builds the before body of the while | ||
| * operation | ||
| * @param afterBody Function that builds the after body of the while operation | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfWhile([&](auto& b) { | ||
| * b.h(q0); | ||
| * auto res = b.measure(q0) | ||
| * b.condition(res) | ||
| * }, [&](auto& b) { | ||
| * b.x(q0); | ||
| * b.yield() | ||
| * }); | ||
| * ``` | ||
| * ```mlir | ||
| * scf.while : () -> () { | ||
| * qc.h %q0 : !qc.qubit | ||
| * %res = qc.measure %q0 : !qc.qubit -> i1 | ||
| * scf.condition(%tres) | ||
| * } do { | ||
| * qc.x %q0 : !qc.qubit | ||
| * scf.yield | ||
| * } | ||
| * ``` | ||
| */ | ||
| QCProgramBuilder& scfWhile(const std::function<void(OpBuilder&)>& beforeBody, | ||
| const std::function<void(OpBuilder&)>& afterBody); | ||
|
|
Contributor
There was a problem hiding this comment.
scfWhile doc example calls non-existent builder methods
The example uses:
builder.scfWhile([&](auto& b) {
b.h(q0);
auto res = b.measure(q0)
b.condition(res)
}, [&](auto& b) {
b.x(q0);
b.yield()
});But the public API here exposes scfCondition(...) and no yield() helper on QCProgramBuilder. Also, the callbacks take OpBuilder&, not a QCProgramBuilder&, so calling b.h(...) / b.measure(...) on b is misleading.
Consider updating the example to either:
- Use
builderinside the lambdas for QC operations andbuilder.scfCondition(...), or - Show explicit
b.create<scf::ConditionOp>(...)/b.create<scf::YieldOp>(...)calls onOpBuilder& b.
🤖 Prompt for AI Agents
In @mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h around lines 897 -
929, The doc example for QCProgramBuilder::scfWhile is incorrect: it shows using
non-existent QCProgramBuilder helpers (yield(), condition()) and uses OpBuilder&
in the callbacks but calls QC ops as if on QCProgramBuilder; update the example
so the lambda bodies are accurate — either call QCProgramBuilder methods on the
outer builder (e.g., use builder.scfCondition(...) / builder.yield equivalents)
or show the correct OpBuilder usage by creating explicit ops (e.g.,
b.create<scf::ConditionOp>(...) and b.create<scf::YieldOp>(...)) and ensure the
parameter type in the prose matches the signature of scfWhile(const
std::function<void(OpBuilder&)>&,...).
Comment on lines
+1011
to
+1035
| * @brief Constructs a func.func operation with return values | ||
| * | ||
| * @param name Name of the function that is called | ||
| * @param argTypes TypeRange of the arguments | ||
| * @param body Body of the function | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.funcFunc("test", argTypes, [&](OpBuilder& b, | ||
| * ValueRange args) { | ||
| * b.h(args[0]); | ||
| * b.funcReturn(); | ||
| * }) | ||
| * ``` | ||
| * ```mlir | ||
| * func.func @test(%arg0 : !qco.qubit) { | ||
| * qc.h %arg0 : !qc.qubit | ||
| * func.return | ||
| * } | ||
| * ``` | ||
| */ | ||
| QCProgramBuilder& | ||
| funcFunc(StringRef name, TypeRange argTypes, | ||
| const std::function<void(OpBuilder&, ValueRange)>& body); |
Contributor
There was a problem hiding this comment.
QC builder funcFunc doc still uses !qco.qubit
In the funcFunc example you have:
func.func @test(%arg0 : !qco.qubit) {
qc.h %arg0 : !qc.qubit
func.return
}This is the QC builder, so the argument type should be !qc.qubit, not !qco.qubit.
Please update the snippet to use !qc.qubit to avoid confusion between the QC and QCO dialects.
🤖 Prompt for AI Agents
In @mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h around lines 1011 -
1035, The documentation example for the funcFunc method contains an incorrect
dialect type in the MLIR snippet. In the example showing the generated MLIR code
for funcFunc, the function parameter is declared with type !qco.qubit, but since
this is the QC builder, it should be !qc.qubit to match the dialect and be
consistent with the operation type used in the function body. Update the example
in the funcFunc documentation to change the argument type from !qco.qubit to
!qc.qubit.
Comment on lines
+1035
to
+1266
| /** | ||
| * @brief Constructs a scf.for operation with iterArgs | ||
| * | ||
| * @param lowerbound Lowerbound of the loop | ||
| * @param upperbound Upperbound of the loop | ||
| * @param step Stepsize of the loop | ||
| * @param initArgs Initial arguments for the iterArgs | ||
| * @param body Function that builds the body of the for operation | ||
| * @return ValueRange of the results | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfFor(lb, ub, step, initArgs, [&](auto& b) { | ||
| * auto q1 = b.x(initArgs[0]); | ||
| * b.scfYield(q1); | ||
| }); | ||
| * ``` | ||
| * ```mlir | ||
| * %q1 = scf.for %iv = %lb to %ub step %step iter_args(%arg0 = %q0) -> | ||
| !qco.qubit { | ||
| * %q2 = qc.x %arg0 : !qco.qubit -> !qco.qubit | ||
| * scf.yield %q2 : !qco.qubit | ||
| * } | ||
| * ``` | ||
| */ | ||
| ValueRange | ||
| scfFor(Value lowerbound, Value upperbound, Value step, ValueRange initArgs, | ||
| const std::function<ValueRange(OpBuilder&, Value, ValueRange)>& body); | ||
| /** | ||
| * @brief Constructs a scf.while operation with return values | ||
| * | ||
| * @param args Arguments for the while loop | ||
| * @param beforeBody Function that builds the before body of the while | ||
| * operation | ||
| * @param afterBody Function that builds the after body of the while operation | ||
| * @return ValueRange of the results | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfWhile(args, [&](auto& b, ValueRange iterArgs) { | ||
| * auto q1 = b.h(iterArgs[0]); | ||
| * auto [q2, measureRes] = b.measure(q1); | ||
| * b.condition(measureRes); | ||
| * }, [&](auto& b, ValueRange iterArgs) { | ||
| * auto q1 = b.x(iterArgs[0]); | ||
| * b.scfYield(q1); | ||
| * }); | ||
| * ``` | ||
| * ```mlir | ||
| * %q1 = scf.while (%arg0 = %q0): (!qco.qubit) -> (!qco.qubit) { | ||
| * %q2 = qco.h(%arg0) | ||
| * %q3, %result = qco.measure %q2 : !qco.qubit | ||
| * scf.condition(%result) %q3 : !qco.qubit | ||
| * } do { | ||
| * ^bb0(%arg0 : !qco.qubit): | ||
| * %q4 = qco.x %arg0 : !qco.qubit -> !qco.qubit | ||
| * scf.yield %q4 : !qco.qubit | ||
| * } | ||
| * ``` | ||
| */ | ||
| ValueRange | ||
| scfWhile(ValueRange args, | ||
| const std::function<ValueRange(OpBuilder&, ValueRange)>& beforeBody, | ||
| const std::function<ValueRange(OpBuilder&, ValueRange)>& afterBody); | ||
|
|
||
| /** | ||
| * @brief Constructs a scf.if operation with return values | ||
| * | ||
| * @param condition Condition for the if operation | ||
| * @param qubits Qubits used in the if/else body | ||
| * @param thenBody Function that builds the then body of the if | ||
| * operation | ||
| * @param elseBody Function that builds the else body of the if operation | ||
| * @return ValueRange of the results | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfIf(condition, qubits, [&](auto& b) { | ||
| * auto q1 = b.h(q0); | ||
| * b.scfYield(q1); | ||
| * }, [&](auto& b) { | ||
| * auto q1 = b.x(q0); | ||
| * b.scfYield(q1); | ||
| * }); | ||
| * ``` | ||
| * ```mlir | ||
| * %q1 = scf.if %condition -> (!qco.qubit) { | ||
| * %q2 = qco.h %q0 : !qco.qubit -> !qco.qubit | ||
| * scf.yield %q2 : !qco.qubit | ||
| * } else { | ||
| * %q2 = qco.x %q0 : !qco.qubit -> !qco.qubit | ||
| * scf.yield %q2 : !qco.qubit | ||
| * } | ||
| * ``` | ||
| */ | ||
| ValueRange scfIf(Value condition, ValueRange qubits, | ||
| const std::function<ValueRange(OpBuilder&)>& thenBody, | ||
| const std::function<ValueRange(OpBuilder&)>& elseBody); | ||
|
|
||
| /** | ||
| * @brief Constructs a scf.condition operation with yielded values | ||
| * | ||
| * @param condition Condition for condition operation | ||
| * @param yieldedValues ValueRange of the yieldedValues | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfCondition(condition, yieldedValues); | ||
| * ``` | ||
| * ```mlir | ||
| * scf.condition(%condition) %q0 : !qco.qubit | ||
| * ``` | ||
| */ | ||
| QCOProgramBuilder& scfCondition(Value condition, ValueRange yieldedValues); | ||
|
|
||
| /** | ||
| * @brief Constructs a scf.yield operation with yielded values | ||
| * | ||
| * @param yieldedValues ValueRange of the yieldedValues | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.scfYield(yieldedValues); | ||
| * ``` | ||
| * ```mlir | ||
| * scf.yield %q0 : !qco.qubit | ||
| * ``` | ||
| */ | ||
| QCOProgramBuilder& scfYield(ValueRange yieldedValues); | ||
|
|
||
| //===--------------------------------------------------------------------===// | ||
| // Func operations | ||
| //===--------------------------------------------------------------------===// | ||
|
|
||
| /** | ||
| * @brief Constructs a func.return operation with return values | ||
| * | ||
| * @param returnValues ValueRange of the returned values | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.funcReturn( yieldedValues); | ||
| * ``` | ||
| * ```mlir | ||
| * func.return %q0 : !qco.qubit | ||
| * ``` | ||
| */ | ||
| QCOProgramBuilder& funcReturn(ValueRange returnValues); | ||
|
|
||
| /** | ||
| * @brief Constructs a func.call operation with return values | ||
| * | ||
| * @param name Name of the function that is called | ||
| * @param operands ValueRange of the used operands | ||
| * @return ValueRange of the results | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * auto q1 = builder.funcCall("test", {q0}); | ||
| * ``` | ||
| * ```mlir | ||
| * %q1 = func.call @test(%q0) : (!qco.qubit) -> !qco.qubit | ||
| * ``` | ||
| */ | ||
| ValueRange funcCall(StringRef name, ValueRange operands); | ||
|
|
||
| /** | ||
| * @brief Constructs a func.func operation with return values | ||
| * | ||
| * @param name Name of the function that is called | ||
| * @param argTypes TypeRange of the arguments | ||
| * @param resultTypes TypeRange of the results | ||
| * @param body Body of the function | ||
| * @return Reference to this builder for method chaining | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.funcFunc("test", argTypes, resultTypes, [&](OpBuilder& b, | ||
| * ValueRange args) { | ||
| * auto q1 = b.h(args[0]); | ||
| * b.funcReturn({q1}); | ||
| * }) | ||
| * ``` | ||
| * ```mlir | ||
| * func.func @test(%arg0 : !qco.qubit) -> !qco.qubit { | ||
| * %q1 = qco.h %arg0 : !qco.qubit -> !qco.qubit | ||
| * func.return %q1 : !qco.qubit | ||
| * } | ||
| * ``` | ||
| */ | ||
| QCOProgramBuilder& | ||
| funcFunc(StringRef name, TypeRange argTypes, TypeRange resultTypes, | ||
| const std::function<void(OpBuilder&, ValueRange)>& body); | ||
|
|
||
| //===--------------------------------------------------------------------===// | ||
| // Arith operations | ||
| //===--------------------------------------------------------------------===// | ||
|
|
||
| /** | ||
| * @brief Constructs a arith.constant of type Index with a given value | ||
| * | ||
| * @param index Value of the constant operation | ||
| * @return Result of the constant operation | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.arithConstantIndex(4); | ||
| * ``` | ||
| * ```mlir | ||
| * arith.constant 4 : index | ||
| * ``` | ||
| */ | ||
| Value arithConstantIndex(int i); | ||
|
|
||
| /** | ||
| * @brief Constructs a arith.constant of type i1 with a given bool value | ||
| * | ||
| * @param b Bool value of the constant operation | ||
| * @return Result of the constant operation | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.arithConstantBool(true); | ||
| * ``` | ||
| * ```mlir | ||
| * arith.constant 1 : i1 | ||
| * ``` | ||
| */ | ||
| Value arithConstantBool(bool b); |
Contributor
There was a problem hiding this comment.
Several SCF/FUNC doc examples don’t match the declared APIs
Within the new SCF/FUNC section, multiple examples are inconsistent with the signatures:
scfForexample body usesinitArgs[0], but the callback signature isValueRange initArgsin the header (body(OpBuilder&, Value, ValueRange)); the example should reference the lambda’sValueRangeparameter, not an outerinitArgs.scfIfexample callsbuilder.scfIf(condition, qubits, [&](auto& b) { ... });which is fine, but similar toscfFor/scfWhileyou should ensure the lambda parameters match the actual function type (they’reOpBuilder&and possiblyValueRange, not a freeauto&that can call arbitrary QCO-specific methods).- In several places, the examples imply calling QCO-specific helpers (
b.h,b.scfYield, etc.) on the lambda parameterb, but the declared callbacks takeOpBuilder&, which does not expose those methods.
These are documentation issues only, but they will confuse users trying to follow the examples literally. It’s worth tightening the examples so they line up with the declared callback signatures and the actual helper method names.
🤖 Prompt for AI Agents
In @mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h around lines 1035
- 1266, The doc examples for scfFor, scfWhile, scfIf (and other SCF/FUNC
entries) are inconsistent with the declared callback signatures (e.g.,
scfFor(..., const std::function<ValueRange(OpBuilder&, Value, ValueRange)>&
body), scfWhile(..., std::function<ValueRange(OpBuilder&, ValueRange)>,
scfIf(..., std::function<ValueRange(OpBuilder&)>) ) and the fact that the
callbacks receive an OpBuilder& (not a QCOProgramBuilder with helpers like .h or
.scfYield). Update each example to match the actual signatures: use the correct
lambda parameter types/names (OpBuilder& plus Value/ValueRange where declared),
reference the lambda’s ValueRange parameter (not an outer initArgs), and either
call functions on the outer QCOProgramBuilder instance or show how to obtain QCO
helpers from the provided OpBuilder (or explicitly state that QCO-specific
helpers are invoked via the QCOProgramBuilder wrapper), so the example bodies
compile against scfFor, scfWhile, scfIf, scfCondition, scfYield and funcFunc
signatures.
Comment on lines
+816
to
+862
| /** | ||
| * @brief Converts scf.if with value semantics to scf.if with memory semantics | ||
| * for qubit values | ||
| * | ||
| * @par Example: | ||
| * ```mlir | ||
| * %targets_out = scf.if %cond -> (!qco.qubit) { | ||
| * %q1 = qco.h %q0 : !qco.qubit -> !qco.qubit | ||
| * scf.yield %q1 : !qco.qubit | ||
| * } else { | ||
| * scf.yield %q0 : !qco.qubit | ||
| * } | ||
| * ``` | ||
| * is converted to | ||
| * ```mlir | ||
| * scf.if %cond { | ||
| * qc.x %q0 | ||
| * scf.yield | ||
| * } | ||
| * ``` | ||
| */ | ||
| struct ConvertQCOScfIfOp final : OpConversionPattern<scf::IfOp> { | ||
| using OpConversionPattern::OpConversionPattern; | ||
|
|
||
| LogicalResult | ||
| matchAndRewrite(scf::IfOp op, OpAdaptor /*adaptor*/, | ||
| ConversionPatternRewriter& rewriter) const override { | ||
| // Create the new if operation | ||
| auto newIf = | ||
| rewriter.create<scf::IfOp>(op.getLoc(), ValueRange{}, op.getCondition(), | ||
| op.getElseRegion().empty()); | ||
| // Inline the regions | ||
| rewriter.inlineRegionBefore(op.getThenRegion(), newIf.getThenRegion(), | ||
| newIf.getThenRegion().end()); | ||
| if (!op.getElseRegion().empty()) { | ||
| rewriter.inlineRegionBefore(op.getElseRegion(), newIf.getElseRegion(), | ||
| newIf.getElseRegion().end()); | ||
| } | ||
| // Erase the empty block that was created during the initialization | ||
| rewriter.eraseBlock(&newIf.getThenRegion().front()); | ||
|
|
||
| const auto& yield = | ||
| dyn_cast<scf::YieldOp>(newIf.getThenRegion().front().getTerminator()); | ||
|
|
||
| rewriter.replaceOp(op, yield->getOperands()); | ||
| return success(); | ||
| } |
Contributor
There was a problem hiding this comment.
ConvertQCOScfIfOp produces invalid SSA and ignores else-branch results
This pattern replaces the original scf.if op with the operands of the then-branch scf.yield:
const auto& yield =
dyn_cast<scf::YieldOp>(newIf.getThenRegion().front().getTerminator());
rewriter.replaceOp(op, yield->getOperands());Problems:
- The yielded values are defined inside the then-region; using them directly to replace uses of the outer
scf.ifresult violates SSA dominance (values from inside a region must not be used outside). - The else-branch result values are ignored; the
scf.ifresult is treated as if it always came from the then-branch, which is semantically wrong when branches yield different qubits.
You’ll need a different strategy: either keep the scf.if results (and convert the result types from !qco.qubit to !qc.qubit) or explicitly model the reference-semantics by rewriting users to refer to a pre-existing !qc.qubit value that is valid in both branches, rather than taking operands from a branch-local scf.yield.
🤖 Prompt for AI Agents
In @mlir/lib/Conversion/QCOToQC/QCOToQC.cpp around lines 816 - 862, The current
ConvertQCOScfIfOp::matchAndRewrite replaces the original scf.if with values
yielded from the then-region, breaking SSA dominance and dropping else-branch
results; instead, allocate or obtain a single outer-scoped !qc.qubit value (or
convert the scf.if result type to !qc.qubit and keep the scf.if result) that is
visible outside the if, write/assign the branch-local qubit into that shared
value in both then and else regions (replace the branch scf.yield to yield
nothing or to yield the shared reference), and finally replace the original op’s
uses with that outer/shared !qc.qubit value; update logic in matchAndRewrite
around newIf, the region inlining, scf::YieldOp handling, and the
rewriter.replaceOp call so you never forward a region-local value to outer uses
and you correctly handle both branches’ results.
Comment on lines
+119
to
+187
| /** | ||
| * @brief Recursively collects all the QC qubit references used by an | ||
| * operation and store them in map | ||
| * | ||
| * @param Operation The operation that is currently traversed | ||
| * @param state The lowering state | ||
| * @param ctx The MLIRContext of the current program | ||
| * @return llvm::Setvector<Value> The set of unique QC qubit references | ||
| */ | ||
| static llvm::SetVector<Value> | ||
| collectUniqueQubits(Operation* op, LoweringState* state, MLIRContext* ctx) { | ||
| // get the regions of the current operation | ||
| const auto& regions = op->getRegions(); | ||
| SetVector<Value> uniqueQubits; | ||
| for (auto& region : regions) { | ||
| // skip empty regions e.g. empty else region of an If operation | ||
| if (region.empty()) { | ||
| continue; | ||
| } | ||
| // iterate over all operations inside the region | ||
| // currently assumes that each region only has one block | ||
| for (auto& operation : region.front().getOperations()) { | ||
| // check if the operation has an region, if yes recursively collect the | ||
| // qubits | ||
| if (operation.getNumRegions() > 0) { | ||
| const auto& qubits = collectUniqueQubits(&operation, state, ctx); | ||
| uniqueQubits.set_union(qubits); | ||
| } | ||
| // collect qubits form the operands | ||
| for (const auto& operand : operation.getOperands()) { | ||
| if (operand.getType() == qc::QubitType::get(ctx)) { | ||
| uniqueQubits.insert(operand); | ||
| } | ||
| } | ||
| // collect qubits from the results | ||
| for (const auto& result : operation.getResults()) { | ||
| if (result.getType() == qc::QubitType::get(ctx)) { | ||
| uniqueQubits.insert(result); | ||
| } | ||
| } | ||
| // mark scf terminator operations if they need to return a value after the | ||
| // conversion | ||
| if ((llvm::isa<scf::YieldOp>(operation) || | ||
| llvm::isa<scf::ConditionOp>(operation)) && | ||
| !uniqueQubits.empty()) { | ||
| operation.setAttr("needChange", StringAttr::get(ctx, "yes")); | ||
| } | ||
| // mark func.return operation for functions that need to return a qubit | ||
| // value | ||
| if (llvm::isa<func::ReturnOp>(operation)) { | ||
| if (auto func = operation.getParentOfType<func::FuncOp>()) { | ||
| if (!func.getArgumentTypes().empty() && | ||
| func.getArgumentTypes().front() == qc::QubitType::get(ctx)) { | ||
| operation.setAttr("needChange", StringAttr::get(ctx, "yes")); | ||
| state->regionMap[func] = uniqueQubits; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| // mark scf operations that need to be changed afterwards | ||
| if (!uniqueQubits.empty() && | ||
| (llvm::isa<scf::IfOp>(op) || (llvm::isa<scf::ForOp>(op)) || | ||
| llvm::isa<scf::WhileOp>(op))) { | ||
| state->regionMap[op] = uniqueQubits; | ||
| op->setAttr("needChange", StringAttr::get(ctx, "yes")); | ||
| } | ||
| return uniqueQubits; | ||
| } |
Contributor
There was a problem hiding this comment.
collectUniqueQubits still assumes single-block regions
The traversal only inspects region.front().getOperations() and carries a comment:
// currently assumes that each region only has one block
for (auto& operation : region.front().getOperations()) {
...
}For operations whose regions may legally contain multiple blocks (e.g., general func.func bodies), this will silently ignore qubits used in non-entry blocks, leading to incomplete regionMap and missing needChange attributes.
Either:
- Assert the precondition (
assert(region.hasOneBlock() && "...")) if multi-block regions are truly unsupported, or - Iterate all blocks in the region and process operations from each block.
🤖 Prompt for AI Agents
In @mlir/lib/Conversion/QCToQCO/QCToQCO.cpp around lines 119 - 187,
collectUniqueQubits currently only inspects region.front().getOperations() and
thus ignores non-entry blocks; either enforce the single-block assumption or
iterate all blocks: replace the single-block usage by either asserting
region.hasOneBlock() with a clear message, or loop over region.getBlocks()/for
(auto &block : region) and traverse block.getOperations() so all operations'
operands/results are collected and state->regionMap and the "needChange" attrs
are correctly computed for constructs like func::FuncOp and scf ops.
Comment on lines
+77
to
+521
| TEST_F(ConversionTest, ScfForTest) { | ||
| // Test conversion from qc to qco for scf.for operation | ||
| auto input = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto c0 = b.arithConstantIndex(0); | ||
| auto c1 = b.arithConstantIndex(1); | ||
| auto c2 = b.arithConstantIndex(2); | ||
| b.scfFor(c0, c2, c1, [&](OpBuilder& b) { | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).x(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCToQCO()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QC-QCO conversion for scf.for"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto c0 = b.arithConstantIndex(0); | ||
| auto c1 = b.arithConstantIndex(1); | ||
| auto c2 = b.arithConstantIndex(2); | ||
| auto scfForRes = b.scfFor( | ||
| c0, c2, c1, ValueRange{q0}, | ||
| [&](OpBuilder& b, Value, ValueRange iterArgs) { | ||
| auto | ||
| q1 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h(iterArgs[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).x(q1); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h(q2); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield( | ||
| ValueRange{q3}); | ||
| return q3; | ||
| }); | ||
| b.h(scfForRes[0]); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, ScfForTest2) { | ||
| // Test conversion from qco to qc for scf.for operation | ||
| auto input = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto c0 = b.arithConstantIndex(0); | ||
| auto c1 = b.arithConstantIndex(1); | ||
| auto c2 = b.arithConstantIndex(2); | ||
| auto scfForRes = b.scfFor( | ||
| c0, c2, c1, ValueRange{q0}, | ||
| [&](OpBuilder& b, Value, ValueRange iterArgs) { | ||
| auto | ||
| q1 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h(iterArgs[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).x( | ||
| q1); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h( | ||
| q2); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield( | ||
| ValueRange{q3}); | ||
| return q3; | ||
| }); | ||
| b.h(scfForRes[0]); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCOToQC()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QCO-QC conversion for scf.for"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto c0 = b.arithConstantIndex(0); | ||
| auto c1 = b.arithConstantIndex(1); | ||
| auto c2 = b.arithConstantIndex(2); | ||
| b.scfFor(c0, c2, c1, [&](OpBuilder& b) { | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).x(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, ScfWhileTest) { | ||
| // Test conversion from qc to qco for scf.while operation | ||
| auto input = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| b.scfWhile( | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| measureResult = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).measure(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).scfCondition( | ||
| measureResult); | ||
| }, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCToQCO()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QC-QCO conversion for scf.while"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto scfWhileResult = b.scfWhile( | ||
| ValueRange{q0}, | ||
| [&](OpBuilder& b, ValueRange iterArgs) { | ||
| auto | ||
| [q1, | ||
| measureResult] = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).measure( | ||
| iterArgs | ||
| [0]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfCondition( | ||
| measureResult, ValueRange{q1}); | ||
| return q1; | ||
| }, | ||
| [&](OpBuilder& b, ValueRange iterArgs) { | ||
| auto | ||
| q1 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h( | ||
| iterArgs | ||
| [0]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q1); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield({q2}); | ||
| return q2; | ||
| }); | ||
| b.h(scfWhileResult[0]); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, ScfWhileTest2) { | ||
| // Test conversion from qco to qc for scf.while operation | ||
| auto input = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto scfWhileResult = b.scfWhile( | ||
| ValueRange{q0}, | ||
| [&](OpBuilder& b, ValueRange iterArgs) { | ||
| auto | ||
| [q1, | ||
| measureResult] = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).measure( | ||
| iterArgs | ||
| [0]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfCondition( | ||
| measureResult, ValueRange{q1}); | ||
| return q1; | ||
| }, | ||
| [&](OpBuilder& b, ValueRange iterArgs) { | ||
| auto | ||
| q1 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h( | ||
| iterArgs | ||
| [0]); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q1); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield({q2}); | ||
| return q2; | ||
| }); | ||
| b.h(scfWhileResult[0]); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCOToQC()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QCO-QC conversion for scf.while"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| b.scfWhile( | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| measureResult = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).measure( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).scfCondition( | ||
| measureResult); | ||
| }, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, ScfIfTest) { | ||
| // Test conversion from qc to qco for scf.if operation | ||
| auto input = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto measure = b.measure(q0); | ||
| b.scfIf( | ||
| measure, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(q0); | ||
| }, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCToQCO()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QC-QCO conversion for scf.if"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto [q1, measureResult] = b.measure(q0); | ||
| auto scfIfResult = b.scfIf( | ||
| measureResult, ValueRange{q1}, | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| q2 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h(q1); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q2); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield(q3); | ||
| return q3; | ||
| }, | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| q2 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q1); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h( | ||
| q2); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield(q3); | ||
| return q3; | ||
| }); | ||
| b.h(scfIfResult[0]); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, ScfIfTest2) { | ||
| // Test conversion from qco to qc for scf.if operation | ||
| auto input = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto [q1, measureResult] = b.measure(q0); | ||
| auto scfIfResult = b.scfIf( | ||
| measureResult, ValueRange{q1}, | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| q2 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).h(q1); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q2); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield(q3); | ||
| return q3; | ||
| }, | ||
| [&](OpBuilder& b) { | ||
| auto | ||
| q2 = // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).y( | ||
| q1); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h( | ||
| q2); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).scfYield(q3); | ||
| return q3; | ||
| }); | ||
| b.h(scfIfResult[0]); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCOToQC()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QCO-QC conversion for scf.if"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto measure = b.measure(q0); | ||
| b.scfIf( | ||
| measure, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(q0); | ||
| }, | ||
| [&](OpBuilder& | ||
| b) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y( | ||
| q0); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| }); | ||
| b.h(q0); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, FuncFuncTest) { | ||
| // Test conversion from qc to qco for func.func operation | ||
| auto input = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| b.funcCall("test", q0); | ||
| b.h(q0); | ||
| b.funcFunc( | ||
| "test", q0.getType(), | ||
| [&](OpBuilder& b, | ||
| ValueRange | ||
| args) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).funcReturn(); | ||
| }); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCToQCO()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QC-QCO conversion for func.func"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto q1 = b.funcCall("test", q0); | ||
| b.h(q1[0]); | ||
| b.funcFunc( | ||
| "test", q0.getType(), q0.getType(), [&](OpBuilder& b, ValueRange args) { | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y(q2); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).funcReturn(q3); | ||
| }); | ||
| }); | ||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } | ||
|
|
||
| TEST_F(ConversionTest, FuncFuncTest2) { | ||
| // Test conversion from qco to qc for func.func operation | ||
| auto input = buildQCOIR([](mlir::qco::QCOProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| auto q1 = b.funcCall("test", q0); | ||
| b.h(q1[0]); | ||
| b.funcFunc( | ||
| "test", q0.getType(), q0.getType(), [&](OpBuilder& b, ValueRange args) { | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q2 = static_cast<mlir::qco::QCOProgramBuilder&>(b).h(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto q3 = static_cast<mlir::qco::QCOProgramBuilder&>(b).y(q2); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qco::QCOProgramBuilder&>(b).funcReturn(q3); | ||
| }); | ||
| }); | ||
|
|
||
| PassManager pm(context.get()); | ||
| pm.addPass(createQCOToQC()); | ||
| if (failed(pm.run(input.get()))) { | ||
| FAIL() << "Conversion error during QCO-QC conversion for func.func"; | ||
| } | ||
|
|
||
| auto expectedOutput = buildQCIR([](mlir::qc::QCProgramBuilder& b) { | ||
| auto q0 = b.allocQubit(); | ||
| b.funcCall("test", q0); | ||
| b.h(q0); | ||
| b.funcFunc( | ||
| "test", q0.getType(), | ||
| [&](OpBuilder& b, | ||
| ValueRange | ||
| args) { // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).y(args[0]); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).funcReturn(); | ||
| }); | ||
| }); | ||
|
|
||
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider more descriptive test names.
The test names use a "Test"/"Test2" suffix pattern that doesn't clearly indicate the conversion direction. More explicit names would improve readability and maintainability.
For example:
ScfForTest→ScfForQCToQCOTestScfForTest2→ScfForQCOToQCTest
This pattern would make the bidirectional nature of the tests immediately clear.
Comment on lines
+85
to
+90
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).x(q0); | ||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| static_cast<mlir::qc::QCProgramBuilder&>(b).h(q0); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a helper to reduce static_cast boilerplate.
The repeated pattern of downcasting OpBuilder& to specific builder types with NOLINT suppressions appears throughout the test lambdas. While the current approach is functional and the suppressions are appropriate, a helper function could reduce boilerplate.
💡 Optional refactoring idea
You could define helper lambdas that capture the specific builder type:
auto asQC = [](OpBuilder& b) -> mlir::qc::QCProgramBuilder& {
return static_cast<mlir::qc::QCProgramBuilder&>(b);
};
auto asQCO = [](OpBuilder& b) -> qco::QCOProgramBuilder& {
return static_cast<qco::QCOProgramBuilder&>(b);
};Then use:
asQC(b).h(q0);
asQC(b).x(q0);However, this might not significantly improve readability given the context, so the current approach is acceptable.
Also applies to: 110-119, 142-150
🤖 Prompt for AI Agents
In @mlir/unittests/conversion/test_conversion.cpp around lines 85 - 90, Tests
repeat downcasts like static_cast<mlir::qc::QCProgramBuilder&>(b) and
static_cast<qco::QCOProgramBuilder&>(b) with NOLINTs; introduce small helper
lambdas (e.g., asQC and asQCO that take OpBuilder& and return the appropriate
builder reference) and replace the repeated static_cast(...) calls (used before
.h, .x, etc.) with asQC(b).h(q0), asQC(b).x(q0), or asQCO(b).<op>(...) to remove
boilerplate while preserving behavior and NOLINT suppression.
Comment on lines
+124
to
+127
| const auto outputString = getOutputString(input); | ||
| const auto checkString = getOutputString(expectedOutput); | ||
|
|
||
| ASSERT_EQ(outputString, checkString); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
String comparison for IR validation is brittle but acceptable.
The tests compare string-serialized IR for equivalence, which means any non-semantic formatting change will cause test failures. This is a common pattern in MLIR tests but has known fragility.
Consider documenting this limitation or exploring semantic IR comparison if these tests become maintenance-heavy in the future.
Also applies to: 177-180, 240-243
🤖 Prompt for AI Agents
In @mlir/unittests/conversion/test_conversion.cpp around lines 124 - 127, The
test uses string-serialized IR equality (getOutputString -> outputString /
checkString and ASSERT_EQ) which is brittle to non-semantic formatting changes;
update the test by either adding a brief comment above the ASSERT_EQ explaining
this limitation and why string comparison is used, or replace the string
comparison with a semantic IR comparison helper (e.g., parse both strings back
to an MLIR module and compare canonicalized/dialect-normalized IR or use an
existing module-equality utility) for getOutputString usages around these
assertions to make the checks robust.
denialhaag
requested changes
Jan 7, 2026
Member
denialhaag
left a comment
denialhaag
left a comment
There was a problem hiding this comment.
Hey, @li-mingbao! Thanks a lot for your work on this! I have now had a first look at the code and left some comments. I think this is already looking really good overall!
| * } | ||
| * ``` | ||
| */ | ||
| QCProgramBuilder& scfFor(Value lowerbound, Value upperbound, Value step, |
Member
There was a problem hiding this comment.
It might make sense to allow something like std::variant<int, Value> here, similar to the builder methods of parametrized gates:
core/mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h
Lines 390 to 391 in 7778e0a
This also applies to other builder methods of both dialects.
| * ``` | ||
| */ | ||
| QCProgramBuilder& scfFor(Value lowerbound, Value upperbound, Value step, | ||
| const std::function<void(OpBuilder&)>& body); |
Member
There was a problem hiding this comment.
You have already mentioned this yourself, but just as a reminder:
The signature of body should be changed to align with the changes introduced in #1402.
This also applies to other builder methods of both dialects.
Comment on lines
+917
to
+924
| * scf.while : () -> () { | ||
| * qc.h %q0 : !qc.qubit | ||
| * %res = qc.measure %q0 : !qc.qubit -> i1 | ||
| * scf.condition(%tres) | ||
| * } do { | ||
| * qc.x %q0 : !qc.qubit | ||
| * scf.yield | ||
| * } |
Member
There was a problem hiding this comment.
I would maybe indent the regions, similar to how it is done in the MLIR docs.
Suggested change
| * scf.while : () -> () { | |
| * qc.h %q0 : !qc.qubit | |
| * %res = qc.measure %q0 : !qc.qubit -> i1 | |
| * scf.condition(%tres) | |
| * } do { | |
| * qc.x %q0 : !qc.qubit | |
| * scf.yield | |
| * } | |
| * scf.while : () -> () { | |
| * qc.h %q0 : !qc.qubit | |
| * %res = qc.measure %q0 : !qc.qubit -> i1 | |
| * scf.condition(%tres) | |
| * } do { | |
| * qc.x %q0 : !qc.qubit | |
| * scf.yield | |
| * } |
This might also apply to docstrings of other builder methods of both dialects.
Comment on lines
+1037
to
+1071
| //===--------------------------------------------------------------------===// | ||
| // Arith operations | ||
| //===--------------------------------------------------------------------===// | ||
|
|
||
| /** | ||
| * @brief Constructs a arith.constant of type Index with a given value | ||
| * | ||
| * @param index Value of the constant operation | ||
| * @return Result of the constant operation | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.arithConstantIndex(4); | ||
| * ``` | ||
| * ```mlir | ||
| * arith.constant 4 : index | ||
| * ``` | ||
| */ | ||
| Value arithConstantIndex(int index); | ||
|
|
||
| /** | ||
| * @brief Constructs a arith.constant of type i1 with a given bool value | ||
| * | ||
| * @param b Bool value of the constant operation | ||
| * @return Result of the constant operation | ||
| * | ||
| * @par Example: | ||
| * ```c++ | ||
| * builder.arithConstantBool(true); | ||
| * ``` | ||
| * ```mlir | ||
| * arith.constant 1 : i1 | ||
| * ``` | ||
| */ | ||
| Value arithConstantBool(bool b); |
Member
There was a problem hiding this comment.
If we go with std::variant as suggested in https://github.com/munich-quantum-toolkit/core/pull/1396/changes#r2669307611, this might not be needed. 🤔
| * is converted to | ||
| * ```mlir | ||
| * scf.if %cond { | ||
| * qc.x %q0 |
Member
There was a problem hiding this comment.
The assembly format for the QC dialect now also adds the type.
Suggested change
| * qc.x %q0 | |
| * qc.x %q0 !qc.qubit |
This may apply to other docstrings for both conversion directions.
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for the conversion of the
scfoperationsscf.while,scf.for,scf.ifand the conversion of additional functions between theqcand theqcodialect. This allows the conversion of programs with nonlinear controlflow.Reopened since #1391 got automatically closed after the branch was deleted.
Checklist: