From b8f3d81f6c742fd3faccc5df5a1bb91b036c9b15 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Wed, 19 Aug 2026 17:05:49 +0200 Subject: [PATCH 01/11] =?UTF-8?q?=E2=9C=A8=20Import=20captured=20Qiskit=20?= =?UTF-8?q?expressions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Simon Hofmann --- .../qiskit-classical-expression-captures.md | 304 +++++++++++++ bindings/mlir/qiskit/Qiskit2_5.cpp | 423 ++++++++++++++++-- bindings/mlir/qiskit/QiskitImport.cpp | 92 +++- bindings/mlir/qiskit/QiskitTranslation.h | 4 + docs/mlir/python_compiler_collection.md | 8 +- test/python/test_mlir_qiskit_translation.py | 163 +++++++ 6 files changed, 945 insertions(+), 49 deletions(-) create mode 100644 .agent/plans/qiskit-classical-expression-captures.md diff --git a/.agent/plans/qiskit-classical-expression-captures.md b/.agent/plans/qiskit-classical-expression-captures.md new file mode 100644 index 0000000000..5d7a088a2e --- /dev/null +++ b/.agent/plans/qiskit-classical-expression-captures.md @@ -0,0 +1,304 @@ +# Import Qiskit classical-expression captures + +This ExecPlan is a living document. The sections `Progress`, +`Surprises & Discoveries`, `Decision Log`, and `Outcomes & Retrospective` must +be kept up to date as work proceeds. + +This ExecPlan must be maintained in accordance with `.agent/PLANS.md` from the +repository root. + +## Purpose / Big Picture + +Qiskit 2.5 control-flow expressions can read a `Clbit` or `ClassicalRegister` +from the containing circuit. These values can be block captures, but a condition +or switch target can also be their only use. The current importer handles +literal expression trees but rejects these variable leaves. After this change, +`QCProgram.from_qiskit` can import Boolean and unsigned-integer conditions that +read classical bits and registers, including nested control flow and +expression-valued switch targets. The imported MLIR reads the existing +first-class CBit registers, so each expression refers to the same classical +state as the source circuit. + +This plan covers import only. It does not add Qiskit writer APIs or construct +Qiskit control-flow operations during export. + +## Progress + +- [x] (2026-08-19 14:46Z) Read the repository instructions and compare the + current scalar/CBit branch with the earlier full control-flow + implementation. +- [x] (2026-08-19 14:52Z) Extend the normalized expression model with captured + bit and register leaves without changing the scalar `Parameter` model. +- [x] (2026-08-19 14:53Z) Normalize Qiskit expression variables through public + Python bit identity and Qiskit's native local-to-root Clbit maps. +- [x] (2026-08-19 14:54Z) Materialize and validate captured leaves through the + existing CBit load and register-packing helpers. +- [x] (2026-08-19 14:56Z) Add focused bit, register, nested-capture, malformed + capture, and switch-expression import tests. +- [x] (2026-08-19 15:03Z) Build, run the full Qiskit translation test file and + repository lint session, inspect the final diff, and prepare the completed + import slice for a local commit. +- [x] (2026-08-19 15:10Z) Reproduce the valid explicit-body case in which a + condition reads a root Clbit absent from all block operands. +- [x] (2026-08-19 15:19Z) Retain the Python circuit hierarchy, add a + containing-circuit resolver with parent-map composition, add focused if, + switch, and nested-map regressions, rebuild, and pass all eight focused + capture tests. +- [x] (2026-08-19 15:23Z) Pass all 165 Qiskit translation tests and the complete + repository lint session, inspect the final diff, and prepare the existing + local import commit for amendment. +- [x] (2026-08-19 16:12Z) Reproduce a nested legacy tuple condition that reads + root Clbit one as local index zero, route it through the public Python bit + resolver, add the exact `for`-then-`if` regression, rebuild, and pass all + nine focused capture and condition tests. +- [x] (2026-08-19 16:14Z) Pass all 166 Qiskit translation tests, rerun the + complete repository lint session, inspect the final diff, and prepare the + existing local import commit for amendment. +- [x] (2026-08-19 19:45Z) Rebase the focused import commit onto the scalar + commit after first-class CBit support merged, rebuild the release MLIR + binding, and pass all 166 Qiskit translation tests again. +- [x] (2026-08-19 20:08Z) Restack onto the audited scalar parent, update the + recorded parent identity, rebuild the release binding, and pass all 167 + Qiskit translation tests. + +## Surprises & Discoveries + +- Observation: The current branch already contains structured-control import, + CBit register storage, and the scalar symbolic `Parameter` tree. The older + full implementation therefore cannot be cherry-picked safely. Evidence: + `QiskitImport.cpp` already emits `scf.if`, `scf.while`, and + `scf.index_switch`, while the parent scalar-symbol commit adds the independent + parameter work. + +- Observation: Qiskit 2.5 native switch-target accessors are not safe for an + expression-valued target. The public Python `SwitchCaseOp.target` expression + tree must be used for that case. Evidence: the earlier implementation records + that the native C accessors abort when the target is an expression. + +- Observation: A full test run must inject the worktree-built extension into + child Python processes as well as the pytest process. Evidence: one existing + isolation test launches `sys.executable`; after using a temporary + `sitecustomize.py`, all 162 tests exercised the local binding and passed. The + temporary harness was removed after validation. + +- Observation: `CircuitInstruction.clbits` contains the bits passed to the + control-flow blocks, not every bit read by the condition or switch target. An + explicit body can have zero classical operands while its expression reads a + Clbit from the containing circuit. Evidence: an explicit `if_test` with an + empty `clbits` argument is valid Qiskit, but the initial resolver rejected it + because both the instruction and its block had zero Clbits. + +- Observation: A nested expression bit must first be resolved in its containing + Python circuit. A lookup in the root Python circuit can confuse equal Clbit + objects from similar local registers. Evidence: the nested regression maps + local Clbit zero to root Clbit one and observes a load from root index one. + +- Observation: Qiskit's native legacy Clbit-condition accessor returns an index + in the containing nested circuit. Using that number as a root index reads the + wrong CBit register element. Evidence: a tuple condition on root Clbit one + inside a context-managed `for` loop initially emitted `cbit.load` at index + zero; resolving the Python condition bit through the enclosing map emits index + one. + +## Decision Log + +- Decision: Add `ClassicalBit` and `ClassicalRegister` to `ExpressionKind`, with + a global bit index or a normalized register payload on `Expression`. + Rationale: The normalized tree then owns stable capture identity and stays + independent of Python object lifetimes. Date/Author: 2026-08-19 / Codex. + +- Decision: Keep `ParameterKind`, `Parameter`, and `Loop::parameter` unchanged. + Rationale: Scalar symbols and classical captures have different identity and + typing rules. This branch must remain composable with the reviewed scalar + slice. Date/Author: 2026-08-19 / Codex. + +- Decision: Retain the full Python `CircuitInstruction`, the containing Python + circuit, and the root Python circuit in `NativeControlFlowReader`. Resolve a + classical bit in the containing circuit and compose its local index with the + enclosing native capture map when the circuit is nested. Use the current + native block map only to validate the instruction structure. Apply this rule + to expression leaves, switch targets, and legacy tuple conditions. Rationale: + `CircuitInstruction.clbits` describes block operands only, native condition + indices can remain local, and direct root lookup is ambiguous for nested local + registers. Date/Author: 2026-08-19 / Codex. + +- Decision: Parse expression-valued switch targets from the public Python + expression tree. Continue to use native metadata for cases and block maps. + Rationale: This avoids the unsafe Qiskit 2.5 native accessor while keeping the + established native control-flow reader for supported metadata. Date/Author: + 2026-08-19 / Codex. + +- Decision: Document circuit Clbit and ClassicalRegister expression variables + separately from standalone runtime variables. Rationale: circuit-owned bits + resolve to existing CBit state whether or not a block captures them; the + importer still rejects Qiskit runtime inputs, and export remains outside this + slice. Date/Author: 2026-08-19 / Codex. + +## Outcomes & Retrospective + +The import slice now preserves Clbit and ClassicalRegister identity through the +containing Python circuit and Qiskit's native root maps. It lowers variable +leaves through the existing CBit load and little-endian register pack paths, +preflights malformed captures, and reads expression-valued switch targets only +through the public Python expression tree. Conditions and switch targets also +work when their classical bits are absent from every block operand. The public +support table distinguishes these supported circuit values from rejected +standalone runtime inputs. + +The release MLIR binding built successfully. The complete Qiskit translation +test file passed with 167 tests against that local extension, including the +subprocess isolation test, the condition-only regressions, and the nested legacy +Clbit condition. `uvx nox -s lint`, `git diff --check`, Clang format, Ruff, +Rumdl, Prettier, and `ty` all passed. Export-side writer construction remains +deliberately out of scope. + +## Context and Orientation + +`bindings/mlir/qiskit/QiskitTranslation.h` contains version-neutral normalized +data passed between the Qiskit version adapter and the MLIR importer. +`bindings/mlir/qiskit/Qiskit2_5.cpp` reads Qiskit 2.5 through its native C API +and selected public Python objects. `NativeControlFlowReader` supplies one +normalized `ClassicalTarget` for an if, while, or switch operation. +`bindings/mlir/qiskit/QiskitImport.cpp` lowers that target to MLIR. It already +stores classical state in `!cbit.reg` values and provides `loadClassicalBit` +and `packRegister` helpers. + +A block capture is a Clbit used by a control-flow block that comes from its +enclosing circuit. Qiskit exposes Python objects in `CircuitInstruction.clbits` +in block-capture order. Its native control-flow object exposes a map from that +local order to root-circuit Clbit indices. A condition or switch target can also +read a bit that no block uses. The importer therefore retains the containing +Python circuit to find the local bit and uses the enclosing native map to reach +its root index when the circuit is nested. The retained root Python circuit owns +the complete object hierarchy while the reader traverses nested blocks. + +The current scalar `Parameter` tree represents numeric gate and loop +expressions. It is unrelated to Qiskit's typed classical-expression tree and +must not be refactored in this task. + +## Plan of Work + +First, extend `ExpressionKind` and `Expression` in +`bindings/mlir/qiskit/QiskitTranslation.h` with bit and register leaves. Keep +all scalar parameter declarations byte-for-byte unchanged. + +Next, update `bindings/mlir/qiskit/Qiskit2_5.cpp`. Make the native expression +normalizer walk the matching public Python expression node beside each native +node. Resolve a `Var` leaf by inspecting its public `var` object. For a Clbit, +find the bit in the containing Python circuit and compose the local index +through the enclosing native capture map. Use the same resolver for a legacy +tuple condition's Clbit instead of trusting its native local index. For a +classical register, apply the same mapping to each member in register order. +Reject malformed captures, duplicate or invalid types, standalone variables, and +widths outside the existing 64-bit limit. Keep a Python-only expression walker +for switch targets so no unsafe native switch-expression accessor is called. + +Then update `bindings/mlir/qiskit/QiskitImport.cpp`. Pass callbacks into the +recursive expression emitter. A bit leaf calls `loadClassicalBit`; a register +leaf calls `packRegister` and extends it to the normalized expression width. +Extend preflight validation to check leaf types, bit bounds, register size, +unique register bits, and expression widths before MLIR construction begins. + +Finally, add tests to `test/python/test_mlir_qiskit_translation.py`. Cover one +captured Clbit expression, one captured register expression, nested control flow +whose inner expression uses outer captures, and an expression-valued switch +target. Also cover explicit if and switch bodies whose expression bits are +absent from every block operand, plus a nested permutation that proves +parent-map composition. Verify the expected CBit loads and +arithmetic/control-flow ops, and re-import the produced program or source +circuit where export is outside this slice. + +## Concrete Steps + +Run all commands from the repository root. + +Inspect the focused diff and formatting: + + git diff --check + clang-format --dry-run --Werror bindings/mlir/qiskit/Qiskit2_5.cpp \ + bindings/mlir/qiskit/QiskitImport.cpp \ + bindings/mlir/qiskit/QiskitTranslation.h + uvx ruff check test/python/test_mlir_qiskit_translation.py + +Build the Qiskit binding with the configured release tree. If the isolated +worktree has no compatible build tree yet, configure it with the repository's +release preset first: + + cmake --build build/release --parallel 8 + +Run the focused tests: + + uv run --no-sync pytest test/python/test_mlir_qiskit_translation.py \ + -k 'classical_expression or condition_only or switch_expression' + +Run the complete Qiskit translation test file after the focused tests pass: + + uv run --no-sync pytest test/python/test_mlir_qiskit_translation.py + +Run the repository lint session before handoff: + + uvx nox -s lint + +## Validation and Acceptance + +Acceptance requires that a Qiskit if or while condition containing +`expr.lift(circuit.clbits[i])` imports to an MLIR `cbit.load` from the matching +register element. A register expression must load and pack its members in +Qiskit's little-endian register order. An inner control-flow instruction must +resolve its own `CircuitInstruction.clbits` capture order and reach the same +root CBit elements. An expression-valued Qiskit switch must import without +calling a native switch-expression target accessor and must produce +`scf.index_switch`. Explicit if and switch bodies with empty classical operand +lists must still read condition-only and target-only bits from the containing +circuit. A nested condition-only bit must follow the enclosing block's +local-to-root permutation. A nested legacy tuple condition on root Clbit one +must emit a `cbit.load` at index one even when that bit is local index zero in +the enclosing block. + +Malformed block-capture lists and variables absent from the containing circuit +must fail during validation with a clear runtime error. Existing literal +expression, structured-control, CBit, and symbolic parameter tests must continue +to pass. The final tree must have no exporter or writer control-flow +construction changes. + +## Idempotence and Recovery + +All build, format-check, and test commands are repeatable. Source changes are +limited to the version-neutral normalized model, the Qiskit 2.5 reader, the MLIR +importer, one Python test file, and this plan. Do not reset or overwrite +unrelated work. If a test exposes a Qiskit API difference, inspect the installed +2.5 objects from the test environment and adjust only the version-specific +reader. Do not add a private exporter fallback. + +## Artifacts and Notes + +The source branch begins at the focused scalar-symbol parent, which already +includes CBit and symbolic scalar support. Native expression nodes do not carry +sufficient public Clbit identity by themselves. `CircuitInstruction.clbits` +supplies identity for block operands, while the containing Python circuit +supplies identity for bits used only by a condition or switch target. + +## Interfaces and Dependencies + +At completion, `ExpressionKind` in `bindings/mlir/qiskit/QiskitTranslation.h` +has `ClassicalBit` and `ClassicalRegister` cases. `Expression` has +`uint32_t bit` and `Register reg` payloads. `NativeControlFlowReader` in +`Qiskit2_5.cpp` owns the full Python instruction, its operation, its containing +circuit, and the root Python circuit. Its expression normalization resolves all +classical leaves and legacy Clbit conditions to root-circuit indices through the +containing-circuit and parent-map path. `QiskitImport.cpp` accepts expression +leaves only through callbacks backed by `loadClassicalBit` and `packRegister`. + +This work depends only on Qiskit 2.5's existing native extension table, +nanobind's public Python object access, MLIR's arithmetic and structured-control +dialects, and MQT Core's CBit builder methods. It introduces no new dependency. + +Revision note: Created the initial self-contained plan after comparing the +current scalar/CBit branch with the earlier combined implementation. Updated it +after implementation and final validation to record the public documentation +decision, subprocess-aware test setup, and successful results. Updated it again +after the final audit found valid condition-only and target-only bits outside +the block-capture list; the plan now records the containing-circuit resolver and +nested parent-map regression. Updated it once more after the nested legacy +Clbit-condition accessor exposed its containing-circuit index rather than a root +index. diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index c5c32c434a..59bedc994f 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -663,8 +663,10 @@ normalizeBinaryOperation(const QkBinaryOpType op) { "Qiskit returned an unknown unary expression operation"); } -[[nodiscard]] std::unique_ptr -normalizeExpression(const QkExprNode* expression, const size_t depth = 0U) { +template +[[nodiscard]] std::unique_ptr normalizeExpression( + const QkExprNode* expression, const nb::handle pythonExpression, + NormalizeVariable& normalizeVariable, const size_t depth = 0U) { if (expression == nullptr) { throw std::runtime_error("Qiskit returned a null classical expression"); } @@ -679,8 +681,16 @@ normalizeExpression(const QkExprNode* expression, const size_t depth = 0U) { result->kind = ExpressionKind::Binary; result->binaryOperation = normalizeBinaryOperation(info.op); setType(*result, info.ty); - result->left = normalizeExpression(info.left, depth + 1U); - result->right = normalizeExpression(info.right, depth + 1U); + result->left = normalizeExpression( + info.left, + pythonAttribute(pythonExpression, "left", + "Qiskit binary expression has no left operand"), + normalizeVariable, depth + 1U); + result->right = normalizeExpression( + info.right, + pythonAttribute(pythonExpression, "right", + "Qiskit binary expression has no right operand"), + normalizeVariable, depth + 1U); return result; } case QkExprNodeKind_Unary: { @@ -688,22 +698,38 @@ normalizeExpression(const QkExprNode* expression, const size_t depth = 0U) { result->kind = ExpressionKind::Unary; result->unaryOperation = normalizeUnaryOperation(info.op); setType(*result, info.ty); - result->left = normalizeExpression(info.operand, depth + 1U); + result->left = normalizeExpression( + info.operand, + pythonAttribute(pythonExpression, "operand", + "Qiskit unary expression has no operand"), + normalizeVariable, depth + 1U); return result; } case QkExprNodeKind_Cast: { const auto info = qk_expr_cast_info(expression); result->kind = ExpressionKind::Cast; setType(*result, info.ty); - result->left = normalizeExpression(info.operand, depth + 1U); + result->left = normalizeExpression( + info.operand, + pythonAttribute(pythonExpression, "operand", + "Qiskit cast expression has no operand"), + normalizeVariable, depth + 1U); return result; } case QkExprNodeKind_Index: { const auto info = qk_expr_index_info(expression); result->kind = ExpressionKind::Index; setType(*result, info.ty); - result->left = normalizeExpression(info.target, depth + 1U); - result->right = normalizeExpression(info.index, depth + 1U); + result->left = normalizeExpression( + info.target, + pythonAttribute(pythonExpression, "target", + "Qiskit index expression has no target"), + normalizeVariable, depth + 1U); + result->right = normalizeExpression( + info.index, + pythonAttribute(pythonExpression, "index", + "Qiskit index expression has no index"), + normalizeVariable, depth + 1U); return result; } case QkExprNodeKind_Value: { @@ -729,9 +755,9 @@ normalizeExpression(const QkExprNode* expression, const size_t depth = 0U) { return result; } case QkExprNodeKind_Var: - throw std::runtime_error( - "Qiskit circuit import does not support variables in classical " - "expressions"); + setType(*result, qk_var_type_info(qk_expr_as_var(expression))); + normalizeVariable(*result, pythonExpression); + return result; case QkExprNodeKind_Stretch: throw std::runtime_error( "Qiskit circuit import does not support stretch expressions"); @@ -901,6 +927,7 @@ class NativeCircuitReader final : public CircuitReader { data_(pythonAttribute( circuit, "_data", "expected a Qiskit QuantumCircuit with native CircuitData")), + rootPythonCircuit_(pythonCircuit_), circuit_(qk_circuit_borrow_from_python(data_.ptr())) { if (circuit_ == nullptr) { throwPythonError("Qiskit rejected QuantumCircuit._data"); @@ -910,12 +937,14 @@ class NativeCircuitReader final : public CircuitReader { NativeCircuitReader(nb::object pythonCircuit, const QkCircuit* circuit, const QkCircuit* rootCircuit, + nb::object rootPythonCircuit, const QkControlFlowInstruction* parent) : pythonCircuit_(std::move(pythonCircuit)), data_(pythonAttribute( pythonCircuit_, "_data", "Qiskit control-flow block has no native CircuitData")), - circuit_(circuit), rootCircuit_(rootCircuit), parent_(parent) {} + rootPythonCircuit_(std::move(rootPythonCircuit)), circuit_(circuit), + rootCircuit_(rootCircuit), parent_(parent) {} [[nodiscard]] uint32_t numQubits() const override { return qk_circuit_num_qubits(circuit_); @@ -1208,6 +1237,7 @@ class NativeCircuitReader final : public CircuitReader { nb::object pythonCircuit_; nb::object data_; + nb::object rootPythonCircuit_; const QkCircuit* circuit_ = nullptr; const QkCircuit* rootCircuit_ = circuit_; const QkControlFlowInstruction* parent_ = nullptr; @@ -1218,11 +1248,18 @@ class NativeControlFlowReader final : public ControlFlowReader { NativeControlFlowReader(const QkCircuit* rootCircuit, const QkCircuit* circuit, const size_t index, const QkControlFlowInstruction* parent, - nb::object operation) - : rootCircuit_(rootCircuit), + nb::object instruction, + nb::object containingPythonCircuit, + nb::object rootPythonCircuit) + : rootCircuit_(rootCircuit), circuit_(circuit), parent_(parent), controlFlow_( qk_circuit_get_control_flow_instruction(circuit, index, parent)), - operation_(std::move(operation)) { + instruction_(std::move(instruction)), + operation_(pythonAttribute( + instruction_, "operation", + "Qiskit circuit instruction has no control-flow operation")), + containingPythonCircuit_(std::move(containingPythonCircuit)), + rootPythonCircuit_(std::move(rootPythonCircuit)) { if (controlFlow_ == nullptr) { throwPythonError("Qiskit failed to inspect a control-flow instruction"); } @@ -1267,7 +1304,7 @@ class NativeControlFlowReader final : public ControlFlowReader { const auto block = nb::borrow(blocks[index]); return std::make_unique( block, qk_control_flow_block_circuit(controlFlow_, index), rootCircuit_, - controlFlow_); + rootPythonCircuit_, controlFlow_); } [[nodiscard]] std::vector qubitMap() const override { @@ -1303,8 +1340,14 @@ class NativeControlFlowReader final : public ControlFlowReader { switch (qk_control_flow_condition_type(controlFlow_)) { case QkConditionType_ClBit: { const auto bit = qk_control_flow_condition_bit_info(controlFlow_); + const auto condition = pythonAttribute( + operation_, "condition", "Qiskit control flow has no condition"); + if (nb::len(condition) != 2U) { + throw std::runtime_error( + "Qiskit classical-bit condition has an invalid shape"); + } result.kind = ClassicalTargetKind::ClassicalBit; - result.bit = static_cast(bit.clbit); + result.bit = rootClbitIndex(condition[0]); result.expectedBit = bit.condition; return result; } @@ -1330,8 +1373,10 @@ class NativeControlFlowReader final : public ControlFlowReader { } case QkConditionType_Expr: result.kind = ClassicalTargetKind::Expression; - result.expression = - normalizeExpression(qk_control_flow_condition_expr(controlFlow_)); + result.expression = normalizePythonExpression( + qk_control_flow_condition_expr(controlFlow_), + pythonAttribute(operation_, "condition", + "Qiskit control flow has no condition")); return result; } throw std::runtime_error("Qiskit returned an unknown condition type"); @@ -1405,28 +1450,38 @@ class NativeControlFlowReader final : public ControlFlowReader { [[nodiscard]] ClassicalTarget switchTarget() const override { ClassicalTarget result; - switch (qk_control_flow_switch_target_type(controlFlow_)) { - case QkConditionType_ClBit: + const auto target = + pythonAttribute(operation_, "target", "Qiskit switch has no target"); + const auto circuitModule = nb::module_::import_("qiskit.circuit"); + if (nb::isinstance(target, circuitModule.attr("Clbit"))) { result.kind = ClassicalTargetKind::ClassicalBit; - result.bit = qk_control_flow_switch_target_bit(controlFlow_); + result.bit = rootClbitIndex(target); return result; - case QkConditionType_ClReg: + } + if (nb::isinstance(target, circuitModule.attr("ClassicalRegister"))) { result.kind = ClassicalTargetKind::ClassicalRegister; - result.reg = normalizeRegister( - qk_control_flow_switch_target_register(controlFlow_), rootCircuit_); - if (result.reg.bits.empty() || result.reg.bits.size() > 64U) { + result.reg.name = pythonStringAttribute( + target, "name", "Qiskit switch register has no name"); + if (nb::len(target) == 0U || nb::len(target) > 64U) { throw std::runtime_error( "Qiskit switch registers must contain between 1 and 64 bits"); } + result.reg.bits.reserve(nb::len(target)); + for (const nb::handle bit : nb::iter(target)) { + result.reg.bits.push_back(rootClbitIndex(bit)); + } result.width = static_cast(result.reg.bits.size()); return result; - case QkConditionType_Expr: + } + const auto expressionModule = + nb::module_::import_("qiskit.circuit.classical.expr"); + if (nb::isinstance(target, expressionModule.attr("Expr"))) { result.kind = ClassicalTargetKind::Expression; - result.expression = - normalizeExpression(qk_control_flow_switch_target_expr(controlFlow_)); + // Qiskit 2.5's native switch-target accessors abort for expressions. + result.expression = normalizePythonExpressionOnly(target); return result; } - throw std::runtime_error("Qiskit returned an unknown switch-target type"); + throw std::runtime_error("Qiskit switch has an unknown target type"); } [[nodiscard]] std::vector switchCases() const override { @@ -1454,15 +1509,321 @@ class NativeControlFlowReader final : public ControlFlowReader { } private: + [[nodiscard]] uint32_t rootClbitIndex(const nb::handle bit) const { + const auto clbits = pythonAttribute( + instruction_, "clbits", + "Qiskit control-flow instruction has no classical-bit operands"); + if (numBlocks() == 0U || + nb::len(clbits) != qk_circuit_num_clbits(qk_control_flow_block_circuit( + controlFlow_, 0U))) { + throw std::runtime_error( + "Qiskit control flow has incompatible classical-bit captures"); + } + const auto* const map = qk_control_flow_clbit_map(controlFlow_); + if (map == nullptr && nb::len(clbits) != 0U) { + throw std::runtime_error( + "Qiskit control flow has no classical-bit capture map"); + } + // Conditions and switch targets refer to bits in the containing circuit. + // The current block-operand map is not an identity source: a bit can be + // absent from all blocks, and a nested map can still use a local index. + // Resolve the Python bit in the containing circuit, then use the enclosing + // control flow's native map when that circuit is itself a nested block. + try { + const auto findBit = pythonAttribute( + containingPythonCircuit_, "find_bit", + "Qiskit containing circuit cannot resolve expression variables"); + const auto location = findBit(bit); + const auto localIndex = pythonUnsignedAttribute( + location, "index", + "Qiskit expression variable has an invalid circuit index"); + if (localIndex >= qk_circuit_num_clbits(circuit_)) { + throw std::runtime_error( + "Qiskit expression variable has an invalid circuit index"); + } + if (parent_ == nullptr) { + return static_cast(localIndex); + } + + const auto* const parentMap = qk_control_flow_clbit_map(parent_); + if (parentMap == nullptr) { + throw std::runtime_error( + "Qiskit enclosing control flow has no classical-bit capture map"); + } + return parentMap[localIndex]; + } catch (const nb::python_error& error) { + throwPythonError( + "Qiskit expression variable is absent from its containing circuit", + error); + } + } + + static void setPythonExpressionType(Expression& result, + const nb::handle pythonExpression) { + const auto type = pythonAttribute(pythonExpression, "type", + "Qiskit expression has no type"); + const auto typeName = pythonStringAttribute( + pythonAttribute(type, "__class__", + "Qiskit expression type has no Python class"), + "__name__", "Qiskit expression type has no class name"); + if (typeName == "Bool") { + result.type = ClassicalType::Bool; + result.width = 1U; + return; + } + if (typeName == "Uint") { + const auto width = pythonUnsignedAttribute( + type, "width", "Qiskit Uint expression has no width"); + if (width == 0U || width > 64U) { + throw std::runtime_error( + "Qiskit unsigned classical values must be between 1 and 64 bits"); + } + result.type = ClassicalType::Uint; + result.width = static_cast(width); + return; + } + if (typeName == "Float") { + result.type = ClassicalType::Float; + result.width = 64U; + return; + } + if (typeName == "Duration") { + throw std::runtime_error( + "Qiskit circuit import does not support duration expressions"); + } + throw std::runtime_error("Qiskit expression has an unknown Python type"); + } + + [[nodiscard]] static BinaryOperation + pythonBinaryOperation(const std::string_view name) { + if (name == "BIT_AND") { + return BinaryOperation::BitAnd; + } + if (name == "BIT_OR") { + return BinaryOperation::BitOr; + } + if (name == "BIT_XOR") { + return BinaryOperation::BitXor; + } + if (name == "LOGIC_AND") { + return BinaryOperation::LogicAnd; + } + if (name == "LOGIC_OR") { + return BinaryOperation::LogicOr; + } + if (name == "EQUAL") { + return BinaryOperation::Equal; + } + if (name == "NOT_EQUAL") { + return BinaryOperation::NotEqual; + } + if (name == "LESS") { + return BinaryOperation::Less; + } + if (name == "LESS_EQUAL") { + return BinaryOperation::LessEqual; + } + if (name == "GREATER") { + return BinaryOperation::Greater; + } + if (name == "GREATER_EQUAL") { + return BinaryOperation::GreaterEqual; + } + if (name == "SHIFT_LEFT") { + return BinaryOperation::ShiftLeft; + } + if (name == "SHIFT_RIGHT") { + return BinaryOperation::ShiftRight; + } + if (name == "ADD") { + return BinaryOperation::Add; + } + if (name == "SUB") { + return BinaryOperation::Subtract; + } + if (name == "MUL") { + return BinaryOperation::Multiply; + } + if (name == "DIV") { + return BinaryOperation::Divide; + } + throw std::runtime_error( + "Qiskit expression has an unknown Python binary operation"); + } + + [[nodiscard]] static UnaryOperation + pythonUnaryOperation(const std::string_view name) { + if (name == "BIT_NOT") { + return UnaryOperation::BitNot; + } + if (name == "LOGIC_NOT") { + return UnaryOperation::LogicNot; + } + if (name == "NEGATE") { + return UnaryOperation::Negate; + } + throw std::runtime_error( + "Qiskit expression has an unknown Python unary operation"); + } + + [[nodiscard]] std::unique_ptr + normalizePythonExpressionOnly(const nb::handle pythonExpression, + const size_t depth = 0U) const { + if (depth >= MAX_EXPRESSION_DEPTH) { + throw std::runtime_error( + "Qiskit classical expressions exceed the nesting limit of 64"); + } + auto result = std::make_unique(); + setPythonExpressionType(*result, pythonExpression); + const auto className = pythonStringAttribute( + pythonAttribute(pythonExpression, "__class__", + "Qiskit expression has no Python class"), + "__name__", "Qiskit expression has no class name"); + if (className == "Var") { + normalizePythonVariable(*result, pythonExpression); + return result; + } + if (className == "Value") { + result->kind = ExpressionKind::Value; + const auto value = pythonAttribute( + pythonExpression, "value", "Qiskit literal expression has no value"); + switch (result->type) { + case ClassicalType::Bool: + if (!nb::try_cast(value, result->boolValue)) { + throw std::runtime_error( + "Qiskit Boolean expression has an invalid value"); + } + break; + case ClassicalType::Uint: + if (!nb::try_cast(value, result->uintValue)) { + throw std::runtime_error( + "Qiskit Uint expression has an invalid value"); + } + break; + case ClassicalType::Float: + if (!nb::try_cast(value, result->floatValue) || + !std::isfinite(result->floatValue)) { + throw std::runtime_error( + "Qiskit Float expression has an invalid value"); + } + break; + } + return result; + } + if (className == "Unary") { + result->kind = ExpressionKind::Unary; + result->unaryOperation = pythonUnaryOperation(pythonStringAttribute( + pythonAttribute(pythonExpression, "op", + "Qiskit unary expression has no operation"), + "name", "Qiskit unary expression operation has no name")); + result->left = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "operand", + "Qiskit unary expression has no operand"), + depth + 1U); + return result; + } + if (className == "Binary") { + result->kind = ExpressionKind::Binary; + result->binaryOperation = pythonBinaryOperation(pythonStringAttribute( + pythonAttribute(pythonExpression, "op", + "Qiskit binary expression has no operation"), + "name", "Qiskit binary expression operation has no name")); + result->left = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "left", + "Qiskit binary expression has no left operand"), + depth + 1U); + result->right = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "right", + "Qiskit binary expression has no right operand"), + depth + 1U); + return result; + } + if (className == "Cast") { + result->kind = ExpressionKind::Cast; + result->left = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "operand", + "Qiskit cast expression has no operand"), + depth + 1U); + return result; + } + if (className == "Index") { + result->kind = ExpressionKind::Index; + result->left = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "target", + "Qiskit index expression has no target"), + depth + 1U); + result->right = normalizePythonExpressionOnly( + pythonAttribute(pythonExpression, "index", + "Qiskit index expression has no index"), + depth + 1U); + return result; + } + if (className == "Stretch") { + throw std::runtime_error( + "Qiskit circuit import does not support stretch expressions"); + } + throw std::runtime_error("Qiskit expression has an unknown Python node"); + } + + void normalizePythonVariable(Expression& result, + const nb::handle pythonExpression) const { + const auto variable = pythonAttribute( + pythonExpression, "var", "Qiskit variable expression has no value"); + const auto circuitModule = nb::module_::import_("qiskit.circuit"); + if (nb::isinstance(variable, circuitModule.attr("Clbit"))) { + if (result.type != ClassicalType::Bool || result.width != 1U) { + throw std::runtime_error( + "Qiskit classical-bit variable must have Boolean type"); + } + result.kind = ExpressionKind::ClassicalBit; + result.bit = rootClbitIndex(variable); + return; + } + if (nb::isinstance(variable, circuitModule.attr("ClassicalRegister"))) { + if (result.type != ClassicalType::Uint || nb::len(variable) == 0U || + nb::len(variable) > 64U || result.width < nb::len(variable)) { + throw std::runtime_error( + "Qiskit classical-register variable has an invalid type"); + } + result.kind = ExpressionKind::ClassicalRegister; + result.reg.name = pythonStringAttribute( + variable, "name", "Qiskit classical register has no name"); + result.reg.bits.reserve(nb::len(variable)); + for (const nb::handle bit : nb::iter(variable)) { + result.reg.bits.push_back(rootClbitIndex(bit)); + } + return; + } + throw std::runtime_error( + "Qiskit circuit import does not support standalone variables in " + "classical expressions"); + } + + [[nodiscard]] std::unique_ptr + normalizePythonExpression(const QkExprNode* expression, + const nb::handle pythonExpression) const { + auto normalizeVariable = [this](Expression& result, + const nb::handle pythonVariable) { + normalizePythonVariable(result, pythonVariable); + }; + return normalizeExpression(expression, pythonExpression, normalizeVariable); + } + const QkCircuit* rootCircuit_ = nullptr; + const QkCircuit* circuit_ = nullptr; + const QkControlFlowInstruction* parent_ = nullptr; QkControlFlowInstruction* controlFlow_ = nullptr; + nb::object instruction_; nb::object operation_; + nb::object containingPythonCircuit_; + nb::object rootPythonCircuit_; }; std::unique_ptr NativeCircuitReader::controlFlow(const size_t index) const { return std::make_unique( - rootCircuit_, circuit_, index, parent_, pythonOperation(index)); + rootCircuit_, circuit_, index, parent_, + nb::borrow(data_[index]), pythonCircuit_, rootPythonCircuit_); } class NativeCircuitWriter final : public CircuitWriter { diff --git a/bindings/mlir/qiskit/QiskitImport.cpp b/bindings/mlir/qiskit/QiskitImport.cpp index 6a91dfec8f..49501f493a 100644 --- a/bindings/mlir/qiskit/QiskitImport.cpp +++ b/bindings/mlir/qiskit/QiskitImport.cpp @@ -582,8 +582,10 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { return mlir::arith::TruncIOp::create(builder, target, value).getResult(); } -[[nodiscard]] mlir::Value emitExpression(mlir::qc::QCProgramBuilder& builder, - const Expression& expression) { +[[nodiscard]] mlir::Value emitExpression( + mlir::qc::QCProgramBuilder& builder, const Expression& expression, + llvm::function_ref emitClassicalBit, + llvm::function_ref emitClassicalRegister) { const auto resultType = expressionType(builder, expression.type, expression.width); switch (expression.kind) { @@ -597,8 +599,19 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { return floatConstant(builder, expression.floatValue); } break; + case ExpressionKind::ClassicalBit: + return emitClassicalBit(expression.bit); + case ExpressionKind::ClassicalRegister: { + const auto target = llvm::dyn_cast(resultType); + if (!target) { + throw std::runtime_error( + "Qiskit classical-register expressions must have Uint type"); + } + return castInteger(builder, emitClassicalRegister(expression.reg), target); + } case ExpressionKind::Cast: { - const auto operand = emitExpression(builder, *expression.left); + const auto operand = emitExpression( + builder, *expression.left, emitClassicalBit, emitClassicalRegister); if (operand.getType() == resultType) { return operand; } @@ -618,7 +631,8 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { throw std::runtime_error("unsupported Qiskit classical-expression cast"); } case ExpressionKind::Unary: { - const auto operand = emitExpression(builder, *expression.left); + const auto operand = emitExpression( + builder, *expression.left, emitClassicalBit, emitClassicalRegister); switch (expression.unaryOperation) { case UnaryOperation::BitNot: { const auto type = llvm::dyn_cast(operand.getType()); @@ -657,8 +671,10 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { break; } case ExpressionKind::Binary: { - auto left = emitExpression(builder, *expression.left); - auto right = emitExpression(builder, *expression.right); + auto left = emitExpression(builder, *expression.left, emitClassicalBit, + emitClassicalRegister); + auto right = emitExpression(builder, *expression.right, emitClassicalBit, + emitClassicalRegister); const auto comparison = [&]() -> std::optional { std::optional integerPredicate; std::optional floatPredicate; @@ -764,8 +780,10 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { throw std::runtime_error("unsupported Qiskit classical binary operation"); } case ExpressionKind::Index: { - const auto target = emitExpression(builder, *expression.left); - auto index = emitExpression(builder, *expression.right); + const auto target = emitExpression(builder, *expression.left, + emitClassicalBit, emitClassicalRegister); + auto index = emitExpression(builder, *expression.right, emitClassicalBit, + emitClassicalRegister); const auto targetType = llvm::dyn_cast(target.getType()); if (!targetType) { throw std::runtime_error( @@ -861,7 +879,14 @@ emitCondition(mlir::qc::QCProgramBuilder& builder, .getResult(); } case ClassicalTargetKind::Expression: { - const auto condition = emitExpression(builder, *target.expression); + const auto condition = emitExpression( + builder, *target.expression, + [&](const uint32_t bit) { + return loadClassicalBit(builder, classicalBits, rootClbitMap, bit); + }, + [&](const Register& reg) { + return packRegister(builder, classicalBits, rootClbitMap, reg); + }); if (!condition.getType().isInteger(1)) { throw std::runtime_error( "Qiskit control-flow condition expression must have Boolean type"); @@ -886,7 +911,14 @@ emitSwitchTarget(mlir::qc::QCProgramBuilder& builder, value = packRegister(builder, classicalBits, rootClbitMap, target.reg); break; case ClassicalTargetKind::Expression: - value = emitExpression(builder, *target.expression); + value = emitExpression( + builder, *target.expression, + [&](const uint32_t bit) { + return loadClassicalBit(builder, classicalBits, rootClbitMap, bit); + }, + [&](const Register& reg) { + return packRegister(builder, classicalBits, rootClbitMap, reg); + }); break; } if (!llvm::isa(value.getType())) { @@ -1390,22 +1422,48 @@ void validateCircuit(const CircuitReader& circuit, uint32_t rootClbits, size_t definitionDepth, size_t controlFlowDepth); -void validateExpression(const Expression& expression) { - if (expression.type == ClassicalType::Uint && - (expression.width == 0U || expression.width > 64U)) { +void validateExpression(const Expression& expression, + const uint32_t rootClbits) { + if ((expression.type == ClassicalType::Bool && expression.width != 1U) || + (expression.type == ClassicalType::Uint && + (expression.width == 0U || expression.width > 64U)) || + (expression.type == ClassicalType::Float && expression.width != 64U)) { throw std::runtime_error( - "Qiskit unsigned classical values must be between 1 and 64 bits"); + "Qiskit classical expression has an invalid type width"); } - const auto requireOperand = [](const std::unique_ptr& operand) { + const auto requireOperand = [&](const std::unique_ptr& operand) { if (!operand) { throw std::runtime_error( "Qiskit classical expression has a missing operand"); } - validateExpression(*operand); + validateExpression(*operand, rootClbits); }; switch (expression.kind) { case ExpressionKind::Value: return; + case ExpressionKind::ClassicalBit: + if (expression.type != ClassicalType::Bool || expression.width != 1U || + expression.bit >= rootClbits) { + throw std::runtime_error( + "Qiskit classical-bit expression has an invalid reference"); + } + return; + case ExpressionKind::ClassicalRegister: { + if (expression.type != ClassicalType::Uint || expression.reg.bits.empty() || + expression.reg.bits.size() > 64U || + expression.width < expression.reg.bits.size()) { + throw std::runtime_error( + "Qiskit classical-register expression has an invalid type"); + } + llvm::DenseSet seen; + for (const auto bit : expression.reg.bits) { + if (bit >= rootClbits || !seen.insert(bit).second) { + throw std::runtime_error( + "Qiskit classical-register expression has an invalid bit"); + } + } + return; + } case ExpressionKind::Unary: case ExpressionKind::Cast: requireOperand(expression.left); @@ -1443,7 +1501,7 @@ void validateTarget(const ClassicalTarget& target, const uint32_t rootClbits) { throw std::runtime_error( "Qiskit control flow contains an empty classical expression"); } - validateExpression(*target.expression); + validateExpression(*target.expression, rootClbits); return; } } diff --git a/bindings/mlir/qiskit/QiskitTranslation.h b/bindings/mlir/qiskit/QiskitTranslation.h index 67c02b66e9..f6780ca7a4 100644 --- a/bindings/mlir/qiskit/QiskitTranslation.h +++ b/bindings/mlir/qiskit/QiskitTranslation.h @@ -191,6 +191,8 @@ enum class ExpressionKind : uint8_t { Cast, Value, Index, + ClassicalBit, + ClassicalRegister, }; enum class BinaryOperation : uint8_t { BitAnd, @@ -227,6 +229,8 @@ struct Expression { bool boolValue = false; uint64_t uintValue = 0; double floatValue = 0.0; + uint32_t bit = 0; + Register reg; std::unique_ptr left; std::unique_ptr right; }; diff --git a/docs/mlir/python_compiler_collection.md b/docs/mlir/python_compiler_collection.md index d7f2fbc329..ed65eafc96 100644 --- a/docs/mlir/python_compiler_collection.md +++ b/docs/mlir/python_compiler_collection.md @@ -172,13 +172,19 @@ program structures than its C API can construct. | Nested `if`/`else`, `for`, `while`, and `switch` | Supported | Rejected | | Classical-bit and register conditions | Supported | Rejected | | Constant Boolean, `Uint` up to 64 bits, and `Float` expressions | Supported | Rejected | -| Standalone classical variables or variable expressions | Rejected | Rejected | +| Clbit and ClassicalRegister expression variables | Supported | Rejected | +| Standalone classical runtime variables | Rejected | Rejected | | Free symbols and supported real parameter expressions | Supported | Supported | | Parameter-vector elements | Rejected | Not emitted | | Dense numeric unitaries up to eight qubits | Supported | Supported | | Register aliases or interleaved membership | Rejected | Rejected | | Transpiler layout metadata | Accepted and ignored | Not emitted | +Classical-expression variables may refer to Clbits or ClassicalRegisters in the +containing circuit. This includes values used only by the condition or switch +target and not by a control-flow block. Standalone runtime variables remain +unsupported. + Free standalone symbols become named {code}`f64` program inputs. Parameter-vector elements are rejected because converting them to standalone parameters would change positional binding order. Standalone parameter names diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index e89e19f9db..a2196479a8 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -35,6 +35,7 @@ library, ) from qiskit.circuit.classical import expr, types +from qiskit.circuit.controlflow import CASE_DEFAULT from qiskit.quantum_info import Operator, random_unitary from mqt.core.mlir import CompilerTarget, QCProgram, compile_program @@ -1024,6 +1025,168 @@ def test_bool_uint_and_float_expressions(condition: expr.Expr, operation: str) - assert operation in program.ir +def _round_trip_qiskit_import(circuit: QuantumCircuit) -> str: + program = QCProgram.from_qiskit(circuit) + assert QCProgram.from_mlir_str(program.ir).ir == program.ir + return program.ir + + +def _cbit_load_indices(ir: str) -> list[int]: + constants = { + name: int(value) for name, value in re.findall(r"(?m)^\s*(%[-\w.$]+) = arith\.constant (\d+) : index$", ir) + } + return [constants[name] for name in re.findall(r"(?m)^\s*%[-\w.$]+ = cbit\.load [^\[]+\[(%[-\w.$]+)\]", ir)] + + +def test_classical_expression_clbit_captures_round_trip_on_import() -> None: + """Keep Clbit identity when an expression capture uses a nontrivial order.""" + circuit = QuantumCircuit(1, 2) + condition = expr.logic_and(circuit.clbits[1], expr.logic_not(circuit.clbits[0])) + with circuit.if_test(condition): + circuit.x(0) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [1, 0] + assert "arith.xori" in ir + assert "arith.andi" in ir + assert "scf.if" in ir + + +def test_classical_expression_register_captures_round_trip_on_import() -> None: + """Pack a captured register in Qiskit's little-endian bit order.""" + circuit = QuantumCircuit(1, 3) + condition = expr.equal(expr.bit_xor(circuit.cregs[0], 1), 5) + with circuit.if_test(condition): + circuit.x(0) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [0, 1, 2] + assert ir.count("arith.shli") == 2 + assert "arith.xori" in ir + assert "arith.cmpi eq" in ir + + +def test_nested_classical_expression_captures_round_trip_on_import() -> None: + """Compose nested local capture maps without changing root Clbit identity.""" + circuit = QuantumCircuit(1, 3) + with circuit.if_test(expr.logic_not(circuit.clbits[2])): + condition = expr.logic_and(circuit.clbits[0], expr.logic_not(circuit.clbits[1])) + with circuit.while_loop(condition, None, None, None, label=None): + circuit.x(0) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [2, 0, 1] + assert "scf.if" in ir + assert "scf.while" in ir + + +def test_switch_expression_captures_round_trip_on_import() -> None: + """Read an expression switch target through Qiskit's public Python tree.""" + circuit = QuantumCircuit(1, 2) + with circuit.switch(expr.bit_xor(circuit.cregs[0], 1), None, None, None, label=None) as case: + with case(0): + circuit.x(0) + with case(case.DEFAULT): + circuit.h(0) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [0, 1] + assert "arith.xori" in ir + assert "scf.index_switch" in ir + + +def test_condition_only_clbit_expression_round_trips_on_import() -> None: + """Resolve a condition bit that no control-flow block uses.""" + body = QuantumCircuit(1) + body.x(0) + circuit = QuantumCircuit(1, 1) + circuit.if_test(expr.logic_not(circuit.clbits[0]), body, [circuit.qubits[0]], []) + + assert len(circuit.data[0].clbits) == 0 + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [0] + assert "arith.xori" in ir + assert "scf.if" in ir + + +def test_condition_only_switch_expression_round_trips_on_import() -> None: + """Resolve a switch register that no case block uses.""" + zero = QuantumCircuit(1) + zero.x(0) + default = QuantumCircuit(1) + default.h(0) + circuit = QuantumCircuit(1, 2) + # Qiskit's overload omits expression targets although its runtime accepts them. + circuit.switch( # ty: ignore[no-matching-overload] + expr.bit_xor(circuit.cregs[0], 1), + [(0, zero), (CASE_DEFAULT, default)], + [circuit.qubits[0]], + [], + ) + + assert len(circuit.data[0].clbits) == 0 + assert all(block.num_clbits == 0 for block in circuit.data[0].operation.blocks) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [0, 1] + assert "arith.xori" in ir + assert "scf.index_switch" in ir + + +def test_nested_condition_only_expression_uses_parent_capture_map() -> None: + """Map a nested condition-only bit through its enclosing block.""" + inner_body = QuantumCircuit(1) + inner_body.x(0) + middle = QuantumCircuit(1, 2) + middle.if_test(expr.logic_not(middle.clbits[0]), inner_body, [middle.qubits[0]], []) + circuit = QuantumCircuit(1, 2) + circuit.if_test( + (circuit.clbits[0], True), + middle, + [circuit.qubits[0]], + [circuit.clbits[1], circuit.clbits[0]], + ) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [0, 1] + assert ir.count("scf.if") == 2 + + +def test_nested_legacy_clbit_condition_uses_root_index() -> None: + """Resolve a nested tuple condition through its enclosing Clbit map.""" + circuit = QuantumCircuit(2, 2) + with circuit.for_loop(range(2), None, None, None, None, label=None) as iteration: + circuit.rx(iteration, 0) + with circuit.if_test((circuit.clbits[1], True)): + circuit.x(0) + + ir = _round_trip_qiskit_import(circuit) + + assert _cbit_load_indices(ir) == [1] + assert "scf.for" in ir + assert "scf.if" in ir + + +def test_classical_expression_rejects_mismatched_instruction_captures() -> None: + """Reject an instruction capture list that does not match its block.""" + circuit = QuantumCircuit(1, 1) + with circuit.if_test(expr.logic_not(circuit.clbits[0])): + circuit.x(0) + instruction = circuit.data[0] + circuit._data[0] = instruction.replace(clbits=()) # ruff: ignore[private-member-access] + + with pytest.raises(RuntimeError, match="incompatible classical-bit captures"): + QCProgram.from_qiskit(circuit) + + def test_excessively_nested_classical_expression_is_rejected() -> None: """Bound native normalization before recursive expression traversal.""" condition: expr.Expr = expr.equal(1, 1) From 799fa1093f0a6bc22c938e95f62a2ce9d0ceee3a Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Fri, 21 Aug 2026 15:36:49 +0200 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=90=9B=20Fix=20Qiskit=20classical?= =?UTF-8?q?=20expression=20semantics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- bindings/mlir/qiskit/Qiskit2_5.cpp | 302 ++++---------------- bindings/mlir/qiskit/QiskitImport.cpp | 103 ++++++- test/python/test_mlir_qiskit_translation.py | 93 +++++- 3 files changed, 244 insertions(+), 254 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index 59bedc994f..5461c4b6b5 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -577,212 +578,6 @@ void normalizePythonGate(const nb::handle operation, Instruction& result, "Qiskit operation has an invalid name"); } -[[nodiscard]] ClassicalType normalizeType(const QkExprTypeInfo type) { - switch (type.ty) { - case QkExprType_Bool: - return ClassicalType::Bool; - case QkExprType_Uint: - if (type.width == 0U || type.width > 64U) { - throw std::runtime_error("Qiskit unsigned classical values wider than 64 " - "bits are not supported"); - } - return ClassicalType::Uint; - case QkExprType_Float: - return ClassicalType::Float; - case QkExprType_Duration: - throw std::runtime_error( - "Qiskit circuit import does not support duration expressions"); - } - throw std::runtime_error( - "Qiskit returned an unknown classical expression type"); -} - -void setType(Expression& result, const QkExprTypeInfo type) { - result.type = normalizeType(type); - if (result.type == ClassicalType::Bool) { - result.width = 1U; - } else if (result.type == ClassicalType::Float) { - result.width = 64U; - } else { - result.width = static_cast(type.width); - } -} - -[[nodiscard]] BinaryOperation -normalizeBinaryOperation(const QkBinaryOpType op) { - switch (op) { - case QkBinaryOpType_BitAnd: - return BinaryOperation::BitAnd; - case QkBinaryOpType_BitOr: - return BinaryOperation::BitOr; - case QkBinaryOpType_BitXor: - return BinaryOperation::BitXor; - case QkBinaryOpType_LogicAnd: - return BinaryOperation::LogicAnd; - case QkBinaryOpType_LogicOr: - return BinaryOperation::LogicOr; - case QkBinaryOpType_Equal: - return BinaryOperation::Equal; - case QkBinaryOpType_NotEqual: - return BinaryOperation::NotEqual; - case QkBinaryOpType_Less: - return BinaryOperation::Less; - case QkBinaryOpType_LessEqual: - return BinaryOperation::LessEqual; - case QkBinaryOpType_Greater: - return BinaryOperation::Greater; - case QkBinaryOpType_GreaterEqual: - return BinaryOperation::GreaterEqual; - case QkBinaryOpType_ShiftLeft: - return BinaryOperation::ShiftLeft; - case QkBinaryOpType_ShiftRight: - return BinaryOperation::ShiftRight; - case QkBinaryOpType_Add: - return BinaryOperation::Add; - case QkBinaryOpType_Sub: - return BinaryOperation::Subtract; - case QkBinaryOpType_Mul: - return BinaryOperation::Multiply; - case QkBinaryOpType_Div: - return BinaryOperation::Divide; - } - throw std::runtime_error( - "Qiskit returned an unknown binary expression operation"); -} - -[[nodiscard]] UnaryOperation normalizeUnaryOperation(const QkUnaryOpType op) { - switch (op) { - case QkUnaryOpType_BitNot: - return UnaryOperation::BitNot; - case QkUnaryOpType_LogicNot: - return UnaryOperation::LogicNot; - case QkUnaryOpType_Negate: - return UnaryOperation::Negate; - } - throw std::runtime_error( - "Qiskit returned an unknown unary expression operation"); -} - -template -[[nodiscard]] std::unique_ptr normalizeExpression( - const QkExprNode* expression, const nb::handle pythonExpression, - NormalizeVariable& normalizeVariable, const size_t depth = 0U) { - if (expression == nullptr) { - throw std::runtime_error("Qiskit returned a null classical expression"); - } - if (depth >= MAX_EXPRESSION_DEPTH) { - throw std::runtime_error( - "Qiskit classical expressions exceed the nesting limit of 64"); - } - auto result = std::make_unique(); - switch (qk_expr_kind(expression)) { - case QkExprNodeKind_Binary: { - const auto info = qk_expr_binary_info(expression); - result->kind = ExpressionKind::Binary; - result->binaryOperation = normalizeBinaryOperation(info.op); - setType(*result, info.ty); - result->left = normalizeExpression( - info.left, - pythonAttribute(pythonExpression, "left", - "Qiskit binary expression has no left operand"), - normalizeVariable, depth + 1U); - result->right = normalizeExpression( - info.right, - pythonAttribute(pythonExpression, "right", - "Qiskit binary expression has no right operand"), - normalizeVariable, depth + 1U); - return result; - } - case QkExprNodeKind_Unary: { - const auto info = qk_expr_unary_info(expression); - result->kind = ExpressionKind::Unary; - result->unaryOperation = normalizeUnaryOperation(info.op); - setType(*result, info.ty); - result->left = normalizeExpression( - info.operand, - pythonAttribute(pythonExpression, "operand", - "Qiskit unary expression has no operand"), - normalizeVariable, depth + 1U); - return result; - } - case QkExprNodeKind_Cast: { - const auto info = qk_expr_cast_info(expression); - result->kind = ExpressionKind::Cast; - setType(*result, info.ty); - result->left = normalizeExpression( - info.operand, - pythonAttribute(pythonExpression, "operand", - "Qiskit cast expression has no operand"), - normalizeVariable, depth + 1U); - return result; - } - case QkExprNodeKind_Index: { - const auto info = qk_expr_index_info(expression); - result->kind = ExpressionKind::Index; - setType(*result, info.ty); - result->left = normalizeExpression( - info.target, - pythonAttribute(pythonExpression, "target", - "Qiskit index expression has no target"), - normalizeVariable, depth + 1U); - result->right = normalizeExpression( - info.index, - pythonAttribute(pythonExpression, "index", - "Qiskit index expression has no index"), - normalizeVariable, depth + 1U); - return result; - } - case QkExprNodeKind_Value: { - const auto* value = qk_expr_as_value(expression); - const auto type = qk_value_type_info(value); - result->kind = ExpressionKind::Value; - setType(*result, type); - switch (result->type) { - case ClassicalType::Bool: - result->boolValue = qk_value_bool(value); - break; - case ClassicalType::Uint: - result->uintValue = qk_value_uint(value); - break; - case ClassicalType::Float: - result->floatValue = qk_value_float(value); - if (!std::isfinite(result->floatValue)) { - throw std::runtime_error( - "Qiskit classical floating-point literals must be finite"); - } - break; - } - return result; - } - case QkExprNodeKind_Var: - setType(*result, qk_var_type_info(qk_expr_as_var(expression))); - normalizeVariable(*result, pythonExpression); - return result; - case QkExprNodeKind_Stretch: - throw std::runtime_error( - "Qiskit circuit import does not support stretch expressions"); - } - throw std::runtime_error( - "Qiskit returned an unknown classical expression node"); -} - -[[nodiscard]] Register normalizeRegister(const QkClassicalRegister* reg, - const QkCircuit* rootCircuit) { - // qk_str_free requires the mutable allocation returned by Qiskit. - // NOLINTNEXTLINE(misc-const-correctness) - char* const name = qk_classical_register_name(reg); - if (name == nullptr) { - throwPythonError("Qiskit failed to read a classical-register name"); - } - Register result{.name = name}; - qk_str_free(name); - result.bits.resize(qk_classical_register_num_bits(reg)); - if (!result.bits.empty()) { - qk_classical_register_circuit_bits(reg, rootCircuit, result.bits.data()); - } - return result; -} - class OwnedParameter final { public: OwnedParameter() : value_(qk_param_zero()) { @@ -1337,49 +1132,59 @@ class NativeControlFlowReader final : public ControlFlowReader { [[nodiscard]] ClassicalTarget condition() const override { ClassicalTarget result; - switch (qk_control_flow_condition_type(controlFlow_)) { - case QkConditionType_ClBit: { - const auto bit = qk_control_flow_condition_bit_info(controlFlow_); - const auto condition = pythonAttribute( - operation_, "condition", "Qiskit control flow has no condition"); - if (nb::len(condition) != 2U) { + const auto condition = pythonAttribute( + operation_, "condition", "Qiskit control flow has no condition"); + const auto expressionModule = + nb::module_::import_("qiskit.circuit.classical.expr"); + if (nb::isinstance(condition, expressionModule.attr("Expr"))) { + result.kind = ClassicalTargetKind::Expression; + result.expression = normalizePythonExpressionOnly(condition); + return result; + } + + const auto tupleType = nb::module_::import_("builtins").attr("tuple"); + if (!nb::isinstance(condition, tupleType) || nb::len(condition) != 2U) { + throw std::runtime_error("Qiskit control-flow condition has an invalid " + "shape"); + } + const nb::handle target = condition[0]; + uint64_t expected = 0U; + if (!nb::try_cast(condition[1], expected)) { + throw std::runtime_error( + "Qiskit control-flow condition has an invalid value"); + } + + const auto circuitModule = nb::module_::import_("qiskit.circuit"); + if (nb::isinstance(target, circuitModule.attr("Clbit"))) { + if (expected > 1U) { throw std::runtime_error( - "Qiskit classical-bit condition has an invalid shape"); + "Qiskit classical-bit condition must compare against zero or one"); } result.kind = ClassicalTargetKind::ClassicalBit; - result.bit = rootClbitIndex(condition[0]); - result.expectedBit = bit.condition; + result.bit = rootClbitIndex(target); + result.expectedBit = expected != 0U; return result; } - case QkConditionType_ClReg: { - const auto conditionWidth = - qk_control_flow_condition_reg_cond_bit_width(controlFlow_); - if (conditionWidth > 64U) { + if (nb::isinstance(target, circuitModule.attr("ClassicalRegister"))) { + const auto size = nb::len(target); + if (size == 0U || size > 64U) { throw std::runtime_error( - "Qiskit register conditions wider than 64 bits are not supported"); + "Qiskit register conditions require between 1 and 64 bits"); } result.kind = ClassicalTargetKind::ClassicalRegister; - result.reg = normalizeRegister( - qk_control_flow_condition_reg(controlFlow_), rootCircuit_); - if (result.reg.bits.empty() || result.reg.bits.size() > 64U) { - throw std::runtime_error( - "Qiskit register conditions require between 1 and 64 bits"); + result.reg.name = pythonStringAttribute( + target, "name", "Qiskit condition register has no name"); + result.reg.bits.reserve(size); + for (const nb::handle bit : nb::iter(target)) { + result.reg.bits.push_back(rootClbitIndex(bit)); } result.width = static_cast( - std::max(conditionWidth, result.reg.bits.size())); - result.expectedRegister = - qk_control_flow_condition_reg_cond_uint(controlFlow_); + std::max(size, std::bit_width(expected))); + result.expectedRegister = expected; return result; } - case QkConditionType_Expr: - result.kind = ClassicalTargetKind::Expression; - result.expression = normalizePythonExpression( - qk_control_flow_condition_expr(controlFlow_), - pythonAttribute(operation_, "condition", - "Qiskit control flow has no condition")); - return result; - } - throw std::runtime_error("Qiskit returned an unknown condition type"); + throw std::runtime_error("Qiskit control flow has an unknown condition " + "target"); } [[nodiscard]] Loop loop() const override { @@ -1688,16 +1493,21 @@ class NativeControlFlowReader final : public ControlFlowReader { const auto value = pythonAttribute( pythonExpression, "value", "Qiskit literal expression has no value"); switch (result->type) { - case ClassicalType::Bool: - if (!nb::try_cast(value, result->boolValue)) { + case ClassicalType::Bool: { + uint64_t boolValue = 0U; + if (!nb::try_cast(value, boolValue) || boolValue > 1U) { throw std::runtime_error( "Qiskit Boolean expression has an invalid value"); } + result->boolValue = boolValue != 0U; break; + } case ClassicalType::Uint: - if (!nb::try_cast(value, result->uintValue)) { + if (!nb::try_cast(value, result->uintValue) || + (result->width < 64U && + result->uintValue >= (uint64_t{1} << result->width))) { throw std::runtime_error( - "Qiskit Uint expression has an invalid value"); + "Qiskit Uint literal does not fit its declared width"); } break; case ClassicalType::Float: @@ -1799,16 +1609,6 @@ class NativeControlFlowReader final : public ControlFlowReader { "classical expressions"); } - [[nodiscard]] std::unique_ptr - normalizePythonExpression(const QkExprNode* expression, - const nb::handle pythonExpression) const { - auto normalizeVariable = [this](Expression& result, - const nb::handle pythonVariable) { - normalizePythonVariable(result, pythonVariable); - }; - return normalizeExpression(expression, pythonExpression, normalizeVariable); - } - const QkCircuit* rootCircuit_ = nullptr; const QkCircuit* circuit_ = nullptr; const QkControlFlowInstruction* parent_ = nullptr; diff --git a/bindings/mlir/qiskit/QiskitImport.cpp b/bindings/mlir/qiskit/QiskitImport.cpp index 49501f493a..7c3afca930 100644 --- a/bindings/mlir/qiskit/QiskitImport.cpp +++ b/bindings/mlir/qiskit/QiskitImport.cpp @@ -615,6 +615,21 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { if (operand.getType() == resultType) { return operand; } + if (expression.type == ClassicalType::Bool) { + if (const auto source = + llvm::dyn_cast(operand.getType())) { + return mlir::arith::CmpIOp::create( + builder, mlir::arith::CmpIPredicate::ne, operand, + integerConstant(builder, source.getWidth(), 0U)) + .getResult(); + } + if (operand.getType().isF64()) { + return mlir::arith::CmpFOp::create(builder, + mlir::arith::CmpFPredicate::UNE, + operand, floatConstant(builder, 0.0)) + .getResult(); + } + } if (const auto target = llvm::dyn_cast(resultType)) { if (llvm::isa(operand.getType())) { return castInteger(builder, operand, target); @@ -1438,6 +1453,19 @@ void validateExpression(const Expression& expression, } validateExpression(*operand, rootClbits); }; + const auto sameType = [](const Expression& first, const Expression& second) { + return first.type == second.type && first.width == second.width; + }; + const auto hasType = [](const Expression& value, const ClassicalType type) { + return value.type == type; + }; + const auto requireCompatible = [](const bool compatible) { + if (!compatible) { + throw std::runtime_error( + "Qiskit classical expression has incompatible operator and operand " + "types"); + } + }; switch (expression.kind) { case ExpressionKind::Value: return; @@ -1464,14 +1492,85 @@ void validateExpression(const Expression& expression, } return; } - case ExpressionKind::Unary: + case ExpressionKind::Unary: { + requireOperand(expression.left); + const auto& operand = *expression.left; + switch (expression.unaryOperation) { + case UnaryOperation::BitNot: + requireCompatible((hasType(operand, ClassicalType::Bool) || + hasType(operand, ClassicalType::Uint)) && + sameType(expression, operand)); + return; + case UnaryOperation::LogicNot: + requireCompatible(hasType(expression, ClassicalType::Bool) && + hasType(operand, ClassicalType::Bool)); + return; + case UnaryOperation::Negate: + requireCompatible(hasType(expression, ClassicalType::Float) && + hasType(operand, ClassicalType::Float)); + return; + } + return; + } case ExpressionKind::Cast: requireOperand(expression.left); return; - case ExpressionKind::Binary: + case ExpressionKind::Binary: { + requireOperand(expression.left); + requireOperand(expression.right); + const auto& left = *expression.left; + const auto& right = *expression.right; + switch (expression.binaryOperation) { + case BinaryOperation::BitAnd: + case BinaryOperation::BitOr: + case BinaryOperation::BitXor: + requireCompatible(sameType(left, right) && sameType(expression, left) && + (hasType(left, ClassicalType::Bool) || + hasType(left, ClassicalType::Uint))); + return; + case BinaryOperation::LogicAnd: + case BinaryOperation::LogicOr: + requireCompatible(hasType(expression, ClassicalType::Bool) && + hasType(left, ClassicalType::Bool) && + hasType(right, ClassicalType::Bool)); + return; + case BinaryOperation::Equal: + case BinaryOperation::NotEqual: + requireCompatible(hasType(expression, ClassicalType::Bool) && + sameType(left, right)); + return; + case BinaryOperation::Less: + case BinaryOperation::LessEqual: + case BinaryOperation::Greater: + case BinaryOperation::GreaterEqual: + requireCompatible(hasType(expression, ClassicalType::Bool) && + sameType(left, right) && + (hasType(left, ClassicalType::Uint) || + hasType(left, ClassicalType::Float))); + return; + case BinaryOperation::ShiftLeft: + case BinaryOperation::ShiftRight: + requireCompatible(hasType(left, ClassicalType::Uint) && + hasType(right, ClassicalType::Uint) && + sameType(expression, left)); + return; + case BinaryOperation::Add: + case BinaryOperation::Subtract: + case BinaryOperation::Multiply: + case BinaryOperation::Divide: + requireCompatible(sameType(left, right) && sameType(expression, left) && + (hasType(left, ClassicalType::Uint) || + hasType(left, ClassicalType::Float))); + return; + } + return; + } case ExpressionKind::Index: requireOperand(expression.left); requireOperand(expression.right); + requireCompatible(hasType(expression, ClassicalType::Bool) && + hasType(*expression.left, ClassicalType::Uint) && + hasType(*expression.right, ClassicalType::Uint)); return; } } diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index a2196479a8..5fb7e5f668 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -35,7 +35,7 @@ library, ) from qiskit.circuit.classical import expr, types -from qiskit.circuit.controlflow import CASE_DEFAULT +from qiskit.circuit.controlflow import CASE_DEFAULT, IfElseOp from qiskit.quantum_info import Operator, random_unitary from mqt.core.mlir import CompilerTarget, QCProgram, compile_program @@ -1011,6 +1011,7 @@ def test_qiskit_import_zero_initializes_clbits_before_control_flow() -> None: expr.greater(expr.cast(expr.lift(2, types.Uint(8)), types.Float()), 0.5), "arith.uitofp", ), + (expr.cast(expr.lift(0.5, types.Float()), types.Bool()), "arith.cmpf une"), (expr.greater(expr.negate(expr.lift(0.5, types.Float())), -1.0), "arith.negf"), ], ) @@ -1038,6 +1039,96 @@ def _cbit_load_indices(ir: str) -> list[int]: return [constants[name] for name in re.findall(r"(?m)^\s*%[-\w.$]+ = cbit\.load [^\[]+\[(%[-\w.$]+)\]", ir)] +def test_boolean_expression_literals_are_imported() -> None: + """Normalize Qiskit's integer-backed Boolean Value nodes.""" + false_literal = False + true_literal = True + circuit = QuantumCircuit(1) + with circuit.if_test(expr.logic_or(expr.lift(false_literal), expr.lift(true_literal))): + circuit.x(0) + + ir = QCProgram.from_qiskit(circuit).ir + + assert "arith.constant false" in ir + assert "arith.constant true" in ir + assert "arith.ori" in ir + + +def test_uint_register_cast_to_bool_tests_all_bits() -> None: + """Treat a Uint register as true when any bit is set.""" + circuit = QuantumCircuit(1, 2) + circuit.x(0) + circuit.measure(0, 1) + with circuit.if_test(expr.cast(circuit.cregs[0], types.Bool())): + circuit.z(0) + + ir = QCProgram.from_qiskit(circuit).ir + + assert "arith.cmpi ne" in ir + assert "arith.trunci" not in ir + + +def test_public_expression_condition_mutation_is_observed() -> None: + """Import the current public expression after condition mutation.""" + circuit = QuantumCircuit(1, 2) + with circuit.if_test(expr.logic_and(circuit.clbits[0], circuit.clbits[1])): + circuit.x(0) + operation = circuit.data[0].operation + assert isinstance(operation, IfElseOp) + operation.condition = expr.logic_or(circuit.clbits[0], circuit.clbits[1]) + + ir = QCProgram.from_qiskit(circuit).ir + + assert "arith.ori" in ir + assert "arith.andi" not in ir + + +def test_public_tuple_condition_mutation_is_observed() -> None: + """Import the current bit and value after tuple-condition mutation.""" + body = QuantumCircuit(1) + body.x(0) + circuit = QuantumCircuit(1, 2) + circuit.if_test((circuit.clbits[0], False), body, circuit.qubits, []) + operation = circuit.data[0].operation + assert isinstance(operation, IfElseOp) + operation.condition = (circuit.clbits[1], True) + + ir = QCProgram.from_qiskit(circuit).ir + + assert _cbit_load_indices(ir) == [1] + assert "arith.constant true" in ir + assert "arith.constant false" not in ir + + +def test_narrow_uint_switch_literal_is_rejected() -> None: + """Reject a Uint literal that does not fit its declared width.""" + circuit = QuantumCircuit(1, 1) + with circuit.switch(expr.Value(3, types.Uint(1)), None, None, None, label=None) as case, case(0): + circuit.x(0) + + with pytest.raises(RuntimeError, match=r"Uint literal.*does not fit"): + QCProgram.from_qiskit(circuit) + + +def test_malformed_public_expression_type_is_rejected() -> None: + """Reject a public expression whose declared result type is inconsistent.""" + invalid = expr.Binary( + expr.Binary.Op.ADD, + expr.Value(1, types.Uint(1)), + expr.Value(1, types.Uint(1)), + types.Bool(), + ) + circuit = QuantumCircuit(1) + with circuit.if_test(expr.equal(1, 1)): + circuit.x(0) + operation = circuit.data[0].operation + assert isinstance(operation, IfElseOp) + operation.condition = invalid + + with pytest.raises(RuntimeError, match="incompatible operator and operand types"): + QCProgram.from_qiskit(circuit) + + def test_classical_expression_clbit_captures_round_trip_on_import() -> None: """Keep Clbit identity when an expression capture uses a nontrivial order.""" circuit = QuantumCircuit(1, 2) From 5406ea68f4a78d5c18b1b069fb7b481e732f6bed Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Fri, 21 Aug 2026 15:37:48 +0200 Subject: [PATCH 03/11] =?UTF-8?q?=F0=9F=93=9D=20Update=20Qiskit=20capture?= =?UTF-8?q?=20plan=20and=20changelog?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../qiskit-classical-expression-captures.md | 135 ++++++++++++++---- CHANGELOG.md | 3 +- 2 files changed, 106 insertions(+), 32 deletions(-) diff --git a/.agent/plans/qiskit-classical-expression-captures.md b/.agent/plans/qiskit-classical-expression-captures.md index 5d7a088a2e..7a5849a600 100644 --- a/.agent/plans/qiskit-classical-expression-captures.md +++ b/.agent/plans/qiskit-classical-expression-captures.md @@ -60,6 +60,23 @@ Qiskit control-flow operations during export. - [x] (2026-08-19 20:08Z) Restack onto the audited scalar parent, update the recorded parent identity, rebuild the release binding, and pass all 167 Qiskit translation tests. +- [x] (2026-08-21 12:30Z) Rebase the capture-only change onto current `main` + after symbolic Qiskit parameter support merged, dropping the superseded + parent commit while preserving the focused six-file feature delta. +- [x] (2026-08-21 13:05Z) Reproduce three review findings: stale native + condition operators after public Python mutation, an aborting out-of-range + `Uint` switch literal, and low-bit truncation for `Uint`-to-`Bool` casts. +- [x] (2026-08-21 13:25Z) Make conditions Python-authoritative, validate literal + widths, lower Boolean casts as nonzero comparisons, remove the dead hybrid + native-expression walker, normalize integer-backed Boolean values, and + preflight operator/type compatibility. +- [x] (2026-08-21 13:35Z) Rebuild and refresh the editable MLIR binding, pass + all 21 focused capture and corrective cases, and pass all 174 Qiskit + translation tests against the updated extension. +- [x] (2026-08-21 13:40Z) Pass the complete repository lint session, pinned + formatting hooks, and `git diff --check`. +- [x] (2026-08-21 13:45Z) Inspect the final diff, create separate gitmoji + implementation and documentation commits, and push only PR #2175. ## Surprises & Discoveries @@ -100,6 +117,32 @@ Qiskit control-flow operations during export. zero; resolving the Python condition bit through the enclosing map emits index one. +- Observation: Qiskit's public control-flow condition setter updates the Python + operation while the native control-flow view can retain the tree recorded at + insertion time. Evidence: mutating a public condition from logical AND to OR + leaves the native operator as AND, so combining native operators with Python + leaves silently imports a mixed, stale expression. + +- Observation: Qiskit accepts a public `expr.Value(3, Uint(1))` switch target, + so the importer must reject a value that does not fit its declared width + before constructing an LLVM `APInt`. Without the preflight, LLVM aborts the + Python process instead of reporting a recoverable import error. + +- Observation: Public Qiskit Boolean `Value` nodes expose their value as Python + integer zero or one. A strict nanobind conversion to C++ `bool` rejects both, + so normalization must accept only the integer range `[0, 1]` and convert it + explicitly. + +- Observation: A Qiskit cast to `Bool` tests whether the complete source value + is nonzero. Truncating a packed register to `i1` inspects only its least + significant bit; for example, `0b10` must be true rather than false. + +- Observation: Qiskit's low-level public expression constructors and public + condition setter permit a node whose declared result type conflicts with its + operator and operands. The Python-authoritative path therefore needs its own + recursive type preflight rather than relying on constructor helpers having + produced every tree. + ## Decision Log - Decision: Add `ClassicalBit` and `ClassicalRegister` to `ExpressionKind`, with @@ -107,7 +150,7 @@ Qiskit control-flow operations during export. Rationale: The normalized tree then owns stable capture identity and stays independent of Python object lifetimes. Date/Author: 2026-08-19 / Codex. -- Decision: Keep `ParameterKind`, `Parameter`, and `Loop::parameter` unchanged. +- Decision: Keep the scalar `Parameter` model and `Loop::parameter` unchanged. Rationale: Scalar symbols and classical captures have different identity and typing rules. This branch must remain composable with the reviewed scalar slice. Date/Author: 2026-08-19 / Codex. @@ -128,6 +171,24 @@ Qiskit control-flow operations during export. established native control-flow reader for supported metadata. Date/Author: 2026-08-19 / Codex. +- Decision: Parse the complete public Python condition for expressions, Clbits, + registers, and comparison values; retain native metadata only for blocks, + capture maps, loops, and switch cases. Rationale: one authoritative tree + prevents stale native operators or values from being combined with current + Python capture identities. Date/Author: 2026-08-21 / Codex. + +- Decision: Range-check every unsigned literal against its normalized width and + lower casts to `Bool` with integer or unordered floating-point comparisons + against zero. Rationale: malformed public inputs must raise a runtime error, + and Boolean conversion must inspect the whole value, including NaN for Qiskit + floating-point expressions. Date/Author: 2026-08-21 / Codex. + +- Decision: Validate operator, operand, and result-type compatibility on the + normalized expression before emitting MLIR. Rationale: malformed public trees + must fail deterministically during preflight instead of producing ill-typed + semantics or partially constructing a program. Date/Author: 2026-08-21 / + Codex. + - Decision: Document circuit Clbit and ClassicalRegister expression variables separately from standalone runtime variables. Rationale: circuit-owned bits resolve to existing CBit state whether or not a block captures them; the @@ -145,12 +206,13 @@ work when their classical bits are absent from every block operand. The public support table distinguishes these supported circuit values from rejected standalone runtime inputs. -The release MLIR binding built successfully. The complete Qiskit translation -test file passed with 167 tests against that local extension, including the -subprocess isolation test, the condition-only regressions, and the nested legacy -Clbit condition. `uvx nox -s lint`, `git diff --check`, Clang format, Ruff, -Rumdl, Prettier, and `ty` all passed. Export-side writer construction remains -deliberately out of scope. +After rebasing onto current `main`, the corrective review pass rebuilt and +refreshed the MLIR binding, passed 21 focused cases covering current public +conditions, bounded literals, Boolean casts, and malformed expression typing, +and passed all 174 tests in the complete Qiskit translation file. The complete +repository lint session, pinned Clang and Python formatting, Rumdl, Ruff, `ty`, +targeted Clang-Tidy 21.1.1, and `git diff --check` all pass. Export-side writer +construction remains deliberately out of scope. ## Context and Orientation @@ -182,22 +244,24 @@ First, extend `ExpressionKind` and `Expression` in `bindings/mlir/qiskit/QiskitTranslation.h` with bit and register leaves. Keep all scalar parameter declarations byte-for-byte unchanged. -Next, update `bindings/mlir/qiskit/Qiskit2_5.cpp`. Make the native expression -normalizer walk the matching public Python expression node beside each native -node. Resolve a `Var` leaf by inspecting its public `var` object. For a Clbit, -find the bit in the containing Python circuit and compose the local index -through the enclosing native capture map. Use the same resolver for a legacy -tuple condition's Clbit instead of trusting its native local index. For a -classical register, apply the same mapping to each member in register order. -Reject malformed captures, duplicate or invalid types, standalone variables, and -widths outside the existing 64-bit limit. Keep a Python-only expression walker -for switch targets so no unsafe native switch-expression accessor is called. +Next, update `bindings/mlir/qiskit/Qiskit2_5.cpp`. Normalize condition and +switch expression trees entirely from the current public Python operation. +Resolve a `Var` leaf by inspecting its public `var` object. For a Clbit, find +the bit in the containing Python circuit and compose the local index through the +enclosing native capture map. Use the same resolver for a legacy tuple +condition's Clbit instead of trusting its native local index. For a classical +register, apply the same mapping to each member in register order. Reject +malformed captures, duplicate or invalid types, standalone variables, and widths +outside the existing 64-bit limit, and reject unsigned literals that do not fit +their declared width before MLIR construction. Then update `bindings/mlir/qiskit/QiskitImport.cpp`. Pass callbacks into the recursive expression emitter. A bit leaf calls `loadClassicalBit`; a register leaf calls `packRegister` and extends it to the normalized expression width. -Extend preflight validation to check leaf types, bit bounds, register size, -unique register bits, and expression widths before MLIR construction begins. +Lower casts to `Bool` as nonzero comparisons instead of integer truncation or +floating-point conversion. Extend preflight validation to check leaf types, bit +bounds, register size, unique register bits, and expression widths before MLIR +construction begins. Finally, add tests to `test/python/test_mlir_qiskit_translation.py`. Cover one captured Clbit expression, one captured register expression, nested control flow @@ -220,16 +284,17 @@ Inspect the focused diff and formatting: bindings/mlir/qiskit/QiskitTranslation.h uvx ruff check test/python/test_mlir_qiskit_translation.py -Build the Qiskit binding with the configured release tree. If the isolated -worktree has no compatible build tree yet, configure it with the repository's -release preset first: +Build the Qiskit binding with the configured Python release tree. If the +worktree has no compatible build tree yet, refresh the editable installation +through the repository's standard `uv` workflow first: - cmake --build build/release --parallel 8 + cmake --build build/python/Release --target mqt-core-mlir-bindings --parallel 8 + uv sync --inexact --no-dev --no-build-isolation-package mqt-core Run the focused tests: uv run --no-sync pytest test/python/test_mlir_qiskit_translation.py \ - -k 'classical_expression or condition_only or switch_expression' + -k 'classical_expression or condition_only or switch_expression or bool_uint_and_float or boolean_expression or cast_to_bool or condition_mutation or narrow_uint or malformed_public_expression' Run the complete Qiskit translation test file after the focused tests pass: @@ -253,7 +318,12 @@ lists must still read condition-only and target-only bits from the containing circuit. A nested condition-only bit must follow the enclosing block's local-to-root permutation. A nested legacy tuple condition on root Clbit one must emit a `cbit.load` at index one even when that bit is local index zero in -the enclosing block. +the enclosing block. Publicly mutating either an expression or tuple condition +must import the current Python operator, target, and comparison value. A packed +register containing `0b10` must cast to true. An unsigned literal outside its +declared width must raise `RuntimeError` rather than aborting the process. A +public expression whose declared result type conflicts with its operator and +operands must fail during preflight. Malformed block-capture lists and variables absent from the containing circuit must fail during validation with a clear runtime error. Existing literal @@ -272,7 +342,7 @@ reader. Do not add a private exporter fallback. ## Artifacts and Notes -The source branch begins at the focused scalar-symbol parent, which already +The focused capture branch is based directly on current `main`, which already includes CBit and symbolic scalar support. Native expression nodes do not carry sufficient public Clbit identity by themselves. `CircuitInstruction.clbits` supplies identity for block operands, while the containing Python circuit @@ -284,10 +354,11 @@ At completion, `ExpressionKind` in `bindings/mlir/qiskit/QiskitTranslation.h` has `ClassicalBit` and `ClassicalRegister` cases. `Expression` has `uint32_t bit` and `Register reg` payloads. `NativeControlFlowReader` in `Qiskit2_5.cpp` owns the full Python instruction, its operation, its containing -circuit, and the root Python circuit. Its expression normalization resolves all -classical leaves and legacy Clbit conditions to root-circuit indices through the -containing-circuit and parent-map path. `QiskitImport.cpp` accepts expression -leaves only through callbacks backed by `loadClassicalBit` and `packRegister`. +circuit, and the root Python circuit. Its Python-authoritative condition and +expression normalization resolves all classical leaves and legacy Clbit or +register conditions to root-circuit indices through the containing-circuit and +parent-map path. `QiskitImport.cpp` accepts expression leaves only through +callbacks backed by `loadClassicalBit` and `packRegister`. This work depends only on Qiskit 2.5's existing native extension table, nanobind's public Python object access, MLIR's arithmetic and structured-control @@ -301,4 +372,6 @@ after the final audit found valid condition-only and target-only bits outside the block-capture list; the plan now records the containing-circuit resolver and nested parent-map regression. Updated it once more after the nested legacy Clbit-condition accessor exposed its containing-circuit index rather than a root -index. +index. Updated it after rebasing onto current `main` and addressing the final +review findings to record Python-authoritative conditions, bounded unsigned +literals, nonzero Boolean casts, and their focused regressions. diff --git a/CHANGELOG.md b/CHANGELOG.md index 5630bd82f5..d3a1f7dd25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ releases may include breaking changes. #### Import and export - ✨ Add Qiskit circuit import and target-aware export to the compiler - collection ([#2031], [#2133], [#2140], [#2150]) ([**@burgholzer**], + collection ([#2031], [#2133], [#2140], [#2150], [#2175]) ([**@burgholzer**], [**@simon1hofmann**]) - ✨ Add conversions between `jeff` and QCO ([#1479], [#1548], [#1565], [#1637], [#1676], [#1706], [#1776], [#1836], [#1934], [#2000], [#2018], [#2105]) @@ -787,6 +787,7 @@ for previous changelogs._ +[#2175]: https://github.com/munich-quantum-toolkit/core/pull/2175 [#2169]: https://github.com/munich-quantum-toolkit/core/pull/2169 [#2168]: https://github.com/munich-quantum-toolkit/core/pull/2168 [#2158]: https://github.com/munich-quantum-toolkit/core/pull/2158 From 125355deaeaee9cf78e5ddbebe62c48523b600f8 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Fri, 21 Aug 2026 15:57:30 +0200 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=90=9B=20Delay=20Qiskit=20control-f?= =?UTF-8?q?low=20handle=20acquisition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acquire the native control-flow handle only after Python object initialization succeeds. Document the shared import test helpers. Assisted-by: GPT-5.6 via Codex Signed-off-by: Simon Hofmann --- bindings/mlir/qiskit/Qiskit2_5.cpp | 8 ++++---- test/python/test_mlir_qiskit_translation.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index 5461c4b6b5..2ae4a10022 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -1047,14 +1047,14 @@ class NativeControlFlowReader final : public ControlFlowReader { nb::object containingPythonCircuit, nb::object rootPythonCircuit) : rootCircuit_(rootCircuit), circuit_(circuit), parent_(parent), - controlFlow_( - qk_circuit_get_control_flow_instruction(circuit, index, parent)), instruction_(std::move(instruction)), operation_(pythonAttribute( instruction_, "operation", "Qiskit circuit instruction has no control-flow operation")), containingPythonCircuit_(std::move(containingPythonCircuit)), - rootPythonCircuit_(std::move(rootPythonCircuit)) { + rootPythonCircuit_(std::move(rootPythonCircuit)), + controlFlow_( + qk_circuit_get_control_flow_instruction(circuit, index, parent)) { if (controlFlow_ == nullptr) { throwPythonError("Qiskit failed to inspect a control-flow instruction"); } @@ -1612,11 +1612,11 @@ class NativeControlFlowReader final : public ControlFlowReader { const QkCircuit* rootCircuit_ = nullptr; const QkCircuit* circuit_ = nullptr; const QkControlFlowInstruction* parent_ = nullptr; - QkControlFlowInstruction* controlFlow_ = nullptr; nb::object instruction_; nb::object operation_; nb::object containingPythonCircuit_; nb::object rootPythonCircuit_; + QkControlFlowInstruction* controlFlow_ = nullptr; }; std::unique_ptr diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index 5fb7e5f668..fbeb253359 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -1027,12 +1027,28 @@ def test_bool_uint_and_float_expressions(condition: expr.Expr, operation: str) - def _round_trip_qiskit_import(circuit: QuantumCircuit) -> str: + """Import a Qiskit circuit and validate its MLIR round trip. + + Args: + circuit: Qiskit circuit to import. + + Returns: + The imported MLIR text. + """ program = QCProgram.from_qiskit(circuit) assert QCProgram.from_mlir_str(program.ir).ir == program.ir return program.ir def _cbit_load_indices(ir: str) -> list[int]: + """Extract the constant indices used by CBit loads. + + Args: + ir: MLIR text to inspect. + + Returns: + The CBit load indices in occurrence order. + """ constants = { name: int(value) for name, value in re.findall(r"(?m)^\s*(%[-\w.$]+) = arith\.constant (\d+) : index$", ir) } From 7b81e7cccd18cb59fa0bcbecd7f1166cb9a5319e Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Fri, 21 Aug 2026 16:16:03 +0200 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=90=9B=20Bound=20Qiskit=20classical?= =?UTF-8?q?=20expression=20size?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track the total number of nodes during Python classical-expression normalization and reject trees larger than 4096 nodes before allocating the excess node. Assisted-by: GPT-5.6 via Codex Signed-off-by: Simon Hofmann --- bindings/mlir/qiskit/Qiskit2_5.cpp | 25 ++++++++++++++------- test/python/test_mlir_qiskit_translation.py | 19 ++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index 2ae4a10022..89b2a414f8 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -72,6 +72,7 @@ namespace nb = nanobind; namespace { constexpr size_t MAX_EXPRESSION_DEPTH = 64U; +constexpr size_t MAX_EXPRESSION_NODES = 4096U; constexpr size_t MAX_ANNOTATED_OPERATION_DEPTH = 64U; [[nodiscard]] nb::object pythonAttribute(const nb::handle object, @@ -1138,7 +1139,8 @@ class NativeControlFlowReader final : public ControlFlowReader { nb::module_::import_("qiskit.circuit.classical.expr"); if (nb::isinstance(condition, expressionModule.attr("Expr"))) { result.kind = ClassicalTargetKind::Expression; - result.expression = normalizePythonExpressionOnly(condition); + size_t nodeCount = 0U; + result.expression = normalizePythonExpressionOnly(condition, nodeCount); return result; } @@ -1283,7 +1285,8 @@ class NativeControlFlowReader final : public ControlFlowReader { if (nb::isinstance(target, expressionModule.attr("Expr"))) { result.kind = ClassicalTargetKind::Expression; // Qiskit 2.5's native switch-target accessors abort for expressions. - result.expression = normalizePythonExpressionOnly(target); + size_t nodeCount = 0U; + result.expression = normalizePythonExpressionOnly(target, nodeCount); return result; } throw std::runtime_error("Qiskit switch has an unknown target type"); @@ -1473,11 +1476,17 @@ class NativeControlFlowReader final : public ControlFlowReader { [[nodiscard]] std::unique_ptr normalizePythonExpressionOnly(const nb::handle pythonExpression, + size_t& nodeCount, const size_t depth = 0U) const { if (depth >= MAX_EXPRESSION_DEPTH) { throw std::runtime_error( "Qiskit classical expressions exceed the nesting limit of 64"); } + if (nodeCount >= MAX_EXPRESSION_NODES) { + throw std::runtime_error( + "Qiskit classical expressions exceed the node limit of 4096"); + } + ++nodeCount; auto result = std::make_unique(); setPythonExpressionType(*result, pythonExpression); const auto className = pythonStringAttribute( @@ -1529,7 +1538,7 @@ class NativeControlFlowReader final : public ControlFlowReader { result->left = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "operand", "Qiskit unary expression has no operand"), - depth + 1U); + nodeCount, depth + 1U); return result; } if (className == "Binary") { @@ -1541,11 +1550,11 @@ class NativeControlFlowReader final : public ControlFlowReader { result->left = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "left", "Qiskit binary expression has no left operand"), - depth + 1U); + nodeCount, depth + 1U); result->right = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "right", "Qiskit binary expression has no right operand"), - depth + 1U); + nodeCount, depth + 1U); return result; } if (className == "Cast") { @@ -1553,7 +1562,7 @@ class NativeControlFlowReader final : public ControlFlowReader { result->left = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "operand", "Qiskit cast expression has no operand"), - depth + 1U); + nodeCount, depth + 1U); return result; } if (className == "Index") { @@ -1561,11 +1570,11 @@ class NativeControlFlowReader final : public ControlFlowReader { result->left = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "target", "Qiskit index expression has no target"), - depth + 1U); + nodeCount, depth + 1U); result->right = normalizePythonExpressionOnly( pythonAttribute(pythonExpression, "index", "Qiskit index expression has no index"), - depth + 1U); + nodeCount, depth + 1U); return result; } if (className == "Stretch") { diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index fbeb253359..bac25d5bf6 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -1307,6 +1307,25 @@ def test_excessively_nested_classical_expression_is_rejected() -> None: QCProgram.from_qiskit(circuit) +def test_oversized_classical_expression_is_rejected() -> None: + """Bound the total size of a balanced classical expression.""" + level = [expr.equal(1, 1) for _ in range(1025)] + while len(level) > 1: + level = [ + expr.logic_or(level[index], level[index + 1]) if index + 1 < len(level) else level[index] + for index in range(0, len(level), 2) + ] + circuit = QuantumCircuit(1) + with circuit.if_test(level[0]): + circuit.x(0) + source_data = list(circuit.data) + + with pytest.raises(RuntimeError, match="expressions exceed the node limit of 4096"): + QCProgram.from_qiskit(circuit) + + assert list(circuit.data) == source_data + + def test_excessively_nested_control_flow_is_rejected() -> None: """Bound control-flow traversal independently of definition depth.""" body = QuantumCircuit(1, 1) From a2f36ac28b54c9ae8bc640bbc55932dc040c1a56 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:23:15 +0000 Subject: [PATCH 06/11] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Share=20Qiskit=20cla?= =?UTF-8?q?ssical=20target=20normalization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- bindings/mlir/qiskit/Qiskit2_5.cpp | 105 +++++++++++++---------------- 1 file changed, 45 insertions(+), 60 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index 89b2a414f8..b508da127e 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -1132,56 +1132,36 @@ class NativeControlFlowReader final : public ControlFlowReader { } [[nodiscard]] ClassicalTarget condition() const override { - ClassicalTarget result; const auto condition = pythonAttribute( operation_, "condition", "Qiskit control flow has no condition"); const auto expressionModule = nb::module_::import_("qiskit.circuit.classical.expr"); if (nb::isinstance(condition, expressionModule.attr("Expr"))) { - result.kind = ClassicalTargetKind::Expression; - size_t nodeCount = 0U; - result.expression = normalizePythonExpressionOnly(condition, nodeCount); - return result; + return normalizePythonTarget(condition); } - const auto tupleType = nb::module_::import_("builtins").attr("tuple"); - if (!nb::isinstance(condition, tupleType) || nb::len(condition) != 2U) { + if (!nb::isinstance(condition) || nb::len(condition) != 2U) { throw std::runtime_error("Qiskit control-flow condition has an invalid " "shape"); } - const nb::handle target = condition[0]; uint64_t expected = 0U; if (!nb::try_cast(condition[1], expected)) { throw std::runtime_error( "Qiskit control-flow condition has an invalid value"); } - const auto circuitModule = nb::module_::import_("qiskit.circuit"); - if (nb::isinstance(target, circuitModule.attr("Clbit"))) { + auto result = normalizePythonTarget(condition[0]); + if (result.kind == ClassicalTargetKind::ClassicalBit) { if (expected > 1U) { throw std::runtime_error( "Qiskit classical-bit condition must compare against zero or one"); } - result.kind = ClassicalTargetKind::ClassicalBit; - result.bit = rootClbitIndex(target); result.expectedBit = expected != 0U; return result; } - if (nb::isinstance(target, circuitModule.attr("ClassicalRegister"))) { - const auto size = nb::len(target); - if (size == 0U || size > 64U) { - throw std::runtime_error( - "Qiskit register conditions require between 1 and 64 bits"); - } - result.kind = ClassicalTargetKind::ClassicalRegister; - result.reg.name = pythonStringAttribute( - target, "name", "Qiskit condition register has no name"); - result.reg.bits.reserve(size); - for (const nb::handle bit : nb::iter(target)) { - result.reg.bits.push_back(rootClbitIndex(bit)); - } + if (result.kind == ClassicalTargetKind::ClassicalRegister) { result.width = static_cast( - std::max(size, std::bit_width(expected))); + std::max(result.reg.bits.size(), std::bit_width(expected))); result.expectedRegister = expected; return result; } @@ -1256,40 +1236,9 @@ class NativeControlFlowReader final : public ControlFlowReader { } [[nodiscard]] ClassicalTarget switchTarget() const override { - ClassicalTarget result; - const auto target = - pythonAttribute(operation_, "target", "Qiskit switch has no target"); - const auto circuitModule = nb::module_::import_("qiskit.circuit"); - if (nb::isinstance(target, circuitModule.attr("Clbit"))) { - result.kind = ClassicalTargetKind::ClassicalBit; - result.bit = rootClbitIndex(target); - return result; - } - if (nb::isinstance(target, circuitModule.attr("ClassicalRegister"))) { - result.kind = ClassicalTargetKind::ClassicalRegister; - result.reg.name = pythonStringAttribute( - target, "name", "Qiskit switch register has no name"); - if (nb::len(target) == 0U || nb::len(target) > 64U) { - throw std::runtime_error( - "Qiskit switch registers must contain between 1 and 64 bits"); - } - result.reg.bits.reserve(nb::len(target)); - for (const nb::handle bit : nb::iter(target)) { - result.reg.bits.push_back(rootClbitIndex(bit)); - } - result.width = static_cast(result.reg.bits.size()); - return result; - } - const auto expressionModule = - nb::module_::import_("qiskit.circuit.classical.expr"); - if (nb::isinstance(target, expressionModule.attr("Expr"))) { - result.kind = ClassicalTargetKind::Expression; - // Qiskit 2.5's native switch-target accessors abort for expressions. - size_t nodeCount = 0U; - result.expression = normalizePythonExpressionOnly(target, nodeCount); - return result; - } - throw std::runtime_error("Qiskit switch has an unknown target type"); + // Qiskit 2.5's native switch-target accessors abort for expressions. + return normalizePythonTarget( + pythonAttribute(operation_, "target", "Qiskit switch has no target")); } [[nodiscard]] std::vector switchCases() const override { @@ -1317,6 +1266,42 @@ class NativeControlFlowReader final : public ControlFlowReader { } private: + [[nodiscard]] ClassicalTarget + normalizePythonTarget(const nb::handle target) const { + ClassicalTarget result; + const auto circuitModule = nb::module_::import_("qiskit.circuit"); + if (nb::isinstance(target, circuitModule.attr("Clbit"))) { + result.kind = ClassicalTargetKind::ClassicalBit; + result.bit = rootClbitIndex(target); + return result; + } + if (nb::isinstance(target, circuitModule.attr("ClassicalRegister"))) { + const auto size = nb::len(target); + if (size == 0U || size > 64U) { + throw std::runtime_error( + "Qiskit classical targets require between 1 and 64 bits"); + } + result.kind = ClassicalTargetKind::ClassicalRegister; + result.reg.name = pythonStringAttribute( + target, "name", "Qiskit classical target register has no name"); + result.reg.bits.reserve(size); + for (const nb::handle bit : nb::iter(target)) { + result.reg.bits.push_back(rootClbitIndex(bit)); + } + result.width = static_cast(size); + return result; + } + const auto expressionModule = + nb::module_::import_("qiskit.circuit.classical.expr"); + if (nb::isinstance(target, expressionModule.attr("Expr"))) { + result.kind = ClassicalTargetKind::Expression; + size_t nodeCount = 0U; + result.expression = normalizePythonExpressionOnly(target, nodeCount); + return result; + } + throw std::runtime_error("Qiskit classical target has an unknown type"); + } + [[nodiscard]] uint32_t rootClbitIndex(const nb::handle bit) const { const auto clbits = pythonAttribute( instruction_, "clbits", From 7b07c3bcf3db1eedd2b8101fc5a8397780d7ab03 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:23:51 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20LLVM=20string?= =?UTF-8?q?=20switches=20for=20Qiskit=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- bindings/mlir/qiskit/Qiskit2_5.cpp | 98 +++++++++++------------------- 1 file changed, 36 insertions(+), 62 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index b508da127e..c2b6e050e4 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -11,6 +11,8 @@ #include "QiskitTranslation.h" #include "mlir/Dialect/QC/Translation/StandardGate.h" +#include + // Qiskit requires its umbrella header before the extension function table. #include #include @@ -1389,74 +1391,46 @@ class NativeControlFlowReader final : public ControlFlowReader { [[nodiscard]] static BinaryOperation pythonBinaryOperation(const std::string_view name) { - if (name == "BIT_AND") { - return BinaryOperation::BitAnd; - } - if (name == "BIT_OR") { - return BinaryOperation::BitOr; - } - if (name == "BIT_XOR") { - return BinaryOperation::BitXor; - } - if (name == "LOGIC_AND") { - return BinaryOperation::LogicAnd; - } - if (name == "LOGIC_OR") { - return BinaryOperation::LogicOr; - } - if (name == "EQUAL") { - return BinaryOperation::Equal; - } - if (name == "NOT_EQUAL") { - return BinaryOperation::NotEqual; - } - if (name == "LESS") { - return BinaryOperation::Less; - } - if (name == "LESS_EQUAL") { - return BinaryOperation::LessEqual; - } - if (name == "GREATER") { - return BinaryOperation::Greater; - } - if (name == "GREATER_EQUAL") { - return BinaryOperation::GreaterEqual; - } - if (name == "SHIFT_LEFT") { - return BinaryOperation::ShiftLeft; - } - if (name == "SHIFT_RIGHT") { - return BinaryOperation::ShiftRight; - } - if (name == "ADD") { - return BinaryOperation::Add; - } - if (name == "SUB") { - return BinaryOperation::Subtract; - } - if (name == "MUL") { - return BinaryOperation::Multiply; - } - if (name == "DIV") { - return BinaryOperation::Divide; + const auto operation = + llvm::StringSwitch>(name) + .Case("BIT_AND", BinaryOperation::BitAnd) + .Case("BIT_OR", BinaryOperation::BitOr) + .Case("BIT_XOR", BinaryOperation::BitXor) + .Case("LOGIC_AND", BinaryOperation::LogicAnd) + .Case("LOGIC_OR", BinaryOperation::LogicOr) + .Case("EQUAL", BinaryOperation::Equal) + .Case("NOT_EQUAL", BinaryOperation::NotEqual) + .Case("LESS", BinaryOperation::Less) + .Case("LESS_EQUAL", BinaryOperation::LessEqual) + .Case("GREATER", BinaryOperation::Greater) + .Case("GREATER_EQUAL", BinaryOperation::GreaterEqual) + .Case("SHIFT_LEFT", BinaryOperation::ShiftLeft) + .Case("SHIFT_RIGHT", BinaryOperation::ShiftRight) + .Case("ADD", BinaryOperation::Add) + .Case("SUB", BinaryOperation::Subtract) + .Case("MUL", BinaryOperation::Multiply) + .Case("DIV", BinaryOperation::Divide) + .Default(std::nullopt); + if (!operation) { + throw std::runtime_error( + "Qiskit expression has an unknown Python binary operation"); } - throw std::runtime_error( - "Qiskit expression has an unknown Python binary operation"); + return *operation; } [[nodiscard]] static UnaryOperation pythonUnaryOperation(const std::string_view name) { - if (name == "BIT_NOT") { - return UnaryOperation::BitNot; - } - if (name == "LOGIC_NOT") { - return UnaryOperation::LogicNot; - } - if (name == "NEGATE") { - return UnaryOperation::Negate; + const auto operation = + llvm::StringSwitch>(name) + .Case("BIT_NOT", UnaryOperation::BitNot) + .Case("LOGIC_NOT", UnaryOperation::LogicNot) + .Case("NEGATE", UnaryOperation::Negate) + .Default(std::nullopt); + if (!operation) { + throw std::runtime_error( + "Qiskit expression has an unknown Python unary operation"); } - throw std::runtime_error( - "Qiskit expression has an unknown Python unary operation"); + return *operation; } [[nodiscard]] std::unique_ptr From 1a5fd2dcedfc25b972ecb3ff185afc81d4abd8e2 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:24:17 +0000 Subject: [PATCH 08/11] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Remove=20duplicate?= =?UTF-8?q?=20Qiskit=20root=20ownership?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- bindings/mlir/qiskit/Qiskit2_5.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/bindings/mlir/qiskit/Qiskit2_5.cpp b/bindings/mlir/qiskit/Qiskit2_5.cpp index c2b6e050e4..c45255ee4c 100644 --- a/bindings/mlir/qiskit/Qiskit2_5.cpp +++ b/bindings/mlir/qiskit/Qiskit2_5.cpp @@ -725,7 +725,6 @@ class NativeCircuitReader final : public CircuitReader { data_(pythonAttribute( circuit, "_data", "expected a Qiskit QuantumCircuit with native CircuitData")), - rootPythonCircuit_(pythonCircuit_), circuit_(qk_circuit_borrow_from_python(data_.ptr())) { if (circuit_ == nullptr) { throwPythonError("Qiskit rejected QuantumCircuit._data"); @@ -735,14 +734,12 @@ class NativeCircuitReader final : public CircuitReader { NativeCircuitReader(nb::object pythonCircuit, const QkCircuit* circuit, const QkCircuit* rootCircuit, - nb::object rootPythonCircuit, const QkControlFlowInstruction* parent) : pythonCircuit_(std::move(pythonCircuit)), data_(pythonAttribute( pythonCircuit_, "_data", "Qiskit control-flow block has no native CircuitData")), - rootPythonCircuit_(std::move(rootPythonCircuit)), circuit_(circuit), - rootCircuit_(rootCircuit), parent_(parent) {} + circuit_(circuit), rootCircuit_(rootCircuit), parent_(parent) {} [[nodiscard]] uint32_t numQubits() const override { return qk_circuit_num_qubits(circuit_); @@ -1035,7 +1032,6 @@ class NativeCircuitReader final : public CircuitReader { nb::object pythonCircuit_; nb::object data_; - nb::object rootPythonCircuit_; const QkCircuit* circuit_ = nullptr; const QkCircuit* rootCircuit_ = circuit_; const QkControlFlowInstruction* parent_ = nullptr; @@ -1047,15 +1043,13 @@ class NativeControlFlowReader final : public ControlFlowReader { const QkCircuit* circuit, const size_t index, const QkControlFlowInstruction* parent, nb::object instruction, - nb::object containingPythonCircuit, - nb::object rootPythonCircuit) + nb::object containingPythonCircuit) : rootCircuit_(rootCircuit), circuit_(circuit), parent_(parent), instruction_(std::move(instruction)), operation_(pythonAttribute( instruction_, "operation", "Qiskit circuit instruction has no control-flow operation")), containingPythonCircuit_(std::move(containingPythonCircuit)), - rootPythonCircuit_(std::move(rootPythonCircuit)), controlFlow_( qk_circuit_get_control_flow_instruction(circuit, index, parent)) { if (controlFlow_ == nullptr) { @@ -1102,7 +1096,7 @@ class NativeControlFlowReader final : public ControlFlowReader { const auto block = nb::borrow(blocks[index]); return std::make_unique( block, qk_control_flow_block_circuit(controlFlow_, index), rootCircuit_, - rootPythonCircuit_, controlFlow_); + controlFlow_); } [[nodiscard]] std::vector qubitMap() const override { @@ -1583,7 +1577,6 @@ class NativeControlFlowReader final : public ControlFlowReader { nb::object instruction_; nb::object operation_; nb::object containingPythonCircuit_; - nb::object rootPythonCircuit_; QkControlFlowInstruction* controlFlow_ = nullptr; }; @@ -1591,7 +1584,7 @@ std::unique_ptr NativeCircuitReader::controlFlow(const size_t index) const { return std::make_unique( rootCircuit_, circuit_, index, parent_, - nb::borrow(data_[index]), pythonCircuit_, rootPythonCircuit_); + nb::borrow(data_[index]), pythonCircuit_); } class NativeCircuitWriter final : public CircuitWriter { From 421f0ec6308f807577b82f1ffa7ea8159ff6cb18 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:26:08 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Pass=20classical=20s?= =?UTF-8?q?tate=20to=20Qiskit=20expression=20lowering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- bindings/mlir/qiskit/QiskitImport.cpp | 151 ++++++++++++-------------- 1 file changed, 72 insertions(+), 79 deletions(-) diff --git a/bindings/mlir/qiskit/QiskitImport.cpp b/bindings/mlir/qiskit/QiskitImport.cpp index 7c3afca930..9a1d395196 100644 --- a/bindings/mlir/qiskit/QiskitImport.cpp +++ b/bindings/mlir/qiskit/QiskitImport.cpp @@ -582,10 +582,56 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { return mlir::arith::TruncIOp::create(builder, target, value).getResult(); } -[[nodiscard]] mlir::Value emitExpression( - mlir::qc::QCProgramBuilder& builder, const Expression& expression, - llvm::function_ref emitClassicalBit, - llvm::function_ref emitClassicalRegister) { +struct ClassicalBitRef { + mlir::Value storage; + int64_t index; +}; + +[[nodiscard]] mlir::Value +loadClassicalBit(mlir::qc::QCProgramBuilder& builder, + const llvm::ArrayRef classicalBits, + const llvm::ArrayRef rootClbitMap, + const uint32_t index) { + if (index >= rootClbitMap.size() || + rootClbitMap[index] >= classicalBits.size()) { + throw std::runtime_error( + "Qiskit control flow references an invalid classical bit"); + } + const auto& bit = classicalBits[rootClbitMap[index]]; + return builder.loadClassicalBit(bit.storage, bit.index); +} + +[[nodiscard]] mlir::Value +packRegister(mlir::qc::QCProgramBuilder& builder, + const llvm::ArrayRef classicalBits, + const llvm::ArrayRef rootClbitMap, const Register& reg) { + if (reg.bits.empty() || reg.bits.size() > 64U) { + throw std::runtime_error( + "Qiskit classical registers must contain between 1 and 64 bits"); + } + const auto width = static_cast(reg.bits.size()); + const auto type = builder.getIntegerType(width); + auto packed = integerConstant(builder, width, 0U); + for (size_t index = 0; index < reg.bits.size(); ++index) { + auto bit = castInteger( + builder, + loadClassicalBit(builder, classicalBits, rootClbitMap, reg.bits[index]), + type); + if (index != 0U) { + bit = mlir::arith::ShLIOp::create(builder, bit, + integerConstant(builder, width, index)) + .getResult(); + } + packed = mlir::arith::OrIOp::create(builder, packed, bit).getResult(); + } + return packed; +} + +[[nodiscard]] mlir::Value +emitExpression(mlir::qc::QCProgramBuilder& builder, + const Expression& expression, + const llvm::ArrayRef classicalBits, + const llvm::ArrayRef rootClbitMap) { const auto resultType = expressionType(builder, expression.type, expression.width); switch (expression.kind) { @@ -600,18 +646,22 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { } break; case ExpressionKind::ClassicalBit: - return emitClassicalBit(expression.bit); + return loadClassicalBit(builder, classicalBits, rootClbitMap, + expression.bit); case ExpressionKind::ClassicalRegister: { const auto target = llvm::dyn_cast(resultType); if (!target) { throw std::runtime_error( "Qiskit classical-register expressions must have Uint type"); } - return castInteger(builder, emitClassicalRegister(expression.reg), target); + return castInteger( + builder, + packRegister(builder, classicalBits, rootClbitMap, expression.reg), + target); } case ExpressionKind::Cast: { - const auto operand = emitExpression( - builder, *expression.left, emitClassicalBit, emitClassicalRegister); + const auto operand = + emitExpression(builder, *expression.left, classicalBits, rootClbitMap); if (operand.getType() == resultType) { return operand; } @@ -646,8 +696,8 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { throw std::runtime_error("unsupported Qiskit classical-expression cast"); } case ExpressionKind::Unary: { - const auto operand = emitExpression( - builder, *expression.left, emitClassicalBit, emitClassicalRegister); + const auto operand = + emitExpression(builder, *expression.left, classicalBits, rootClbitMap); switch (expression.unaryOperation) { case UnaryOperation::BitNot: { const auto type = llvm::dyn_cast(operand.getType()); @@ -686,10 +736,10 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { break; } case ExpressionKind::Binary: { - auto left = emitExpression(builder, *expression.left, emitClassicalBit, - emitClassicalRegister); - auto right = emitExpression(builder, *expression.right, emitClassicalBit, - emitClassicalRegister); + auto left = + emitExpression(builder, *expression.left, classicalBits, rootClbitMap); + auto right = + emitExpression(builder, *expression.right, classicalBits, rootClbitMap); const auto comparison = [&]() -> std::optional { std::optional integerPredicate; std::optional floatPredicate; @@ -795,10 +845,10 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { throw std::runtime_error("unsupported Qiskit classical binary operation"); } case ExpressionKind::Index: { - const auto target = emitExpression(builder, *expression.left, - emitClassicalBit, emitClassicalRegister); - auto index = emitExpression(builder, *expression.right, emitClassicalBit, - emitClassicalRegister); + const auto target = + emitExpression(builder, *expression.left, classicalBits, rootClbitMap); + auto index = + emitExpression(builder, *expression.right, classicalBits, rootClbitMap); const auto targetType = llvm::dyn_cast(target.getType()); if (!targetType) { throw std::runtime_error( @@ -824,51 +874,6 @@ circuitRegisters(const CircuitReader& circuit, const bool quantum) { throw std::runtime_error("unsupported normalized Qiskit expression"); } -struct ClassicalBitRef { - mlir::Value storage; - int64_t index; -}; - -[[nodiscard]] mlir::Value -loadClassicalBit(mlir::qc::QCProgramBuilder& builder, - const llvm::ArrayRef classicalBits, - const llvm::ArrayRef rootClbitMap, - const uint32_t index) { - if (index >= rootClbitMap.size() || - rootClbitMap[index] >= classicalBits.size()) { - throw std::runtime_error( - "Qiskit control flow references an invalid classical bit"); - } - const auto& bit = classicalBits[rootClbitMap[index]]; - return builder.loadClassicalBit(bit.storage, bit.index); -} - -[[nodiscard]] mlir::Value -packRegister(mlir::qc::QCProgramBuilder& builder, - const llvm::ArrayRef classicalBits, - const llvm::ArrayRef rootClbitMap, const Register& reg) { - if (reg.bits.empty() || reg.bits.size() > 64U) { - throw std::runtime_error( - "Qiskit classical registers must contain between 1 and 64 bits"); - } - const auto width = static_cast(reg.bits.size()); - const auto type = builder.getIntegerType(width); - auto packed = integerConstant(builder, width, 0U); - for (size_t index = 0; index < reg.bits.size(); ++index) { - auto bit = castInteger( - builder, - loadClassicalBit(builder, classicalBits, rootClbitMap, reg.bits[index]), - type); - if (index != 0U) { - bit = mlir::arith::ShLIOp::create(builder, bit, - integerConstant(builder, width, index)) - .getResult(); - } - packed = mlir::arith::OrIOp::create(builder, packed, bit).getResult(); - } - return packed; -} - [[nodiscard]] mlir::Value emitCondition(mlir::qc::QCProgramBuilder& builder, const ClassicalTarget& target, @@ -894,14 +899,8 @@ emitCondition(mlir::qc::QCProgramBuilder& builder, .getResult(); } case ClassicalTargetKind::Expression: { - const auto condition = emitExpression( - builder, *target.expression, - [&](const uint32_t bit) { - return loadClassicalBit(builder, classicalBits, rootClbitMap, bit); - }, - [&](const Register& reg) { - return packRegister(builder, classicalBits, rootClbitMap, reg); - }); + const auto condition = emitExpression(builder, *target.expression, + classicalBits, rootClbitMap); if (!condition.getType().isInteger(1)) { throw std::runtime_error( "Qiskit control-flow condition expression must have Boolean type"); @@ -926,14 +925,8 @@ emitSwitchTarget(mlir::qc::QCProgramBuilder& builder, value = packRegister(builder, classicalBits, rootClbitMap, target.reg); break; case ClassicalTargetKind::Expression: - value = emitExpression( - builder, *target.expression, - [&](const uint32_t bit) { - return loadClassicalBit(builder, classicalBits, rootClbitMap, bit); - }, - [&](const Register& reg) { - return packRegister(builder, classicalBits, rootClbitMap, reg); - }); + value = emitExpression(builder, *target.expression, classicalBits, + rootClbitMap); break; } if (!llvm::isa(value.getType())) { From ceb58135a4b34547e4765845fea015f072be0102 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:26:48 +0000 Subject: [PATCH 10/11] =?UTF-8?q?=E2=9C=85=20Limit=20captured-expression?= =?UTF-8?q?=20MLIR=20round=20trips?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- test/python/test_mlir_qiskit_translation.py | 42 ++++++++------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index bac25d5bf6..d9ac7569f8 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -1026,20 +1026,6 @@ def test_bool_uint_and_float_expressions(condition: expr.Expr, operation: str) - assert operation in program.ir -def _round_trip_qiskit_import(circuit: QuantumCircuit) -> str: - """Import a Qiskit circuit and validate its MLIR round trip. - - Args: - circuit: Qiskit circuit to import. - - Returns: - The imported MLIR text. - """ - program = QCProgram.from_qiskit(circuit) - assert QCProgram.from_mlir_str(program.ir).ir == program.ir - return program.ir - - def _cbit_load_indices(ir: str) -> list[int]: """Extract the constant indices used by CBit loads. @@ -1145,14 +1131,14 @@ def test_malformed_public_expression_type_is_rejected() -> None: QCProgram.from_qiskit(circuit) -def test_classical_expression_clbit_captures_round_trip_on_import() -> None: +def test_classical_expression_clbit_captures_import() -> None: """Keep Clbit identity when an expression capture uses a nontrivial order.""" circuit = QuantumCircuit(1, 2) condition = expr.logic_and(circuit.clbits[1], expr.logic_not(circuit.clbits[0])) with circuit.if_test(condition): circuit.x(0) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [1, 0] assert "arith.xori" in ir @@ -1167,7 +1153,9 @@ def test_classical_expression_register_captures_round_trip_on_import() -> None: with circuit.if_test(condition): circuit.x(0) - ir = _round_trip_qiskit_import(circuit) + program = QCProgram.from_qiskit(circuit) + assert QCProgram.from_mlir_str(program.ir).ir == program.ir + ir = program.ir assert _cbit_load_indices(ir) == [0, 1, 2] assert ir.count("arith.shli") == 2 @@ -1175,7 +1163,7 @@ def test_classical_expression_register_captures_round_trip_on_import() -> None: assert "arith.cmpi eq" in ir -def test_nested_classical_expression_captures_round_trip_on_import() -> None: +def test_nested_classical_expression_captures_import() -> None: """Compose nested local capture maps without changing root Clbit identity.""" circuit = QuantumCircuit(1, 3) with circuit.if_test(expr.logic_not(circuit.clbits[2])): @@ -1183,14 +1171,14 @@ def test_nested_classical_expression_captures_round_trip_on_import() -> None: with circuit.while_loop(condition, None, None, None, label=None): circuit.x(0) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [2, 0, 1] assert "scf.if" in ir assert "scf.while" in ir -def test_switch_expression_captures_round_trip_on_import() -> None: +def test_switch_expression_captures_import() -> None: """Read an expression switch target through Qiskit's public Python tree.""" circuit = QuantumCircuit(1, 2) with circuit.switch(expr.bit_xor(circuit.cregs[0], 1), None, None, None, label=None) as case: @@ -1199,14 +1187,14 @@ def test_switch_expression_captures_round_trip_on_import() -> None: with case(case.DEFAULT): circuit.h(0) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [0, 1] assert "arith.xori" in ir assert "scf.index_switch" in ir -def test_condition_only_clbit_expression_round_trips_on_import() -> None: +def test_condition_only_clbit_expression_imports() -> None: """Resolve a condition bit that no control-flow block uses.""" body = QuantumCircuit(1) body.x(0) @@ -1215,14 +1203,14 @@ def test_condition_only_clbit_expression_round_trips_on_import() -> None: assert len(circuit.data[0].clbits) == 0 - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [0] assert "arith.xori" in ir assert "scf.if" in ir -def test_condition_only_switch_expression_round_trips_on_import() -> None: +def test_condition_only_switch_expression_imports() -> None: """Resolve a switch register that no case block uses.""" zero = QuantumCircuit(1) zero.x(0) @@ -1240,7 +1228,7 @@ def test_condition_only_switch_expression_round_trips_on_import() -> None: assert len(circuit.data[0].clbits) == 0 assert all(block.num_clbits == 0 for block in circuit.data[0].operation.blocks) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [0, 1] assert "arith.xori" in ir @@ -1261,7 +1249,7 @@ def test_nested_condition_only_expression_uses_parent_capture_map() -> None: [circuit.clbits[1], circuit.clbits[0]], ) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [0, 1] assert ir.count("scf.if") == 2 @@ -1275,7 +1263,7 @@ def test_nested_legacy_clbit_condition_uses_root_index() -> None: with circuit.if_test((circuit.clbits[1], True)): circuit.x(0) - ir = _round_trip_qiskit_import(circuit) + ir = QCProgram.from_qiskit(circuit).ir assert _cbit_load_indices(ir) == [1] assert "scf.for" in ir From 4701bb6f24bfb807dc060191b32ab98e80ce1945 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Fri, 21 Aug 2026 22:30:07 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=93=9D=20Record=20Qiskit=20import?= =?UTF-8?q?=20simplifications?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5.6 via Codex Signed-off-by: Lukas Burgholzer --- .../qiskit-classical-expression-captures.md | 65 ++++++++++++------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/.agent/plans/qiskit-classical-expression-captures.md b/.agent/plans/qiskit-classical-expression-captures.md index 7a5849a600..0b79230562 100644 --- a/.agent/plans/qiskit-classical-expression-captures.md +++ b/.agent/plans/qiskit-classical-expression-captures.md @@ -77,6 +77,9 @@ Qiskit control-flow operations during export. formatting hooks, and `git diff --check`. - [x] (2026-08-21 13:45Z) Inspect the final diff, create separate gitmoji implementation and documentation commits, and push only PR #2175. +- [x] (2026-08-21 22:29Z) Apply the complexity review findings as five focused + commits, rebuild the binding, pass all 22 focused tests and all 175 Qiskit + translation tests, and pass the complete repository lint session. ## Surprises & Discoveries @@ -155,15 +158,20 @@ Qiskit control-flow operations during export. typing rules. This branch must remain composable with the reviewed scalar slice. Date/Author: 2026-08-19 / Codex. -- Decision: Retain the full Python `CircuitInstruction`, the containing Python - circuit, and the root Python circuit in `NativeControlFlowReader`. Resolve a - classical bit in the containing circuit and compose its local index with the - enclosing native capture map when the circuit is nested. Use the current - native block map only to validate the instruction structure. Apply this rule - to expression leaves, switch targets, and legacy tuple conditions. Rationale: - `CircuitInstruction.clbits` describes block operands only, native condition - indices can remain local, and direct root lookup is ambiguous for nested local - registers. Date/Author: 2026-08-19 / Codex. +- Decision: Initially retain the full Python `CircuitInstruction`, the + containing Python circuit, and the root Python circuit in + `NativeControlFlowReader`. Rationale: `CircuitInstruction.clbits` describes + block operands only, native condition indices can remain local, and direct + root lookup is ambiguous for nested local registers. This decision was + superseded after the final complexity review. Date/Author: 2026-08-19 / Codex. + +- Decision: Retain the Python instruction and containing circuit in each + `NativeControlFlowReader`, but let the top-level `NativeCircuitReader` own the + root circuit for the synchronous traversal. Resolve a classical bit in the + containing circuit and compose its local index with the enclosing native map. + Rationale: The recursive call stack already keeps the root reader alive, so + copying the root Python object through every nested reader adds no lifetime + protection. Date/Author: 2026-08-21 / Codex. - Decision: Parse expression-valued switch targets from the public Python expression tree. Continue to use native metadata for cases and block maps. @@ -214,6 +222,12 @@ repository lint session, pinned Clang and Python formatting, Rumdl, Ruff, `ty`, targeted Clang-Tidy 21.1.1, and `git diff --check` all pass. Export-side writer construction remains deliberately out of scope. +The final complexity pass shared public target normalization, replaced manual +operation lookup with `llvm::StringSwitch`, removed duplicate root ownership and +expression callbacks, and kept one representative MLIR text round trip. The +rebuilt binding passed all 22 focused tests, all 175 Qiskit translation tests, +and the complete repository lint session. + ## Context and Orientation `bindings/mlir/qiskit/QiskitTranslation.h` contains version-neutral normalized @@ -231,8 +245,8 @@ in block-capture order. Its native control-flow object exposes a map from that local order to root-circuit Clbit indices. A condition or switch target can also read a bit that no block uses. The importer therefore retains the containing Python circuit to find the local bit and uses the enclosing native map to reach -its root index when the circuit is nested. The retained root Python circuit owns -the complete object hierarchy while the reader traverses nested blocks. +its root index when the circuit is nested. The top-level reader owns the root +Python circuit throughout the synchronous traversal. The current scalar `Parameter` tree represents numeric gate and loop expressions. It is unrelated to Qiskit's typed classical-expression tree and @@ -255,13 +269,13 @@ malformed captures, duplicate or invalid types, standalone variables, and widths outside the existing 64-bit limit, and reject unsigned literals that do not fit their declared width before MLIR construction. -Then update `bindings/mlir/qiskit/QiskitImport.cpp`. Pass callbacks into the -recursive expression emitter. A bit leaf calls `loadClassicalBit`; a register -leaf calls `packRegister` and extends it to the normalized expression width. -Lower casts to `Bool` as nonzero comparisons instead of integer truncation or -floating-point conversion. Extend preflight validation to check leaf types, bit -bounds, register size, unique register bits, and expression widths before MLIR -construction begins. +Then update `bindings/mlir/qiskit/QiskitImport.cpp`. Pass the classical-bit +state and root map directly into the recursive expression emitter. A bit leaf +calls `loadClassicalBit`; a register leaf calls `packRegister` and extends it to +the normalized expression width. Lower casts to `Bool` as nonzero comparisons +instead of integer truncation or floating-point conversion. Extend preflight +validation to check leaf types, bit bounds, register size, unique register bits, +and expression widths before MLIR construction begins. Finally, add tests to `test/python/test_mlir_qiskit_translation.py`. Cover one captured Clbit expression, one captured register expression, nested control flow @@ -354,11 +368,11 @@ At completion, `ExpressionKind` in `bindings/mlir/qiskit/QiskitTranslation.h` has `ClassicalBit` and `ClassicalRegister` cases. `Expression` has `uint32_t bit` and `Register reg` payloads. `NativeControlFlowReader` in `Qiskit2_5.cpp` owns the full Python instruction, its operation, its containing -circuit, and the root Python circuit. Its Python-authoritative condition and -expression normalization resolves all classical leaves and legacy Clbit or -register conditions to root-circuit indices through the containing-circuit and -parent-map path. `QiskitImport.cpp` accepts expression leaves only through -callbacks backed by `loadClassicalBit` and `packRegister`. +circuit. Its Python-authoritative condition and expression normalization +resolves all classical leaves and legacy Clbit or register conditions to +root-circuit indices through the containing-circuit and parent-map path. +`QiskitImport.cpp` passes the classical-bit state and root map directly into the +expression emitter, which calls `loadClassicalBit` and `packRegister`. This work depends only on Qiskit 2.5's existing native extension table, nanobind's public Python object access, MLIR's arithmetic and structured-control @@ -374,4 +388,7 @@ nested parent-map regression. Updated it once more after the nested legacy Clbit-condition accessor exposed its containing-circuit index rather than a root index. Updated it after rebasing onto current `main` and addressing the final review findings to record Python-authoritative conditions, bounded unsigned -literals, nonzero Boolean casts, and their focused regressions. +literals, nonzero Boolean casts, and their focused regressions. Updated it after +the complexity pass to record the shared target normalization, direct lowering +state, root lifetime ownership, reduced round-trip coverage, and final +validation results.