✨ Add OpenQASM-to-QC translation#1671
Open
J4MMlE wants to merge 23 commits into
Open
Conversation
burgholzer
added
feature
denialhaag
reviewed
Apr 29, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
denialhaag
reviewed
May 21, 2026
denialhaag
reviewed
May 21, 2026
denialhaag
reviewed
May 21, 2026
Member
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a direct OpenQASM3 → MLIR QC dialect translator: public API header, visitor-based importer with gate dispatch, complete translation logic (gates, measurements, controls, compound gates, if/else), tool integration into mqt-cc, and parameterized unit tests with shared QASM fixtures. ChangesQASM3-to-QC Direct Importer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)mlir/tools/mqt-cc/mqt-cc.cppmlir/tools/mqt-cc/mqt-cc.cpp:11:10: fatal error: 'mlir/Compiler/CompilerPipeline.h' file not found ... [truncated 2200 characters] ... od in file "src/clang/cFrontend_decl.ml" (inlined), line 54, characters 4-52 mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cppmlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp:11:10: fatal error: 'mlir/Dialect/QC/Translation/TranslateQASM3ToQC.h' file not found ... [truncated 2200 characters] ... decl_funct.add_method.f in file "src/clang/cFrontend_decl.ml", line 48, characters 12-91 mlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cppmlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cpp:11:10: fatal error: 'TestCaseUtils.h' file not found ... [truncated 2200 characters] ... tend__CFrontend_decl.CFrontend_decl_funct.add_method in file "src/clang/cFrontend_decl.ml" (inlined), line 54, characters 4-52 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/tools/mqt-cc/mqt-cc.cpp (1)
142-146: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRemove duplicate
MemRefDialectregistration.
memref::MemRefDialectandmlir::memref::MemRefDialectare the same dialect here, so this registers it twice. Keep a single insertion to avoid redundant setup and future confusion.Suggested diff
registry.insert<func::FuncDialect>(); registry.insert<memref::MemRefDialect>(); registry.insert<scf::SCFDialect>(); registry.insert<LLVM::LLVMDialect>(); - registry.insert<mlir::memref::MemRefDialect>(); registry.insert<qtensor::QTensorDialect>();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mlir/tools/mqt-cc/mqt-cc.cpp` around lines 142 - 146, The code registers the same dialect twice via registry.insert<memref::MemRefDialect>() and registry.insert<mlir::memref::MemRefDialect>(); remove one of these duplicate registry.insert calls (keep either the unqualified memref::MemRefDialect or the fully-qualified mlir::memref::MemRefDialect for clarity) so the dialect is registered only once and the subsequent registry.insert<qtensor::QTensorDialect>() remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mlir/include/mlir/Dialect/QC/Translation/TranslateQASM3ToQC.h`:
- Around line 28-32: Add a Doxygen comment for the stream overload
translateQASM3ToQC(MLIRContext* context, std::istream& input) matching the
filename overload: describe the function purpose, document parameters (context
and input), state the error/reporting behavior (errors are reported via
llvm::errs()), and specify the return contract (returns an OwningOpRef<ModuleOp>
or nullptr on failure). Place the comment immediately above the second
declaration so the stream-variant’s contract is clear to callers.
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp`:
- Around line 891-909: The helper function convertComparisonKind is currently
dead code because the BinaryExpression branch in translateCondition throws
unconditionally; either remove convertComparisonKind to avoid unusedFunction
warnings or rewire translateCondition's BinaryExpression case to call
convertComparisonKind (and handle result) so register comparisons are actually
translated; locate convertComparisonKind and the BinaryExpression branch in
translateCondition and implement one of the two fixes consistently (delete the
unused static function and its declaration, or replace the throw in
translateCondition's BinaryExpression path with logic that maps
::qc::ComparisonKind via convertComparisonKind and uses the resulting
arith::CmpIPredicate).
- Around line 587-610: The modifier-controls loop iterates ctrlIdx and indexes
expandedOperands without checking bounds, which can UB if there are fewer
operands; add a guard inside the for (const auto& [n, positive] : ctrlSpec) /
inner for (size_t i = 0; i < n; ++i, ++ctrlIdx) before accessing
expandedOperands[ctrlIdx] to verify ctrlIdx < expandedOperands.size() and that
expandedOperands[ctrlIdx].size() == 1, and if not throw a qasm3::CompilerError
(using stmt->debugInfo) with a clear message (similar to the existing implicit
OQ2 control check) so posControls/negControls pushes are safe.
- Around line 666-678: The loop uses concat<const Value>(iterQubits,
posControls, negControls) which applies a const qualifier to the MLIR handle
type; change the call to concat<Value>(...) (drop the const) so the range yields
plain Value handles and matches project conventions—update the concat call where
seen (llvm::SmallDenseSet<Value> seen) and the subsequent for (auto q : ...)
loop that precedes emitGate(dispIt->second, iterQubits, params, posControls,
negControls, invert).
- Around line 825-837: The unary-negation branch currently assumes
unaryExpr->operand is an IndexedIdentifier and passes the result of
std::dynamic_pointer_cast<qasm3::IndexedIdentifier>(unaryExpr->operand) directly
to lookupBitValue, which can yield nullptr and cause a crash; update this branch
to check that idExpr is non-null and if it is null throw a CompilerError (or use
the project's existing error reporting) with a clear message (e.g., "unsupported
operand for unary negation: expected IndexedIdentifier") instead of calling
lookupBitValue; reference UnaryExpression, IndexedIdentifier, lookupBitValue and
the CompilerError path when locating and changing the code.
- Around line 742-766: The compound-gate body loop in the bodyFn lambda silently
drops unhandled statement kinds; update the for-loop over gate.body (inside
bodyFn) to add a terminal else branch that emits a clear diagnostic/throws
(using the statement's debugInfo when available) rather than ignoring unknown
types, so any unrecognized bodyStmt triggers a failure; locate the bodyFn lambda
and add the else to complement the existing
GateCallStatement/BarrierStatement/ResetStatement branches (which call
applyGateCallStatement, resolveGateOperandInScope, builder.barrier,
builder.reset) and produce a descriptive error/exception mentioning the
unexpected statement kind and debugInfo.
- Around line 312-331: The default branch of the switch on sizedTy->type
currently does nothing and can silently skip allocating registers (leaving
declarations populated but no register in qubitRegisters or classicalRegisters);
update the default case inside the switch (the branch handling sizedTy->type) to
explicitly fail fast for unsupported sized types (e.g., Float, Angle) by
reporting a clear error via the translation context/builder error API (or
throwing/returning an error) that includes the identifier and the unsupported
type, so unsupported declarations are rejected immediately instead of causing
later failures.
---
Outside diff comments:
In `@mlir/tools/mqt-cc/mqt-cc.cpp`:
- Around line 142-146: The code registers the same dialect twice via
registry.insert<memref::MemRefDialect>() and
registry.insert<mlir::memref::MemRefDialect>(); remove one of these duplicate
registry.insert calls (keep either the unqualified memref::MemRefDialect or the
fully-qualified mlir::memref::MemRefDialect for clarity) so the dialect is
registered only once and the subsequent
registry.insert<qtensor::QTensorDialect>() remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49e31367-7ed5-4787-8afb-2adf9d0d361d
📒 Files selected for processing (10)
mlir/include/mlir/Dialect/QC/Translation/TranslateQASM3ToQC.hmlir/lib/Dialect/QC/Translation/CMakeLists.txtmlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cppmlir/tools/mqt-cc/mqt-cc.cppmlir/unittests/Dialect/QC/Translation/CMakeLists.txtmlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cppmlir/unittests/programs/CMakeLists.txtmlir/unittests/programs/qasm_programs.cppmlir/unittests/programs/qasm_programs.htest/circuits/bell.qasm
💤 Files with no reviewable changes (1)
- test/circuits/bell.qasm
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp (1)
369-406: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
visitGateStatementsilently accepts user-defined gates that later fail with a misleading "Unknown gate" error.For a non-opaque, non-standard gate declaration, this method validates
paramNames/qubitNamesinto locals that are never stored, and never inserts the gate intogates. When the same gate is invoked,applyGateCallStatement(line 533-534) raisesUnknown gate '<name>', which is misleading — the gate is in fact a compound gate that was intentionally dropped (per the "Drop support for compound gates for now" commit).Either fail fast here with a clear diagnostic, or remove the dead local validation. The former gives users a useful error pointing at the declaration site.
♻️ Suggested change — fail fast with a clear message
void visitGateStatement(std::shared_ptr<qasm3::GateDeclaration> stmt) override { auto id = stmt->identifier; if (stmt->isOpaque) { if (!gates.contains(id)) { throw qasm3::CompilerError("Unsupported opaque gate '" + id + "'.", stmt->debugInfo); } return; } if (gates.contains(id)) { if (std::dynamic_pointer_cast<qasm3::StandardGate>(gates[id])) { return; // ignore redeclaration of standard gate } throw qasm3::CompilerError("Gate '" + id + "' already declared.", stmt->debugInfo); } - std::vector<std::string> paramNames; - for (const auto& p : stmt->parameters->identifiers) { - if (std::ranges::find(paramNames, p->identifier) != paramNames.end()) { - throw qasm3::CompilerError("Parameter '" + p->identifier + - "' already declared in gate '" + id + - "'.", - stmt->debugInfo); - } - paramNames.push_back(p->identifier); - } - std::vector<std::string> qubitNames; - for (const auto& q : stmt->qubits->identifiers) { - if (std::ranges::find(qubitNames, q->identifier) != qubitNames.end()) { - throw qasm3::CompilerError("Qubit '" + q->identifier + - "' already declared in gate '" + id + - "'.", - stmt->debugInfo); - } - qubitNames.push_back(q->identifier); - } + throw qasm3::CompilerError( + "User-defined (compound) gate '" + id + + "' is not supported by direct MLIR import.", + stmt->debugInfo); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp` around lines 369 - 406, The visitGateStatement function currently validates parameters/qubits for non-opaque, non-standard gate declarations but never inserts the gate into the gates map, causing later "Unknown gate" errors; change visitGateStatement to fail fast for such compound (non-opaque, non-standard) gate declarations by throwing a qasm3::CompilerError (use stmt->debugInfo) with a clear message like "Compound gates are not supported: '<id>'" instead of silently dropping them (refer to visitGateStatement and applyGateCallStatement to locate the flow), so users get a direct diagnostic at the declaration site.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp`:
- Around line 369-406: The visitGateStatement function currently validates
parameters/qubits for non-opaque, non-standard gate declarations but never
inserts the gate into the gates map, causing later "Unknown gate" errors; change
visitGateStatement to fail fast for such compound (non-opaque, non-standard)
gate declarations by throwing a qasm3::CompilerError (use stmt->debugInfo) with
a clear message like "Compound gates are not supported: '<id>'" instead of
silently dropping them (refer to visitGateStatement and applyGateCallStatement
to locate the flow), so users get a direct diagnostic at the declaration site.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f89c1f1-14f7-475f-bfe6-57b1e1604f05
📒 Files selected for processing (3)
mlir/include/mlir/Dialect/QC/Translation/TranslateQASM3ToQC.hmlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cppmlir/tools/mqt-cc/mqt-cc.cpp
denialhaag
force-pushed
the
open-qasm
branch
2 times, most recently
from
78a4271 to
4ec599d
Compare
Member
|
Follow-ups are collected in #1733. |
Member
denialhaag
force-pushed
the
open-qasm
branch
2 times, most recently
from
e9ab789 to
ad5a0af
Compare
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp (1)
761-783:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStill missing a fail-fast path for unsupported compound-gate body statements.
bodyFnhandles gate calls, barriers, and resets, then falls through. Any other parsed statement kind is silently ignored, which changes the gate semantics without a diagnostic instead of failing translation.♻️ Proposed fix
} else if (const auto reset = std::dynamic_pointer_cast<qasm3::ResetStatement>( bodyStmt)) { for (auto q : resolveGateOperandInScope(reset->gate, localScope, reset->debugInfo)) { builder.reset(q); } + } else { + throw qasm3::CompilerError( + "Unsupported statement in compound gate body.", + bodyStmt ? bodyStmt->debugInfo : debugInfo); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp` around lines 761 - 783, The loop over gate.body (iterating bodyStmt) currently handles GateCallStatement, BarrierStatement, and ResetStatement and silently ignores any other statement kinds; update the loop to add an explicit fail-fast branch for unsupported/unknown statement types: when bodyStmt is not a GateCallStatement, BarrierStatement, or ResetStatement, emit a clear translation error or abort translation (e.g., use the existing translation error reporting mechanism) that includes the gate name and the unexpected statement kind; reference the existing symbols bodyStmt, gate.body, applyGateCallStatement, resolveGateOperandInScope, builder.barrier, and builder.reset when locating where to insert this diagnostic so unhandled compound-gate body statements no longer get dropped silently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp`:
- Around line 734-742: The loop that builds targetsMap misuses an iterator after
targets.push_back, causing UB: when it == targets.end() you must not call
std::distance on the invalidated iterator. Fix the inner loop in
TranslateQASM3ToQC.cpp that iterates over gateOperands/gate.targetNames by
computing the index safely—either recompute the iterator/index after push_back
(e.g., find again) or use the new index directly (targets.size() - 1) when you
just pushed the target—and then push that index into
targetsMap[gate.targetNames[i]] instead of using the old it-based distance.
- Around line 816-828: emitBlockStatements currently only accepts
GateCallStatement and throws for any other quantum statements (breaking if/else
branches that contain reset, barrier, etc.); update this function to dispatch on
the actual statement subtype instead of only GateCallStatement: detect
ResetStatement, BarrierStatement (and any other quantum statement types the
translator supports) via dynamic_pointer_cast and call the corresponding
lowering helpers (e.g. applyResetStatement, applyBarrierStatement) or refactor
to a single applyStatement dispatcher that handles all supported
qasm3::Statement subclasses; keep using the same debugInfo and qubitRegisters
context when invoking these helpers so branch bodies lower identically to
top-level code.
- Around line 303-349: Move handling of compile-time-only declarations before
performing runtime allocations: after computing sizedTy and size, check
stmt->isConst first and if true, handle only the allowed initializer case (same
measure initializer logic using visitMeasureAssignment with lhsId) then return
without calling builder.allocQubitRegister or builder.allocClassicalBitRegister;
otherwise continue into the existing switch on sizedTy->type (which uses
allocQubitRegister, allocClassicalBitRegister, qubitRegisters,
classicalRegisters) and keep the remaining initializer error handling intact.
In `@mlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cpp`:
- Around line 101-352: Add a test case exercising the negative-control modifier
`negctrl @` so the `emitGate` negative-control bracketing path is covered:
create a QASM fixture (e.g. `singleNegControlledX` and optionally a mixed
`ctrl`+`negctrl` fixture) and add corresponding entries to the test
instantiation using the existing test harness `QASM3TranslationTestCase` with
the builder reference (e.g. add QASM3TranslationTestCase{"SingleNegControlledX",
qasm::singleNegControlledX, MQT_NAMED_BUILDER(qc::singleNegControlledX)} and/or
a mixed case name) so the translation for `negctrl` is validated by the unit
suite.
- Around line 343-352: The five test entries
(QASM3TranslationTestCase{"CtrlTwo", "CtrlTwoMixed", "SimpleIf", "IfTwoQubits",
"IfElse"}) use fully-qualified mlir::qc:: but the rest of the file uses the
unqualified qc:: thanks to using namespace mlir;. Replace mlir::qc::ctrlTwo,
mlir::qc::ctrlTwoMixed, mlir::qc::simpleIf, mlir::qc::ifTwoQubits, and
mlir::qc::ifElse with qc::ctrlTwo, qc::ctrlTwoMixed, qc::simpleIf,
qc::ifTwoQubits, and qc::ifElse respectively so the QASM3TranslationTestCase
entries and the MQT_NAMED_BUILDER calls are consistent with the surrounding
code.
In `@mlir/unittests/programs/CMakeLists.txt`:
- Around line 9-11: MLIRQASMPrograms is missing the project compile/warning
options because it doesn't use MQT::ProjectOptions; call
mqt_mlir_target_use_project_options(MLIRQASMPrograms) (which applies
MQT::ProjectOptions privately to the target) or otherwise link
MQT::ProjectOptions PRIVATE to MLIRQASMPrograms similar to
MLIRQuantumComputationPrograms so the options propagate at build time; apply the
same change for the other program targets MLIRQCPrograms, MLIRQCOPrograms, and
MLIRQIRPrograms for consistency.
---
Duplicate comments:
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp`:
- Around line 761-783: The loop over gate.body (iterating bodyStmt) currently
handles GateCallStatement, BarrierStatement, and ResetStatement and silently
ignores any other statement kinds; update the loop to add an explicit fail-fast
branch for unsupported/unknown statement types: when bodyStmt is not a
GateCallStatement, BarrierStatement, or ResetStatement, emit a clear translation
error or abort translation (e.g., use the existing translation error reporting
mechanism) that includes the gate name and the unexpected statement kind;
reference the existing symbols bodyStmt, gate.body, applyGateCallStatement,
resolveGateOperandInScope, builder.barrier, and builder.reset when locating
where to insert this diagnostic so unhandled compound-gate body statements no
longer get dropped silently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 143f39d4-1c86-4b0c-adbf-4013a109ac54
📒 Files selected for processing (10)
mlir/include/mlir/Dialect/QC/Translation/TranslateQASM3ToQC.hmlir/lib/Dialect/QC/Translation/CMakeLists.txtmlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cppmlir/tools/mqt-cc/mqt-cc.cppmlir/unittests/Dialect/QC/Translation/CMakeLists.txtmlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cppmlir/unittests/programs/CMakeLists.txtmlir/unittests/programs/qasm_programs.cppmlir/unittests/programs/qasm_programs.htest/circuits/bell.qasm
💤 Files with no reviewable changes (1)
- test/circuits/bell.qasm
Comment on lines
+303
to
+349
| const auto ty = std::get<1>(stmt->type); | ||
| const auto sizedTy = | ||
| std::dynamic_pointer_cast<qasm3::DesignatedType<uint64_t>>(ty); | ||
| if (!sizedTy) { | ||
| throw qasm3::CompilerError("Only sized types are supported.", | ||
| stmt->debugInfo); | ||
| } | ||
| const auto size = static_cast<int64_t>(sizedTy->getDesignator()); | ||
|
|
||
| switch (sizedTy->type) { | ||
| case qasm3::Qubit: { | ||
| auto reg = builder.allocQubitRegister(size); | ||
| SmallVector<Value> qubits; | ||
| qubits.reserve(static_cast<size_t>(size)); | ||
| for (int64_t i = 0; i < size; ++i) { | ||
| qubits.push_back(reg[static_cast<size_t>(i)]); | ||
| } | ||
| qubitRegisters[id] = std::move(qubits); | ||
| break; | ||
| } | ||
| case qasm3::Bit: | ||
| case qasm3::Int: | ||
| case qasm3::Uint: { | ||
| classicalRegisters[id] = builder.allocClassicalBitRegister(size, id); | ||
| break; | ||
| } | ||
| default: | ||
| throw qasm3::CompilerError("Unsupported declaration type.", | ||
| stmt->debugInfo); | ||
| } | ||
|
|
||
| // Handle initializer (measure only) | ||
| if (stmt->expression) { | ||
| const auto& expr = stmt->expression->expression; | ||
| if (const auto measureExpr = | ||
| std::dynamic_pointer_cast<qasm3::MeasureExpression>(expr)) { | ||
| auto lhsId = std::make_shared<qasm3::IndexedIdentifier>(id); | ||
| visitMeasureAssignment(lhsId, measureExpr, stmt->debugInfo); | ||
| return; | ||
| } | ||
| if (stmt->isConst) { | ||
| return; // nothing to emit | ||
| } | ||
| throw qasm3::CompilerError( | ||
| "Only measure statements are supported as initializers.", | ||
| stmt->debugInfo); | ||
| } |
Contributor
There was a problem hiding this comment.
Handle const declarations before the runtime-allocation switch.
The current order forces every declaration through runtime storage lowering first. That means const bit/int/uint declarations still emit QC classical registers, and const float/angle declarations are rejected by the default arm even though they only need the const-eval/type-check environments. Compile-time-only declarations should bypass allocQubitRegister/allocClassicalBitRegister entirely.
♻️ Proposed fix
const auto size = static_cast<int64_t>(sizedTy->getDesignator());
+ if (stmt->isConst) {
+ return;
+ }
+
switch (sizedTy->type) {
case qasm3::Qubit: {
auto reg = builder.allocQubitRegister(size);
@@
- if (stmt->isConst) {
- return; // nothing to emit
- }
throw qasm3::CompilerError(
"Only measure statements are supported as initializers.",
stmt->debugInfo);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp` around lines 303 -
349, Move handling of compile-time-only declarations before performing runtime
allocations: after computing sizedTy and size, check stmt->isConst first and if
true, handle only the allowed initializer case (same measure initializer logic
using visitMeasureAssignment with lhsId) then return without calling
builder.allocQubitRegister or builder.allocClassicalBitRegister; otherwise
continue into the existing switch on sizedTy->type (which uses
allocQubitRegister, allocClassicalBitRegister, qubitRegisters,
classicalRegisters) and keep the remaining initializer error handling intact.
Comment on lines
+734
to
+742
| for (size_t i = 0; i < gate.targetNames.size(); ++i) { | ||
| for (auto target : gateOperands[i]) { | ||
| auto* it = llvm::find(targets, target); | ||
| if (it == targets.end()) { | ||
| targets.push_back(target); | ||
| } | ||
| targetsMap[gate.targetNames[i]].push_back( | ||
| std::distance(targets.begin(), it)); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't reuse end() after push_back when building the compound-gate target map.
When a target is first seen, it == targets.end(). targets.push_back(target) invalidates that iterator, so std::distance(targets.begin(), it) is UB. This can miswire the local-scope qubit mapping for compound-gate expansion.
🛠️ Proposed fix
for (size_t i = 0; i < gate.targetNames.size(); ++i) {
for (auto target : gateOperands[i]) {
- auto* it = llvm::find(targets, target);
- if (it == targets.end()) {
+ auto* it = llvm::find(targets, target);
+ size_t index = 0;
+ if (it == targets.end()) {
+ index = targets.size();
targets.push_back(target);
+ } else {
+ index = static_cast<size_t>(std::distance(targets.begin(), it));
}
- targetsMap[gate.targetNames[i]].push_back(
- std::distance(targets.begin(), it));
+ targetsMap[gate.targetNames[i]].push_back(index);
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp` around lines 734 -
742, The loop that builds targetsMap misuses an iterator after
targets.push_back, causing UB: when it == targets.end() you must not call
std::distance on the invalidated iterator. Fix the inner loop in
TranslateQASM3ToQC.cpp that iterates over gateOperands/gate.targetNames by
computing the index safely—either recompute the iterator/index after push_back
(e.g., find again) or use the new index directly (targets.size() - 1) when you
just pushed the target—and then push that index into
targetsMap[gate.targetNames[i]] instead of using the old it-based distance.
Comment on lines
+816
to
+828
| void emitBlockStatements( | ||
| const std::vector<std::shared_ptr<qasm3::Statement>>& statements, | ||
| const std::shared_ptr<qasm3::DebugInfo>& debugInfo) { | ||
| for (const auto& statement : statements) { | ||
| auto gateCall = | ||
| std::dynamic_pointer_cast<qasm3::GateCallStatement>(statement); | ||
| if (gateCall == nullptr) { | ||
| throw qasm3::CompilerError( | ||
| "Only quantum statements are supported in if/else blocks.", | ||
| debugInfo); | ||
| } | ||
| applyGateCallStatement(gateCall, qubitRegisters); | ||
| } |
Contributor
There was a problem hiding this comment.
if branches are narrower than the rest of the translator.
This helper only accepts GateCallStatement, so branch bodies containing supported quantum statements like reset and barrier still fail here. That breaks the PR’s “if/else over quantum statements” contract even though those statements already lower correctly at top level.
♻️ Proposed fix
void emitBlockStatements(
const std::vector<std::shared_ptr<qasm3::Statement>>& statements,
const std::shared_ptr<qasm3::DebugInfo>& debugInfo) {
for (const auto& statement : statements) {
- auto gateCall =
- std::dynamic_pointer_cast<qasm3::GateCallStatement>(statement);
- if (gateCall == nullptr) {
+ if (const auto gateCall =
+ std::dynamic_pointer_cast<qasm3::GateCallStatement>(statement)) {
+ applyGateCallStatement(gateCall, qubitRegisters);
+ } else if (const auto barrier =
+ std::dynamic_pointer_cast<qasm3::BarrierStatement>(
+ statement)) {
+ SmallVector<Value> qubits;
+ for (const auto& gate : barrier->gates) {
+ auto resolved = resolveGateOperand(gate, barrier->debugInfo);
+ qubits.append(resolved.begin(), resolved.end());
+ }
+ builder.barrier(qubits);
+ } else if (const auto reset =
+ std::dynamic_pointer_cast<qasm3::ResetStatement>(
+ statement)) {
+ for (auto q : resolveGateOperand(reset->gate, reset->debugInfo)) {
+ builder.reset(q);
+ }
+ } else {
throw qasm3::CompilerError(
"Only quantum statements are supported in if/else blocks.",
- debugInfo);
+ statement ? statement->debugInfo : debugInfo);
}
- applyGateCallStatement(gateCall, qubitRegisters);
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QC/Translation/TranslateQASM3ToQC.cpp` around lines 816 -
828, emitBlockStatements currently only accepts GateCallStatement and throws for
any other quantum statements (breaking if/else branches that contain reset,
barrier, etc.); update this function to dispatch on the actual statement subtype
instead of only GateCallStatement: detect ResetStatement, BarrierStatement (and
any other quantum statement types the translator supports) via
dynamic_pointer_cast and call the corresponding lowering helpers (e.g.
applyResetStatement, applyBarrierStatement) or refactor to a single
applyStatement dispatcher that handles all supported qasm3::Statement
subclasses; keep using the same debugInfo and qubitRegisters context when
invoking these helpers so branch bodies lower identically to top-level code.
Comment on lines
+101
to
+352
| INSTANTIATE_TEST_SUITE_P( | ||
| QASM3TranslationProgramsTest, QASM3TranslationTest, | ||
| testing::Values( | ||
|
|
||
| QASM3TranslationTestCase{"AllocQubit", qasm::allocQubit, | ||
| MQT_NAMED_BUILDER(qc::allocQubit)}, | ||
| QASM3TranslationTestCase{"AllocQubitRegister", qasm::allocQubitRegister, | ||
| MQT_NAMED_BUILDER(qc::allocQubitRegister)}, | ||
| QASM3TranslationTestCase{ | ||
| "AllocMultipleQubitRegisters", qasm::allocMultipleQubitRegisters, | ||
| MQT_NAMED_BUILDER(qc::allocMultipleQubitRegisters)}, | ||
| QASM3TranslationTestCase{"AllocLargeRegister", qasm::allocLargeRegister, | ||
| MQT_NAMED_BUILDER(qc::allocLargeRegister)}, | ||
| QASM3TranslationTestCase{ | ||
| "SingleMeasurementToSingleBit", qasm::singleMeasurementToSingleBit, | ||
| MQT_NAMED_BUILDER(qc::singleMeasurementToSingleBit)}, | ||
| QASM3TranslationTestCase{ | ||
| "RepeatedMeasurementToSameBit", qasm::repeatedMeasurementToSameBit, | ||
| MQT_NAMED_BUILDER(qc::repeatedMeasurementToSameBit)}, | ||
| QASM3TranslationTestCase{ | ||
| "RepeatedMeasurementToDifferentBits", | ||
| qasm::repeatedMeasurementToDifferentBits, | ||
| MQT_NAMED_BUILDER(qc::repeatedMeasurementToDifferentBits)}, | ||
| QASM3TranslationTestCase{ | ||
| "MultipleClassicalRegistersAndMeasurements", | ||
| qasm::multipleClassicalRegistersAndMeasurements, | ||
| MQT_NAMED_BUILDER(qc::multipleClassicalRegistersAndMeasurements)}, | ||
| QASM3TranslationTestCase{ | ||
| "ResetQubitAfterSingleOp", qasm::resetQubitAfterSingleOp, | ||
| MQT_NAMED_BUILDER(qc::resetQubitAfterSingleOp)}, | ||
| QASM3TranslationTestCase{ | ||
| "ResetMultipleQubitsAfterSingleOp", | ||
| qasm::resetMultipleQubitsAfterSingleOp, | ||
| MQT_NAMED_BUILDER(qc::resetMultipleQubitsAfterSingleOp)}, | ||
| QASM3TranslationTestCase{ | ||
| "RepeatedResetAfterSingleOp", qasm::repeatedResetAfterSingleOp, | ||
| MQT_NAMED_BUILDER(qc::repeatedResetAfterSingleOp)}, | ||
| QASM3TranslationTestCase{"GlobalPhase", qasm::globalPhase, | ||
| MQT_NAMED_BUILDER(qc::globalPhase)}, | ||
| QASM3TranslationTestCase{"Identity", qasm::identity, | ||
| MQT_NAMED_BUILDER(qc::identity)}, | ||
| QASM3TranslationTestCase{ | ||
| "SingleControlledIdentity", qasm::singleControlledIdentity, | ||
| MQT_NAMED_BUILDER(qc::singleControlledIdentity)}, | ||
| QASM3TranslationTestCase{ | ||
| "MultipleControlledIdentity", qasm::multipleControlledIdentity, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledIdentity)}, | ||
| QASM3TranslationTestCase{"X", qasm::x, MQT_NAMED_BUILDER(qc::x)}, | ||
| QASM3TranslationTestCase{"SingleControlledX", qasm::singleControlledX, | ||
| MQT_NAMED_BUILDER(qc::singleControlledX)}, | ||
| QASM3TranslationTestCase{"MultipleControlledX", | ||
| qasm::multipleControlledX, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledX)}, | ||
| QASM3TranslationTestCase{"Y", qasm::y, MQT_NAMED_BUILDER(qc::y)}, | ||
| QASM3TranslationTestCase{"SingleControlledY", qasm::singleControlledY, | ||
| MQT_NAMED_BUILDER(qc::singleControlledY)}, | ||
| QASM3TranslationTestCase{"MultipleControlledY", | ||
| qasm::multipleControlledY, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledY)}, | ||
| QASM3TranslationTestCase{"Z", qasm::z, MQT_NAMED_BUILDER(qc::z)}, | ||
| QASM3TranslationTestCase{"SingleControlledZ", qasm::singleControlledZ, | ||
| MQT_NAMED_BUILDER(qc::singleControlledZ)}, | ||
| QASM3TranslationTestCase{"MultipleControlledZ", | ||
| qasm::multipleControlledZ, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledZ)}, | ||
| QASM3TranslationTestCase{"H", qasm::h, MQT_NAMED_BUILDER(qc::h)}, | ||
| QASM3TranslationTestCase{"SingleControlledH", qasm::singleControlledH, | ||
| MQT_NAMED_BUILDER(qc::singleControlledH)}, | ||
| QASM3TranslationTestCase{"MultipleControlledH", | ||
| qasm::multipleControlledH, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledH)}, | ||
| QASM3TranslationTestCase{"S", qasm::s, MQT_NAMED_BUILDER(qc::s)}, | ||
| QASM3TranslationTestCase{"SingleControlledS", qasm::singleControlledS, | ||
| MQT_NAMED_BUILDER(qc::singleControlledS)}, | ||
| QASM3TranslationTestCase{"MultipleControlledS", | ||
| qasm::multipleControlledS, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledS)}, | ||
| QASM3TranslationTestCase{"Sdg", qasm::sdg, MQT_NAMED_BUILDER(qc::sdg)}, | ||
| QASM3TranslationTestCase{"SingleControlledSdg", | ||
| qasm::singleControlledSdg, | ||
| MQT_NAMED_BUILDER(qc::singleControlledSdg)}, | ||
| QASM3TranslationTestCase{"MultipleControlledSdg", | ||
| qasm::multipleControlledSdg, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledSdg)}, | ||
| QASM3TranslationTestCase{"T", qasm::t_, MQT_NAMED_BUILDER(qc::t_)}, | ||
| QASM3TranslationTestCase{"SingleControlledT", qasm::singleControlledT, | ||
| MQT_NAMED_BUILDER(qc::singleControlledT)}, | ||
| QASM3TranslationTestCase{"MultipleControlledT", | ||
| qasm::multipleControlledT, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledT)}, | ||
| QASM3TranslationTestCase{"Tdg", qasm::tdg, MQT_NAMED_BUILDER(qc::tdg)}, | ||
| QASM3TranslationTestCase{"SingleControlledTdg", | ||
| qasm::singleControlledTdg, | ||
| MQT_NAMED_BUILDER(qc::singleControlledTdg)}, | ||
| QASM3TranslationTestCase{"MultipleControlledTdg", | ||
| qasm::multipleControlledTdg, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledTdg)}, | ||
| QASM3TranslationTestCase{"SX", qasm::sx, MQT_NAMED_BUILDER(qc::sx)}, | ||
| QASM3TranslationTestCase{"SingleControlledSX", qasm::singleControlledSx, | ||
| MQT_NAMED_BUILDER(qc::singleControlledSx)}, | ||
| QASM3TranslationTestCase{"MultipleControlledSX", | ||
| qasm::multipleControlledSx, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledSx)}, | ||
| QASM3TranslationTestCase{"SXdg", qasm::sxdg, | ||
| MQT_NAMED_BUILDER(qc::sxdg)}, | ||
| QASM3TranslationTestCase{"SingleControlledSXdg", | ||
| qasm::singleControlledSxdg, | ||
| MQT_NAMED_BUILDER(qc::singleControlledSxdg)}, | ||
| QASM3TranslationTestCase{"MultipleControlledSXdg", | ||
| qasm::multipleControlledSxdg, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledSxdg)}, | ||
| QASM3TranslationTestCase{"RX", qasm::rx, MQT_NAMED_BUILDER(qc::rx)}, | ||
| QASM3TranslationTestCase{"SingleControlledRX", qasm::singleControlledRx, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRx)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRX", | ||
| qasm::multipleControlledRx, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRx)}, | ||
| QASM3TranslationTestCase{"RY", qasm::ry, MQT_NAMED_BUILDER(qc::ry)}, | ||
| QASM3TranslationTestCase{"SingleControlledRY", qasm::singleControlledRy, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRy)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRY", | ||
| qasm::multipleControlledRy, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRy)}, | ||
| QASM3TranslationTestCase{"RZ", qasm::rz, MQT_NAMED_BUILDER(qc::rz)}, | ||
| QASM3TranslationTestCase{"SingleControlledRZ", qasm::singleControlledRz, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRz)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRZ", | ||
| qasm::multipleControlledRz, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRz)}, | ||
| QASM3TranslationTestCase{"P", qasm::p, MQT_NAMED_BUILDER(qc::p)}, | ||
| QASM3TranslationTestCase{"SingleControlledP", qasm::singleControlledP, | ||
| MQT_NAMED_BUILDER(qc::singleControlledP)}, | ||
| QASM3TranslationTestCase{"MultipleControlledP", | ||
| qasm::multipleControlledP, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledP)}, | ||
| QASM3TranslationTestCase{"R", qasm::r, MQT_NAMED_BUILDER(qc::r)}, | ||
| QASM3TranslationTestCase{"SingleControlledR", qasm::singleControlledR, | ||
| MQT_NAMED_BUILDER(qc::singleControlledR)}, | ||
| QASM3TranslationTestCase{"MultipleControlledR", | ||
| qasm::multipleControlledR, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledR)}, | ||
| QASM3TranslationTestCase{"U2", qasm::u2, MQT_NAMED_BUILDER(qc::u2)}, | ||
| QASM3TranslationTestCase{"SingleControlledU2", qasm::singleControlledU2, | ||
| MQT_NAMED_BUILDER(qc::singleControlledU2)}, | ||
| QASM3TranslationTestCase{"MultipleControlledU2", | ||
| qasm::multipleControlledU2, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledU2)}, | ||
| QASM3TranslationTestCase{"U", qasm::u, MQT_NAMED_BUILDER(qc::u)}, | ||
| QASM3TranslationTestCase{"SingleControlledU", qasm::singleControlledU, | ||
| MQT_NAMED_BUILDER(qc::singleControlledU)}, | ||
| QASM3TranslationTestCase{"MultipleControlledU", | ||
| qasm::multipleControlledU, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledU)}, | ||
| QASM3TranslationTestCase{"SWAP", qasm::swap, | ||
| MQT_NAMED_BUILDER(qc::swap)}, | ||
| QASM3TranslationTestCase{"SingleControlledSWAP", | ||
| qasm::singleControlledSwap, | ||
| MQT_NAMED_BUILDER(qc::singleControlledSwap)}, | ||
| QASM3TranslationTestCase{"MultipleControlledSWAP", | ||
| qasm::multipleControlledSwap, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledSwap)}, | ||
| QASM3TranslationTestCase{"iSWAP", qasm::iswap, | ||
| MQT_NAMED_BUILDER(qc::iswap)}, | ||
| QASM3TranslationTestCase{"SingleControllediSWAP", | ||
| qasm::singleControlledIswap, | ||
| MQT_NAMED_BUILDER(qc::singleControlledIswap)}, | ||
| QASM3TranslationTestCase{ | ||
| "MultipleControllediSWAP", qasm::multipleControlledIswap, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledIswap)}, | ||
| QASM3TranslationTestCase{"InverseISWAP", qasm::inverseIswap, | ||
| MQT_NAMED_BUILDER(qc::inverseIswap)}, | ||
| QASM3TranslationTestCase{ | ||
| "InverseMultiControlledISWAP", qasm::inverseMultipleControlledIswap, | ||
| MQT_NAMED_BUILDER(qc::inverseMultipleControlledIswap)}, | ||
| QASM3TranslationTestCase{"DCX", qasm::dcx, MQT_NAMED_BUILDER(qc::dcx)}, | ||
| QASM3TranslationTestCase{"SingleControlledDCX", | ||
| qasm::singleControlledDcx, | ||
| MQT_NAMED_BUILDER(qc::singleControlledDcx)}, | ||
| QASM3TranslationTestCase{"MultipleControlledDCX", | ||
| qasm::multipleControlledDcx, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledDcx)}, | ||
| QASM3TranslationTestCase{"ECR", qasm::ecr, MQT_NAMED_BUILDER(qc::ecr)}, | ||
| QASM3TranslationTestCase{"SingleControlledECR", | ||
| qasm::singleControlledEcr, | ||
| MQT_NAMED_BUILDER(qc::singleControlledEcr)}, | ||
| QASM3TranslationTestCase{"MultipleControlledECR", | ||
| qasm::multipleControlledEcr, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledEcr)}, | ||
| QASM3TranslationTestCase{"RXX", qasm::rxx, MQT_NAMED_BUILDER(qc::rxx)}, | ||
| QASM3TranslationTestCase{"SingleControlledRXX", | ||
| qasm::singleControlledRxx, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRxx)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRXX", | ||
| qasm::multipleControlledRxx, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRxx)}, | ||
| QASM3TranslationTestCase{"TripleControlledRXX", | ||
| qasm::tripleControlledRxx, | ||
| MQT_NAMED_BUILDER(qc::tripleControlledRxx)}, | ||
| QASM3TranslationTestCase{"RYY", qasm::ryy, MQT_NAMED_BUILDER(qc::ryy)}, | ||
| QASM3TranslationTestCase{"SingleControlledRYY", | ||
| qasm::singleControlledRyy, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRyy)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRYY", | ||
| qasm::multipleControlledRyy, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRyy)}, | ||
| QASM3TranslationTestCase{"RZX", qasm::rzx, MQT_NAMED_BUILDER(qc::rzx)}, | ||
| QASM3TranslationTestCase{"SingleControlledRZX", | ||
| qasm::singleControlledRzx, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRzx)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRZX", | ||
| qasm::multipleControlledRzx, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRzx)}, | ||
| QASM3TranslationTestCase{"RZZ", qasm::rzz, MQT_NAMED_BUILDER(qc::rzz)}, | ||
| QASM3TranslationTestCase{"SingleControlledRZZ", | ||
| qasm::singleControlledRzz, | ||
| MQT_NAMED_BUILDER(qc::singleControlledRzz)}, | ||
| QASM3TranslationTestCase{"MultipleControlledRZZ", | ||
| qasm::multipleControlledRzz, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledRzz)}, | ||
| QASM3TranslationTestCase{"XXPlusYY", qasm::xxPlusYY, | ||
| MQT_NAMED_BUILDER(qc::xxPlusYY)}, | ||
| QASM3TranslationTestCase{ | ||
| "SingleControlledXXPlusYY", qasm::singleControlledXxPlusYY, | ||
| MQT_NAMED_BUILDER(qc::singleControlledXxPlusYY)}, | ||
| QASM3TranslationTestCase{ | ||
| "MultipleControlledXXPlusYY", qasm::multipleControlledXxPlusYY, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledXxPlusYY)}, | ||
| QASM3TranslationTestCase{"XXMinusYY", qasm::xxMinusYY, | ||
| MQT_NAMED_BUILDER(qc::xxMinusYY)}, | ||
| QASM3TranslationTestCase{ | ||
| "SingleControlledXXMinusYY", qasm::singleControlledXxMinusYY, | ||
| MQT_NAMED_BUILDER(qc::singleControlledXxMinusYY)}, | ||
| QASM3TranslationTestCase{ | ||
| "MultipleControlledXXMinusYY", qasm::multipleControlledXxMinusYY, | ||
| MQT_NAMED_BUILDER(qc::multipleControlledXxMinusYY)}, | ||
| QASM3TranslationTestCase{"Barrier", qasm::barrier, | ||
| MQT_NAMED_BUILDER(qc::barrier)}, | ||
| QASM3TranslationTestCase{"BarrierTwoQubits", qasm::barrierTwoQubits, | ||
| MQT_NAMED_BUILDER(qc::barrierTwoQubits)}, | ||
| QASM3TranslationTestCase{"BarrierMultipleQubits", | ||
| qasm::barrierMultipleQubits, | ||
| MQT_NAMED_BUILDER(qc::barrierMultipleQubits)}, | ||
| QASM3TranslationTestCase{"CtrlTwo", qasm::ctrlTwo, | ||
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwo)}, | ||
| QASM3TranslationTestCase{"CtrlTwoMixed", qasm::ctrlTwoMixed, | ||
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwoMixed)}, | ||
| QASM3TranslationTestCase{"SimpleIf", qasm::simpleIf, | ||
| MQT_NAMED_BUILDER(mlir::qc::simpleIf)}, | ||
| QASM3TranslationTestCase{"IfTwoQubits", qasm::ifTwoQubits, | ||
| MQT_NAMED_BUILDER(mlir::qc::ifTwoQubits)}, | ||
| QASM3TranslationTestCase{"IfElse", qasm::ifElse, | ||
| MQT_NAMED_BUILDER(mlir::qc::ifElse)})); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Missing coverage for negctrl @. The PR lists negctrl @ among supported modifiers, but no fixture/case exercises it (only ctrl, ctrl(n), and inv). Consider adding a negative-control case (e.g. singleNegControlledX / mixed ctrl+negctrl) so the negative-control bracketing path in emitGate is validated.
I can draft the QASM fixture plus the matching QCProgramBuilder reference if helpful. If this is intentionally deferred to #1733, feel free to dismiss.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cpp` around
lines 101 - 352, Add a test case exercising the negative-control modifier
`negctrl @` so the `emitGate` negative-control bracketing path is covered:
create a QASM fixture (e.g. `singleNegControlledX` and optionally a mixed
`ctrl`+`negctrl` fixture) and add corresponding entries to the test
instantiation using the existing test harness `QASM3TranslationTestCase` with
the builder reference (e.g. add QASM3TranslationTestCase{"SingleNegControlledX",
qasm::singleNegControlledX, MQT_NAMED_BUILDER(qc::singleNegControlledX)} and/or
a mixed case name) so the translation for `negctrl` is validated by the unit
suite.
Comment on lines
+343
to
+352
| QASM3TranslationTestCase{"CtrlTwo", qasm::ctrlTwo, | ||
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwo)}, | ||
| QASM3TranslationTestCase{"CtrlTwoMixed", qasm::ctrlTwoMixed, | ||
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwoMixed)}, | ||
| QASM3TranslationTestCase{"SimpleIf", qasm::simpleIf, | ||
| MQT_NAMED_BUILDER(mlir::qc::simpleIf)}, | ||
| QASM3TranslationTestCase{"IfTwoQubits", qasm::ifTwoQubits, | ||
| MQT_NAMED_BUILDER(mlir::qc::ifTwoQubits)}, | ||
| QASM3TranslationTestCase{"IfElse", qasm::ifElse, | ||
| MQT_NAMED_BUILDER(mlir::qc::ifElse)})); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Inconsistent namespace qualification. These five cases use mlir::qc:: while every other case (lines 105-342) uses qc::. Since using namespace mlir; is in effect, qc:: resolves identically — align these for consistency.
♻️ Proposed normalization
- MQT_NAMED_BUILDER(mlir::qc::ctrlTwo)},
+ MQT_NAMED_BUILDER(qc::ctrlTwo)},
...
- MQT_NAMED_BUILDER(mlir::qc::ifElse)}));
+ MQT_NAMED_BUILDER(qc::ifElse)}));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| QASM3TranslationTestCase{"CtrlTwo", qasm::ctrlTwo, | |
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwo)}, | |
| QASM3TranslationTestCase{"CtrlTwoMixed", qasm::ctrlTwoMixed, | |
| MQT_NAMED_BUILDER(mlir::qc::ctrlTwoMixed)}, | |
| QASM3TranslationTestCase{"SimpleIf", qasm::simpleIf, | |
| MQT_NAMED_BUILDER(mlir::qc::simpleIf)}, | |
| QASM3TranslationTestCase{"IfTwoQubits", qasm::ifTwoQubits, | |
| MQT_NAMED_BUILDER(mlir::qc::ifTwoQubits)}, | |
| QASM3TranslationTestCase{"IfElse", qasm::ifElse, | |
| MQT_NAMED_BUILDER(mlir::qc::ifElse)})); | |
| QASM3TranslationTestCase{"CtrlTwo", qasm::ctrlTwo, | |
| MQT_NAMED_BUILDER(qc::ctrlTwo)}, | |
| QASM3TranslationTestCase{"CtrlTwoMixed", qasm::ctrlTwoMixed, | |
| MQT_NAMED_BUILDER(qc::ctrlTwoMixed)}, | |
| QASM3TranslationTestCase{"SimpleIf", qasm::simpleIf, | |
| MQT_NAMED_BUILDER(qc::simpleIf)}, | |
| QASM3TranslationTestCase{"IfTwoQubits", qasm::ifTwoQubits, | |
| MQT_NAMED_BUILDER(qc::ifTwoQubits)}, | |
| QASM3TranslationTestCase{"IfElse", qasm::ifElse, | |
| MQT_NAMED_BUILDER(qc::ifElse)})); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/unittests/Dialect/QC/Translation/test_qasm3_translation.cpp` around
lines 343 - 352, The five test entries (QASM3TranslationTestCase{"CtrlTwo",
"CtrlTwoMixed", "SimpleIf", "IfTwoQubits", "IfElse"}) use fully-qualified
mlir::qc:: but the rest of the file uses the unqualified qc:: thanks to using
namespace mlir;. Replace mlir::qc::ctrlTwo, mlir::qc::ctrlTwoMixed,
mlir::qc::simpleIf, mlir::qc::ifTwoQubits, and mlir::qc::ifElse with
qc::ctrlTwo, qc::ctrlTwoMixed, qc::simpleIf, qc::ifTwoQubits, and qc::ifElse
respectively so the QASM3TranslationTestCase entries and the MQT_NAMED_BUILDER
calls are consistent with the surrounding code.
Comment on lines
+9
to
+11
| add_library(MLIRQASMPrograms qasm_programs.cpp) | ||
| target_sources(MLIRQASMPrograms PUBLIC FILE_SET HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES | ||
| qasm_programs.h) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how ProjectOptions is defined/propagated and how sibling Programs libs pull it in.
fd -t f 'CMakeLists.txt' | xargs rg -n 'MQT::ProjectOptions|add_library\(MLIRProjectOptions|ProjectOptions' -C2
# Check whether the QC builder lib links ProjectOptions PUBLIC (transitive to MLIRQCPrograms)
ast-grep --pattern 'target_link_libraries($$$ProjectOptions$$$)'Repository: munich-quantum-toolkit/core
Length of output: 2712
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the full unittests programs CMakeLists around the referenced lines
nl -ba mlir/unittests/programs/CMakeLists.txt | sed -n '1,120p'
# Find where MLIRQASMPrograms is defined/linked
rg -n "MLIRQASMPrograms|mqt_mlir_apply_target_options|mqt_mlir_target_use_project_options|target_link_libraries\\(MLIR" mlir/unittests -S
# Show definitions of the helper in case unittests use it indirectly
nl -ba mlir/CMakeLists.txt | sed -n '1,120p'Repository: munich-quantum-toolkit/core
Length of output: 114
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Print the unittests programs CMakeLists with line numbers (avoid `nl`)
cat -n mlir/unittests/programs/CMakeLists.txt | sed -n '1,160p'
# 2) Locate how MLIRQASMPrograms is defined/linked and whether helper functions are used
rg -n "MLIRQASMPrograms|mqt_mlir_apply_target_options|mqt_mlir_target_use_project_options|target_link_libraries\\(" mlir/unittests/programs -S
# 3) Show the relevant helper functions in mlir/CMakeLists.txt
cat -n mlir/CMakeLists.txt | sed -n '1,120p'Repository: munich-quantum-toolkit/core
Length of output: 4099
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate where the builder targets are defined and check their link dependencies
rg -n "add_library\\(MLIRQCProgramBuilder\\b|add_library\\(MLIRQCOProgramBuilder\\b|add_library\\(MLIRIRProgramBuilder\\b|add_library\\(MLIR.*ProgramBuilder\\b" mlir -S
# Show the relevant CMakeLists sections around those target definitions
# (Using rg to capture nearby target_link_libraries/MQT::ProjectOptions)
for t in MLIRQCProgramBuilder MLIRQCOProgramBuilder MLIRIRProgramBuilder; do
echo "---- $t ----"
rg -n "target_link_libraries\\([^\\)]*\\b$t\\b|\\b$t\\b\\)" mlir -S --glob 'CMakeLists.txt'
rg -n "target_link_libraries\\(.*MQT::ProjectOptions|MQT::ProjectOptions" mlir -S --glob 'CMakeLists.txt' | head -n 200
doneRepository: munich-quantum-toolkit/core
Length of output: 53
🏁 Script executed:
#!/bin/bash
set -uo pipefail
# 1) Find the exact CMake file locations that define the program builder targets
rg -n --glob 'CMakeLists.txt' "add_library\\(\\s*(MLIRQCProgramBuilder|MLIRQCOProgramBuilder|MLIRIRProgramBuilder)\\b" mlir -S
# 2) For each known builder target name, print surrounding lines that include its definition + link libraries / project options usage
for t in MLIRQCProgramBuilder MLIRQCOProgramBuilder MLIRIRProgramBuilder; do
echo "==== Around add_library($t) ===="
rg -n --glob 'CMakeLists.txt' "add_library\\(\\s*${t}\\b" mlir -S -C 5 || true
echo "==== Search target_link_libraries(...$t...) / $t ... MQT::ProjectOptions ===="
rg -n --glob 'CMakeLists.txt' "target_link_libraries\\([^\\)]*\\b${t}\\b|${t}[^\\n]*MQT::ProjectOptions|mqt_mlir_target_use_project_options\\(\\s*${t}\\s*\\)" mlir -S || true
echo
done
# 3) Also check for helper calls on obj.<target> or generic project option apply for these builder names
rg -n --glob 'CMakeLists.txt' "mqt_mlir_(apply_target_options|target_use_project_options)\\(\\s*(MLIRQCProgramBuilder|MLIRQCOProgramBuilder|MLIRIRProgramBuilder)\\s*\\)" mlir -S || trueRepository: munich-quantum-toolkit/core
Length of output: 1370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant builder CMakeLists with line numbers around the helper call
echo "=== QC Builder CMakeLists ==="
cat -n mlir/lib/Dialect/QC/Builder/CMakeLists.txt | sed -n '1,120p'
echo "=== QCO Builder CMakeLists ==="
cat -n mlir/lib/Dialect/QCO/Builder/CMakeLists.txt | sed -n '1,140p'
# Locate IR builder definition/usage
echo "=== Find MLIRIRProgramBuilder occurrences ==="
rg -n "MLIRIRProgramBuilder" mlir -S || true
# If IR builder is defined in a CMakeLists, print that file around the definition + link statements
# (try common pattern: add_library(<name> ... ) )
rg -n --glob 'CMakeLists.txt' "add_library\\(\\s*MLIRIRProgramBuilder\\b" mlir -S || trueRepository: munich-quantum-toolkit/core
Length of output: 2156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "MLIRIRProgramBuilder" -S . || true
rg -n "IRProgramBuilder\\.cpp|QIRProgramBuilder\\.cpp|qir_programs|qir.*ProgramBuilder" -S mlir || true
# Also look for add_mlir_library / add_library definitions for IR program builder candidates
rg -n --glob 'CMakeLists.txt' "add_mlir_library\\(\\s*([^\\s\\)]+ProgramBuilder)\\b" mlir -S || trueRepository: munich-quantum-toolkit/core
Length of output: 1140
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== QIR Builder CMakeLists ==="
cat -n mlir/lib/Dialect/QIR/Builder/CMakeLists.txt | sed -n '1,120p' || true
echo
echo "=== Occurrences of MLIRQASMPrograms in CMake ==="
rg -n "MLIRQASMPrograms" -S . --glob 'CMakeLists.txt' || true
echo
echo "=== Any helper applying project options to MLIRQASMPrograms ==="
rg -n "mqt_mlir_(apply_target_options|target_use_project_options)\\(\\s*MLIRQASMPrograms\\s*\\)|MQT::ProjectOptions.*MLIRQASMPrograms|MLIRQASMPrograms.*MQT::ProjectOptions" -S . --glob 'CMakeLists.txt' || true
echo
echo "=== Occurrences of MLIRQIRProgramBuilder project options usage ==="
rg -n "MLIRQIRProgramBuilder" -S mlir/lib --glob 'CMakeLists.txt' || trueRepository: munich-quantum-toolkit/core
Length of output: 1915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for applying project options directly to the unittests program targets
rg -n --glob 'CMakeLists.txt' "mqt_mlir_target_use_project_options\\(\\s*(MLIRQASMPrograms|MLIRQCPrograms|MLIRQCOPrograms|MLIRQIRPrograms|MLIRQuantumComputationPrograms)\\s*\\)" mlir -S || true
# Search for apply_target_options usage on these
rg -n --glob 'CMakeLists.txt' "mqt_mlir_apply_target_options\\(\\s*(MLIRQASMPrograms|MLIRQCPrograms|MLIRQCOPrograms|MLIRQIRPrograms)\\s*\\)" mlir -S || true
# Search for any direct target_link_libraries(... MQT::ProjectOptions ...) on those unittests program targets
rg -n --glob 'CMakeLists.txt' "target_link_libraries\\([^\\)]*(MLIRQASMPrograms|MLIRQCPrograms|MLIRQCOPrograms|MLIRQIRPrograms)[^\\)]*MQT::ProjectOptions" mlir -S || trueRepository: munich-quantum-toolkit/core
Length of output: 53
MLIRQASMPrograms doesn’t get MQT::ProjectOptions (and isn’t consistent with other MLIR*Programs) — Only MLIRQuantumComputationPrograms links MQT::ProjectOptions; the *ProgramBuilder targets apply MQT::ProjectOptions with mqt_mlir_target_use_project_options(...), which links it PRIVATE to the builder target, so those compile/warning options don’t propagate to MLIRQASMPrograms.
🔧 Proposed addition
add_library(MLIRQASMPrograms qasm_programs.cpp)
+target_link_libraries(MLIRQASMPrograms PRIVATE MQT::ProjectOptions)
target_sources(MLIRQASMPrograms PUBLIC FILE_SET HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES
qasm_programs.h)Also consider doing the same for MLIRQCPrograms, MLIRQCOPrograms, and MLIRQIRPrograms for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| add_library(MLIRQASMPrograms qasm_programs.cpp) | |
| target_sources(MLIRQASMPrograms PUBLIC FILE_SET HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES | |
| qasm_programs.h) | |
| add_library(MLIRQASMPrograms qasm_programs.cpp) | |
| target_link_libraries(MLIRQASMPrograms PRIVATE MQT::ProjectOptions) | |
| target_sources(MLIRQASMPrograms PUBLIC FILE_SET HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES | |
| qasm_programs.h) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/unittests/programs/CMakeLists.txt` around lines 9 - 11, MLIRQASMPrograms
is missing the project compile/warning options because it doesn't use
MQT::ProjectOptions; call mqt_mlir_target_use_project_options(MLIRQASMPrograms)
(which applies MQT::ProjectOptions privately to the target) or otherwise link
MQT::ProjectOptions PRIVATE to MLIRQASMPrograms similar to
MLIRQuantumComputationPrograms so the options propagate at build time; apply the
same change for the other program targets MLIRQCPrograms, MLIRQCOPrograms, and
MLIRQIRPrograms for consistency.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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 a direct OpenQASM-to-QC translation that bypasses the
qc::QuantumComputation.The new flow reuses the existing parser and walks the AST directly to emit QC dialect ops. Skipping
QuantumComputationremoves an intermediate step and enables previously unsupported QASM3 features, such as gate modifiers, classical computations, and control flow structures.What works
ctrl @,negctrl @,inv @, nested combinationsif/elseover quantum statementsqc.staticCurrent limitations
powmodifier is unsupported (but will/should be once ✨ Add power modifier to MLIR #1603 is merged)QC)Checklist
I have updated the documentation to reflect these changes.I have added migration instructions to the upgrade guide (if needed).