From 1aab795e3178f2d2a008cc63d77441078dc66405 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Mon, 7 Sep 2026 22:49:11 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20Export=20independently=20sch?= =?UTF-8?q?eduled=20measurements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Permit measurement destination fusion across disjoint recursive effects while accounting for the earlier write in classical snapshot validation. Assisted-by: GPT-5 via Codex --- .agent/plans/export-grouped-measurements.md | 36 ++++ bindings/mlir/qiskit/QiskitExport.cpp | 112 +++++++++--- test/python/test_mlir_qiskit_translation.py | 180 ++++++++++++++++++-- 3 files changed, 295 insertions(+), 33 deletions(-) create mode 100644 .agent/plans/export-grouped-measurements.md diff --git a/.agent/plans/export-grouped-measurements.md b/.agent/plans/export-grouped-measurements.md new file mode 100644 index 0000000000..56f9eb8aa4 --- /dev/null +++ b/.agent/plans/export-grouped-measurements.md @@ -0,0 +1,36 @@ +# Export independently scheduled measurements + +Status: in progress; implementation and focused validation remain. + +## Goal and scope + +Export valid mapped QC programs whose measurement stores are separated by +independent measurements or control flow. This is an exporter change stacked +on #2351's routing fix, not an additional scheduling constraint on the mapper. +The implementation belongs in `bindings/mlir/qiskit/QiskitExport.cpp`, with +semantic regressions in `test/python/test_mlir_qiskit_translation.py`. + +## Decisions + +- Keep the measurement at its original quantum position and fuse its unique, + static destination only when intervening operations cannot access that bit. + CBit registers are non-aliasing; distinct static indices are disjoint. +- Inspect nested effects, while retaining the verified QC unitary contract + for operations with intentionally conservative quantum memory effects. +- Account for the exporter's earlier measurement writes when validating lazy + classical expressions. Memory effects alone do not preserve an SSA read + captured before a measurement and evaluated by a later conditional. +- Do not add scratch classical bits: Qiskit exposes them in the public result. + Overlapping destinations, unknown effects, and unsupported stale snapshots + remain diagnosed rather than silently changing results. +- Benchpress's temporary textual event-order guard cannot prove equivalence + and rejects legal independent scheduling. Retire it only with the tested + Core snapshot and deterministic semantic regressions; retain input-profile + restrictions and validate native export and target compliance separately. + +## Validation + +Run the Qiskit translation tests, required stub generation, repository lint, +and whole-changed-file C++ lint. Rebuild the Python wheel and test all 31 guarded +Benchpress feed-forward profiles plus BV100 through native export, without +restarting the full benchmark suite. These checks are not yet complete. diff --git a/bindings/mlir/qiskit/QiskitExport.cpp b/bindings/mlir/qiskit/QiskitExport.cpp index e36ea6c277..4c0884f80a 100644 --- a/bindings/mlir/qiskit/QiskitExport.cpp +++ b/bindings/mlir/qiskit/QiskitExport.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -1687,12 +1688,34 @@ exportExpression(mlir::Value value, ExportState& state, return result; } +[[nodiscard]] static mlir::cbit::StoreOp +measurementDestination(mlir::qc::MeasureOp measure) { + mlir::cbit::StoreOp destination; + for (auto* user : measure.getResult().getUsers()) { + if (auto store = llvm::dyn_cast(user)) { + if (destination) { + throw std::runtime_error( + "QC measurement has more than one classical destination"); + } + destination = store; + } + } + if (!destination) { + throw std::runtime_error( + "QC measurement is missing a static classical destination"); + } + return destination; +} + /// Index writes in block order, including effects of nested operations. static void indexWrites(mlir::Block& block, ExportState::WriteIndex& index) { index.try_emplace(&block); for (auto& operation : block) { llvm::DenseSet modified; - if (auto store = llvm::dyn_cast(operation)) { + if (auto measure = llvm::dyn_cast(operation)) { + /// Fusion writes the destination at the measurement's position. + modified.insert(measurementDestination(measure).getReg()); + } else if (auto store = llvm::dyn_cast(operation)) { modified.insert(store.getReg()); } else if (auto write = llvm::dyn_cast(operation)) { modified.insert(write.getReg()); @@ -2053,6 +2076,71 @@ static void validateControlFlowDepth(const size_t controlFlowDepth) { } } +[[nodiscard]] static bool +disjointClassicalBit(mlir::Value reg, mlir::Value index, + mlir::cbit::StoreOp destination) { + if (reg != destination.getReg()) { + return true; + } + const auto bit = mlir::getConstantIntValue(index); + const auto destinationBit = mlir::getConstantIntValue(destination.getIndex()); + return bit && destinationBit && *bit != *destinationBit; +} + +[[nodiscard]] static bool +canFuseMeasurementAcross(mlir::Operation& operation, + mlir::cbit::StoreOp destination) { + return !operation + .walk([&](mlir::Operation* candidate) { + // Verified unitary regions cannot access classical memory. + // Their global phase and call effects are deliberately broad. + if (llvm::isa(candidate)) { + return mlir::WalkResult::skip(); + } + if (auto measure = + llvm::dyn_cast(candidate)) { + auto store = measurementDestination(measure); + return disjointClassicalBit(store.getReg(), store.getIndex(), + destination) + ? mlir::WalkResult::advance() + : mlir::WalkResult::interrupt(); + } + if (auto store = + llvm::dyn_cast(candidate)) { + return disjointClassicalBit(store.getReg(), store.getIndex(), + destination) + ? mlir::WalkResult::advance() + : mlir::WalkResult::interrupt(); + } + if (auto load = llvm::dyn_cast(candidate)) { + return disjointClassicalBit(load.getReg(), load.getIndex(), + destination) + ? mlir::WalkResult::advance() + : mlir::WalkResult::interrupt(); + } + if (auto interface = + llvm::dyn_cast( + candidate)) { + llvm::SmallVector + effects; + interface.getEffects(effects); + // CBit registers do not alias. Unknown locations and reads + // as well as writes to the destination block early fusion. + if (llvm::any_of(effects, [&](const auto& effect) { + return !effect.getValue() || + effect.getValue() == destination.getReg(); + })) { + return mlir::WalkResult::interrupt(); + } + } else if (!candidate->hasTrait< + mlir::OpTrait::HasRecursiveMemoryEffects>()) { + return mlir::WalkResult::interrupt(); + } + return mlir::WalkResult::advance(); + }) + .wasInterrupted(); +} + [[nodiscard]] static bool isFusableMeasurementStore(mlir::qc::MeasureOp measure, mlir::cbit::StoreOp store) { if (store.getValue() != measure.getResult() || @@ -2061,11 +2149,7 @@ static void validateControlFlowDepth(const size_t controlFlowDepth) { } for (auto* operation = measure->getNextNode(); operation != store; operation = operation->getNextNode()) { - // Fusion writes the destination at the measurement. Quantum gates and - // resets cannot observe that earlier classical write and stay in place. - if (operation == nullptr || - !llvm::isa(operation)) { + if (operation == nullptr || !canFuseMeasurementAcross(*operation, store)) { return false; } } @@ -2570,21 +2654,7 @@ collectSwitch(mlir::scf::IndexSwitchOp switchOp, ExportedCircuit& containing, continue; } if (auto measure = llvm::dyn_cast(operation)) { - mlir::cbit::StoreOp destination; - for (auto& use : measure.getResult().getUses()) { - if (auto store = - llvm::dyn_cast(use.getOwner())) { - if (destination) { - throw std::runtime_error( - "QC measurement has more than one classical destination"); - } - destination = store; - } - } - if (!destination) { - throw std::runtime_error( - "QC measurement is missing a static classical destination"); - } + auto destination = measurementDestination(measure); const auto info = state.classicalRegisterInfo.find(destination.getReg()); const auto index = mlir::getConstantIntValue(destination.getIndex()); diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index c1c41acbae..cf11ed6e68 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -2594,21 +2594,171 @@ def test_delayed_measurement_store_across_quantum_operations(gate: str, *, via_q assert sample(restored, shots=1, seed=1) == {"1": 1} +@pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) +@pytest.mark.parametrize("case", ["same-register", "distinct-registers", "reverse-stores", "repeated-qubit"]) +def test_delayed_measurement_store_across_measurements(case: str, *, via_qco: bool) -> None: + """Preserve each grouped measurement's destination and quantum ordering.""" + result_type = "!cbit.reg<2>" + registers = '%c = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<2>' + destinations = ["%c[%zero] : !cbit.reg<2>", "%c[%one] : !cbit.reg<2>"] + return_value = "%c : !cbit.reg<2>" + if case == "distinct-registers": + result_type = "(!cbit.reg<1>, !cbit.reg<1>)" + registers = """%c = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<1> + %d = cbit.alloc(#cbit.init) {mqt.register_name = "d"} : !cbit.reg<1>""" + destinations = ["%c[%zero] : !cbit.reg<1>", "%d[%zero] : !cbit.reg<1>"] + return_value = "%c, %d : !cbit.reg<1>, !cbit.reg<1>" + stores = [ + f"cbit.store %{value}, {destination}" + for value, destination in zip(("first", "second"), destinations, strict=True) + ] + if case == "reverse-stores": + stores.reverse() + store_operations = "\n ".join(stores) + second_qubit = 0 if case == "repeated-qubit" else 1 + between = "qc.x %q0 : !qc.qubit" if case == "repeated-qubit" else "" + program = QCProgram.from_mlir_str( + f"""module {{ + func.func @main() -> {result_type} attributes {{mqt.entry_point}} {{ + %q0 = qc.alloc : !qc.qubit + %q1 = qc.alloc : !qc.qubit + {registers} + %zero = arith.constant 0 : index + %one = arith.constant 1 : index + qc.x %q0 : !qc.qubit + %first = qc.measure %q0 : !qc.qubit -> i1 + {between} + %second = qc.measure %q{second_qubit} : !qc.qubit -> i1 + {store_operations} + qc.dealloc %q0 : !qc.qubit + qc.dealloc %q1 : !qc.qubit + return {return_value} + }} +}} +""" + ) + if via_qco: + program = program.to_qco().to_qc() + + restored = program.to_qiskit() + + assert restored.num_clbits == 2 + assert [ + (restored.find_bit(item.qubits[0]).index, restored.find_bit(item.clbits[0]).index) + for item in restored.data + if item.operation.name == "measure" + ] == [(0, 0), (second_qubit, 1)] + assert QCProgram.from_qiskit(restored).to_qco().sample(shots=1, seed=1) == {"01": 1} + + +@pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) +@pytest.mark.parametrize("control", ["quantum", "classical"]) +@pytest.mark.parametrize("enabled", [False, True], ids=["false", "true"]) +def test_delayed_measurement_store_across_independent_control(control: str, *, enabled: bool, via_qco: bool) -> None: + """Cross control regions without changing their condition or adding clbits.""" + result_type = "!cbit.reg<2>" + return_value = "%c : !cbit.reg<2>" + initialization = "qc.x %control : !qc.qubit" if enabled else "" + instruction = """qc.ctrl(%control) targets (%target = %q1) { + qc.x %target : !qc.qubit + qc.yield + } : {!qc.qubit}, {!qc.qubit}""" + if control == "classical": + result_type = "(!cbit.reg<2>, !cbit.reg<1>)" + return_value = "%c, %other : !cbit.reg<2>, !cbit.reg<1>" + initialization += """ + %other = cbit.alloc(#cbit.init) {mqt.register_name = "other"} : !cbit.reg<1> + %condition_bit = qc.measure %control : !qc.qubit -> i1 + cbit.store %condition_bit, %other[%zero] : !cbit.reg<1>""" + instruction = """%condition = cbit.load %other[%zero] : !cbit.reg<1> + scf.if %condition { + qc.x %q1 : !qc.qubit + }""" + program = QCProgram.from_mlir_str( + f"""module {{ + func.func @main() -> {result_type} attributes {{mqt.entry_point}} {{ + %q0 = qc.alloc : !qc.qubit + %q1 = qc.alloc : !qc.qubit + %control = qc.alloc : !qc.qubit + %c = cbit.alloc(#cbit.init) {{mqt.register_name = "c"}} : !cbit.reg<2> + %zero = arith.constant 0 : index + %one = arith.constant 1 : index + {initialization} + qc.x %q0 : !qc.qubit + %first = qc.measure %q0 : !qc.qubit -> i1 + {instruction} + cbit.store %first, %c[%zero] : !cbit.reg<2> + %second = qc.measure %q1 : !qc.qubit -> i1 + cbit.store %second, %c[%one] : !cbit.reg<2> + qc.dealloc %q0 : !qc.qubit + qc.dealloc %q1 : !qc.qubit + qc.dealloc %control : !qc.qubit + return {return_value} + }} +}} +""" + ) + if via_qco: + program = program.to_qco().to_qc() + + restored = program.to_qiskit() + + assert restored.num_clbits == (3 if control == "classical" else 2) + expected = f"{int(enabled)}1" + if control == "classical": + expected = f"{int(enabled)}{expected}" + assert QCProgram.from_qiskit(restored).to_qco().sample(shots=1, seed=1) == {expected: 1} + + +@pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) +def test_grouped_measurements_reject_shared_destination(*, via_qco: bool) -> None: + """Do not silently reverse writes when two measurements share a destination.""" + program = _single_qubit_program( + [ + '%classical = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<1>', + "%zero = arith.constant 0 : index", + "qc.x %q : !qc.qubit", + "%first = qc.measure %q : !qc.qubit -> i1", + "qc.x %q : !qc.qubit", + "%second = qc.measure %q : !qc.qubit -> i1", + "cbit.store %second, %classical[%zero] : !cbit.reg<1>", + "cbit.store %first, %classical[%zero] : !cbit.reg<1>", + ], + returns_classical=True, + ) + if via_qco: + program = program.to_qco().to_qc() + + with pytest.raises(RuntimeError, match="destination must follow the measurement"): + program.to_qiskit() + + @pytest.mark.parametrize( "write", [ "cbit.store %false, %classical[%zero] : !cbit.reg<1>", "cbit.write %false, %classical : i1, !cbit.reg<1>", + """scf.if %true { + %old = cbit.load %classical[%zero] : !cbit.reg<1> + scf.if %old { qc.x %q : !qc.qubit } + }""", + """scf.if %true { + cbit.store %false, %classical[%zero] : !cbit.reg<1> + }""", + """scf.if %true { + cbit.write %false, %classical : i1, !cbit.reg<1> + }""", ], - ids=["bit-store", "register-write"], + ids=["bit-store", "register-write", "nested-load", "nested-store", "nested-write"], ) def test_delayed_measurement_store_rejects_intervening_write(write: str) -> None: - """Do not fuse a measurement across a write that would overwrite its result.""" + """Do not move a measurement's store across accesses to its destination.""" program = _single_qubit_program( [ '%classical = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<1>', "%zero = arith.constant 0 : index", "%false = arith.constant false", + "%true = arith.constant true", "qc.x %q : !qc.qubit", "%measured = qc.measure %q : !qc.qubit -> i1", write, @@ -2620,29 +2770,35 @@ def test_delayed_measurement_store_rejects_intervening_write(write: str) -> None program.to_qiskit() -def test_delayed_measurement_store_is_rejected() -> None: +@pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) +@pytest.mark.parametrize("consume_before_store", [True, False], ids=["before-store", "after-store"]) +def test_delayed_measurement_store_is_rejected(*, consume_before_store: bool, via_qco: bool) -> None: """Reject a delayed write that would change a captured bit snapshot.""" + consumer = "scf.if %old { qc.x %controlled_qubit : !qc.qubit }" program = QCProgram.from_mlir_str( - """module { - func.func @main() -> !cbit.reg<1> attributes {mqt.entry_point} { + f"""module {{ + func.func @main() -> !cbit.reg<1> attributes {{mqt.entry_point}} {{ %measured_qubit = qc.alloc : !qc.qubit %controlled_qubit = qc.alloc : !qc.qubit - %classical = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<1> + %classical = cbit.alloc(#cbit.init) {{mqt.register_name = "c"}} : !cbit.reg<1> %zero = arith.constant 0 : index %old = cbit.load %classical[%zero] : !cbit.reg<1> + qc.x %measured_qubit : !qc.qubit %measured = qc.measure %measured_qubit : !qc.qubit -> i1 - scf.if %old { - qc.x %controlled_qubit : !qc.qubit - } + {consumer if consume_before_store else ""} cbit.store %measured, %classical[%zero] : !cbit.reg<1> + {"" if consume_before_store else consumer} qc.dealloc %measured_qubit : !qc.qubit qc.dealloc %controlled_qubit : !qc.qubit return %classical : !cbit.reg<1> - } -} + }} +}} """ ) - with pytest.raises(RuntimeError, match="destination must follow the measurement"): + if via_qco: + program = program.to_qco().to_qc() + + with pytest.raises(RuntimeError, match="cannot preserve a stale classical snapshot"): program.to_qiskit() From d59e447eaf45c809e6cc3859ec453245d4d5463d Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Mon, 7 Sep 2026 23:03:22 +0200 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9D=20Record=20grouped=20measureme?= =?UTF-8?q?nt=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document the successful native-export matrix and the retained stale-snapshot boundary. Assisted-by: GPT-5 via Codex --- .agent/plans/export-grouped-measurements.md | 53 ++++++++++++++------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/.agent/plans/export-grouped-measurements.md b/.agent/plans/export-grouped-measurements.md index 56f9eb8aa4..5a675a4681 100644 --- a/.agent/plans/export-grouped-measurements.md +++ b/.agent/plans/export-grouped-measurements.md @@ -1,36 +1,57 @@ # Export independently scheduled measurements -Status: in progress; implementation and focused validation remain. +Status: complete. ## Goal and scope Export valid mapped QC programs whose measurement stores are separated by -independent measurements or control flow. This is an exporter change stacked -on #2351's routing fix, not an additional scheduling constraint on the mapper. -The implementation belongs in `bindings/mlir/qiskit/QiskitExport.cpp`, with -semantic regressions in `test/python/test_mlir_qiskit_translation.py`. +independent measurements or control flow. This is an exporter change stacked on -## Decisions +## 2351's routing fix, not an additional scheduling constraint on the mapper. The + +implementation belongs in `bindings/mlir/qiskit/QiskitExport.cpp`, with semantic +regressions in `test/python/test_mlir_qiskit_translation.py`. + +### Decisions - Keep the measurement at its original quantum position and fuse its unique, static destination only when intervening operations cannot access that bit. CBit registers are non-aliasing; distinct static indices are disjoint. -- Inspect nested effects, while retaining the verified QC unitary contract - for operations with intentionally conservative quantum memory effects. +- Inspect nested effects, while retaining the verified QC unitary contract for + operations with intentionally conservative quantum memory effects. - Account for the exporter's earlier measurement writes when validating lazy classical expressions. Memory effects alone do not preserve an SSA read captured before a measurement and evaluated by a later conditional. - Do not add scratch classical bits: Qiskit exposes them in the public result. Overlapping destinations, unknown effects, and unsupported stale snapshots remain diagnosed rather than silently changing results. -- Benchpress's temporary textual event-order guard cannot prove equivalence - and rejects legal independent scheduling. Retire it only with the tested - Core snapshot and deterministic semantic regressions; retain input-profile +- Snapshot checks remain conservative at register granularity. A whole-register + read consumed across an early fused write still needs scalar materialization, + even if only another bit changes. This is outside the benchmark profiles fixed + here; retain a specific diagnostic rather than weakening the check. +- Benchpress's temporary textual event-order guard cannot prove equivalence and + rejects legal independent scheduling. Retire it only with the tested Core + snapshot and deterministic semantic regressions; retain input-profile restrictions and validate native export and target compliance separately. -## Validation +### Validation + +At source revision `7dad9e19e`, a fresh wheel with Qiskit 2.5.0 passes: + +- `pytest test/python/test_mlir_qiskit_translation.py`: 330 tests, including + deterministic QC/QCO native-export round trips and unsafe-fusion rejections. + The preceding build fails 12 of the new positive regressions. +- All 31 guarded Benchpress feed-forward profiles, ten previously enabled + profiles, and BV100: 42 native-export checks, with 4,621 conditionals and + recursive Qiskit basis/connectivity validation. No OpenQASM fallback is used; + BV100 retains exactly 99 measurements and classical bits. +- All 80 Benchpress integration tests, including deterministic output checks and + an explicit rejection regression for unsupported snapshot capture. +- `uvx nox -s stubs`, `uvx nox -s lint`, and `uvx nox -s cpp-lint -- 8936bc2ab`: + no whole-file C++ lint findings. Stub generation changes no public API files. -Run the Qiskit translation tests, required stub generation, repository lint, -and whole-changed-file C++ lint. Rebuild the Python wheel and test all 31 guarded -Benchpress feed-forward profiles plus BV100 through native export, without -restarting the full benchmark suite. These checks are not yet complete. +The Benchpress update pins this PR snapshot and retains input restrictions. Its +additional whole-register snapshot probe is valid mapped IR but remains +unsupported by native export, as it was before this change. The full benchmark +suite has not been restarted; structural condition counts are not a general +semantic equivalence proof. From 1380f16d5881e513088a77c431f20433eff1c455 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Tue, 8 Sep 2026 08:10:05 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Simplify=20measurement?= =?UTF-8?q?=20fusion=20effect=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use TypeSwitch and shared memory-effect handling while preserving static-index disambiguation and the existing exporter limits. Cover disjoint bit accesses and intervening register allocations. Assisted-by: GPT-5 via Codex --- bindings/mlir/qiskit/QiskitExport.cpp | 98 ++++++++++----------- test/python/test_mlir_qiskit_translation.py | 57 ++++++++++++ 2 files changed, 106 insertions(+), 49 deletions(-) diff --git a/bindings/mlir/qiskit/QiskitExport.cpp b/bindings/mlir/qiskit/QiskitExport.cpp index 4c0884f80a..4c51257220 100644 --- a/bindings/mlir/qiskit/QiskitExport.cpp +++ b/bindings/mlir/qiskit/QiskitExport.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -2090,55 +2091,54 @@ disjointClassicalBit(mlir::Value reg, mlir::Value index, [[nodiscard]] static bool canFuseMeasurementAcross(mlir::Operation& operation, mlir::cbit::StoreOp destination) { - return !operation - .walk([&](mlir::Operation* candidate) { - // Verified unitary regions cannot access classical memory. - // Their global phase and call effects are deliberately broad. - if (llvm::isa(candidate)) { - return mlir::WalkResult::skip(); - } - if (auto measure = - llvm::dyn_cast(candidate)) { - auto store = measurementDestination(measure); - return disjointClassicalBit(store.getReg(), store.getIndex(), - destination) - ? mlir::WalkResult::advance() - : mlir::WalkResult::interrupt(); - } - if (auto store = - llvm::dyn_cast(candidate)) { - return disjointClassicalBit(store.getReg(), store.getIndex(), - destination) - ? mlir::WalkResult::advance() - : mlir::WalkResult::interrupt(); - } - if (auto load = llvm::dyn_cast(candidate)) { - return disjointClassicalBit(load.getReg(), load.getIndex(), - destination) - ? mlir::WalkResult::advance() - : mlir::WalkResult::interrupt(); - } - if (auto interface = - llvm::dyn_cast( - candidate)) { - llvm::SmallVector - effects; - interface.getEffects(effects); - // CBit registers do not alias. Unknown locations and reads - // as well as writes to the destination block early fusion. - if (llvm::any_of(effects, [&](const auto& effect) { - return !effect.getValue() || - effect.getValue() == destination.getReg(); - })) { - return mlir::WalkResult::interrupt(); - } - } else if (!candidate->hasTrait< - mlir::OpTrait::HasRecursiveMemoryEffects>()) { - return mlir::WalkResult::interrupt(); - } - return mlir::WalkResult::advance(); - }) - .wasInterrupted(); + const auto result = operation.walk( + [&](mlir::Operation* candidate) { + return llvm::TypeSwitch(candidate) + .Case([](mlir::qc::UnitaryOpInterface) { + // Verified unitary regions cannot access classical + // memory. Their global phase and call effects are + // deliberately broad. + return mlir::WalkResult::skip(); + }) + .Case([&](mlir::qc::MeasureOp measure) { + auto store = measurementDestination(measure); + return disjointClassicalBit(store.getReg(), store.getIndex(), + destination) + ? mlir::WalkResult::advance() + : mlir::WalkResult::interrupt(); + }) + .Case([&](mlir::MemoryEffectOpInterface mem) { + llvm::SmallVector effects; + mem.getEffects(effects); + // CBit registers do not alias. Same-register bit accesses + // still need static-index disambiguation. + const bool conflicts = + llvm::any_of(effects, [&](const auto& effect) { + if (!effect.getValue()) { + return true; + } + if (effect.getValue() != destination.getReg()) { + return false; + } + return llvm::TypeSwitch(candidate) + .Case( + [&](auto access) { + return !disjointClassicalBit(access.getReg(), + access.getIndex(), + destination); + }) + .Default(true); + }); + return conflicts ? mlir::WalkResult::interrupt() + : mlir::WalkResult::advance(); + }) + .Default([](mlir::Operation* op) { + return op->hasTrait() + ? mlir::WalkResult::advance() + : mlir::WalkResult::interrupt(); + }); + }); + return !result.wasInterrupted(); } [[nodiscard]] static bool isFusableMeasurementStore(mlir::qc::MeasureOp measure, diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index cf11ed6e68..20c3c6aa52 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -2710,6 +2710,63 @@ def test_delayed_measurement_store_across_independent_control(control: str, *, e assert QCProgram.from_qiskit(restored).to_qco().sample(shots=1, seed=1) == {expected: 1} +@pytest.mark.parametrize( + ("access", "expected"), + [ + ("%bit = cbit.load %c[%one] : !cbit.reg<2>\n scf.if %bit { qc.x %q : !qc.qubit }", "01"), + ("cbit.store %true, %c[%one] : !cbit.reg<2>", "11"), + ], + ids=["load", "store"], +) +def test_delayed_measurement_store_across_disjoint_bit(access: str, expected: str) -> None: + """Cross accesses to another static bit in the destination register.""" + program = QCProgram.from_mlir_str( + f"""module {{ + func.func @main() -> !cbit.reg<2> attributes {{mqt.entry_point}} {{ + %q = qc.alloc : !qc.qubit + %c = cbit.alloc(#cbit.init) : !cbit.reg<2> + %zero = arith.constant 0 : index + %one = arith.constant 1 : index + %true = arith.constant true + qc.x %q : !qc.qubit + %measured = qc.measure %q : !qc.qubit -> i1 + {access} + cbit.store %measured, %c[%zero] : !cbit.reg<2> + qc.dealloc %q : !qc.qubit + return %c : !cbit.reg<2> + }} +}} +""" + ) + restored = program.to_qiskit() + assert restored.num_clbits == 2 + assert QCProgram.from_qiskit(restored).to_qco().sample(shots=1, seed=1) == {expected: 1} + + +@pytest.mark.parametrize("allocate_destination", [False, True], ids=["other-register", "destination"]) +def test_delayed_measurement_store_across_allocation(*, allocate_destination: bool) -> None: + """Fuse across another allocation, but never across the destination's allocation.""" + allocation = "%classical = cbit.alloc(#cbit.init) : !cbit.reg<1>" + program = _single_qubit_program( + [ + *([] if allocate_destination else [allocation]), + "%zero = arith.constant 0 : index", + "qc.x %q : !qc.qubit", + "%measured = qc.measure %q : !qc.qubit -> i1", + allocation if allocate_destination else "%other = cbit.alloc(#cbit.init) : !cbit.reg<1>", + "cbit.store %measured, %classical[%zero] : !cbit.reg<1>", + ], + returns_classical=True, + ) + if allocate_destination: + with pytest.raises(RuntimeError, match="destination must follow the measurement"): + program.to_qiskit() + else: + restored = program.to_qiskit() + assert restored.num_clbits == 1 + assert QCProgram.from_qiskit(restored).to_qco().sample(shots=1, seed=1) == {"1": 1} + + @pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) def test_grouped_measurements_reject_shared_destination(*, via_qco: bool) -> None: """Do not silently reverse writes when two measurements share a destination.""" From 5ea693d504af9df31624405608645e2cdc5b62de Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Wed, 9 Sep 2026 08:13:16 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Normalize=20measuremen?= =?UTF-8?q?t=20stores=20before=20export?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move safely fusible stores on the existing export clone before indexing writes. Snapshot analysis then uses the emitted order without synthetic measurement writes or special handling of future measurement stores. Preserve late destination indices, measured-bit control, and ordered shared destinations. Keep conflicting accesses and reversed writes rejected, and leave the caller's program unchanged. Assisted-by: Codex --- .agent/plans/export-grouped-measurements.md | 66 +++++++++------ bindings/mlir/qiskit/QiskitExport.cpp | 43 ++++++---- test/python/test_mlir_qiskit_translation.py | 94 ++++++++++++++++++--- 3 files changed, 153 insertions(+), 50 deletions(-) diff --git a/.agent/plans/export-grouped-measurements.md b/.agent/plans/export-grouped-measurements.md index 5a675a4681..de7c877b7a 100644 --- a/.agent/plans/export-grouped-measurements.md +++ b/.agent/plans/export-grouped-measurements.md @@ -1,42 +1,60 @@ # Export independently scheduled measurements -Status: complete. +Status: in progress. Validate measurement-store normalization on the export +clone. ## Goal and scope Export valid mapped QC programs whose measurement stores are separated by -independent measurements or control flow. This is an exporter change stacked on +independent measurements or control flow. This change is stacked on the routing +fix in PR [#2351](https://github.com/munich-quantum-toolkit/core/pull/2351). It +adds no scheduling constraint to the mapper. The implementation belongs in +`bindings/mlir/qiskit/QiskitExport.cpp`, with semantic regressions in +`test/python/test_mlir_qiskit_translation.py`. -## 2351's routing fix, not an additional scheduling constraint on the mapper. The - -implementation belongs in `bindings/mlir/qiskit/QiskitExport.cpp`, with semantic -regressions in `test/python/test_mlir_qiskit_translation.py`. - -### Decisions +## Decisions - Keep the measurement at its original quantum position and fuse its unique, static destination only when intervening operations cannot access that bit. CBit registers are non-aliasing; distinct static indices are disjoint. - Inspect nested effects, while retaining the verified QC unitary contract for operations with intentionally conservative quantum memory effects. -- Account for the exporter's earlier measurement writes when validating lazy - classical expressions. Memory effects alone do not preserve an SSA read - captured before a measurement and evaluated by a later conditional. +- Move each supported store immediately after its measurement on the exporter's + existing clone before indexing writes. Materialize a late constant destination + index before the measurement. Quantum operations retain their order, and the + caller's program stays unchanged. +- Snapshot analysis uses the actual normalized order. Do not maintain synthetic + writes at measurement positions or model other measurements' future stores. + Retain the parent's indexed lookup and scalar snapshot support. - Do not add scratch classical bits: Qiskit exposes them in the public result. - Overlapping destinations, unknown effects, and unsupported stale snapshots - remain diagnosed rather than silently changing results. -- Snapshot checks remain conservative at register granularity. A whole-register - read consumed across an early fused write still needs scalar materialization, - even if only another bit changes. This is outside the benchmark profiles fixed - here; retain a specific diagnostic rather than weakening the check. + Conflicting destination accesses, unknown effects, and unsupported stale + snapshots remain diagnosed rather than silently changing results. +- Snapshot checks remain conservative at register granularity. Reuse the + parent's scalar snapshot support, including reads consumed across a fused + write. Stale register snapshots wider than 64 bits remain unsupported. - Benchpress's temporary textual event-order guard cannot prove equivalence and rejects legal independent scheduling. Retire it only with the tested Core snapshot and deterministic semantic regressions; retain input-profile restrictions and validate native export and target compliance separately. -### Validation +## Validation + +Current validation on parent `aa1b13cf8`, with rebuilt Python 3.13 bindings and +Qiskit 2.5.2: + +- `pytest test/python/test_mlir_qiskit_translation.py test/python/test_mlir_loops.py`: + all 398 tests pass. Added cases cover direct measured-bit control before a + delayed store, disjoint-bit snapshots without redundant scalar variables, late + destination indices, and ordered writes to a shared destination. Reverse + writes and conflicting effects remain rejected. Export leaves the input + program unchanged. +- The nested Qiskit control program that aborted before the parent's fixes now + compiles through the target pipeline. +- `uvx nox -s stubs`: passes without generated API changes. +- `uvx nox -s cpp-lint -- aa1b13cf8`: no whole-file C++ lint findings. +- `uvx nox -s lint`: passes. -At source revision `7dad9e19e`, a fresh wheel with Qiskit 2.5.0 passes: +Historical validation at source revision `7dad9e19e`, with Qiskit 2.5.0: - `pytest test/python/test_mlir_qiskit_translation.py`: 330 tests, including deterministic QC/QCO native-export round trips and unsafe-fusion rejections. @@ -50,8 +68,8 @@ At source revision `7dad9e19e`, a fresh wheel with Qiskit 2.5.0 passes: - `uvx nox -s stubs`, `uvx nox -s lint`, and `uvx nox -s cpp-lint -- 8936bc2ab`: no whole-file C++ lint findings. Stub generation changes no public API files. -The Benchpress update pins this PR snapshot and retains input restrictions. Its -additional whole-register snapshot probe is valid mapped IR but remains -unsupported by native export, as it was before this change. The full benchmark -suite has not been restarted; structural condition counts are not a general -semantic equivalence proof. +The historical Benchpress update pins that snapshot and retains input +restrictions. Its old snapshot-rejection result predates the parent's scalar +snapshot support and has not been revalidated. Those integration checks and the +full benchmark suite have not been rerun for this update; structural condition +counts are not a general semantic equivalence proof. diff --git a/bindings/mlir/qiskit/QiskitExport.cpp b/bindings/mlir/qiskit/QiskitExport.cpp index 4c51257220..586992d52a 100644 --- a/bindings/mlir/qiskit/QiskitExport.cpp +++ b/bindings/mlir/qiskit/QiskitExport.cpp @@ -1713,10 +1713,7 @@ static void indexWrites(mlir::Block& block, ExportState::WriteIndex& index) { index.try_emplace(&block); for (auto& operation : block) { llvm::DenseSet modified; - if (auto measure = llvm::dyn_cast(operation)) { - /// Fusion writes the destination at the measurement's position. - modified.insert(measurementDestination(measure).getReg()); - } else if (auto store = llvm::dyn_cast(operation)) { + if (auto store = llvm::dyn_cast(operation)) { modified.insert(store.getReg()); } else if (auto write = llvm::dyn_cast(operation)) { modified.insert(write.getReg()); @@ -2100,13 +2097,6 @@ canFuseMeasurementAcross(mlir::Operation& operation, // deliberately broad. return mlir::WalkResult::skip(); }) - .Case([&](mlir::qc::MeasureOp measure) { - auto store = measurementDestination(measure); - return disjointClassicalBit(store.getReg(), store.getIndex(), - destination) - ? mlir::WalkResult::advance() - : mlir::WalkResult::interrupt(); - }) .Case([&](mlir::MemoryEffectOpInterface mem) { llvm::SmallVector effects; mem.getEffects(effects); @@ -2156,6 +2146,31 @@ canFuseMeasurementAcross(mlir::Operation& operation, return true; } +/// Put each supported measurement store at its emitted position before +/// snapshot analysis. Quantum operations keep their order. +static void prepareMeasurementStores(mlir::func::FuncOp function) { + function.walk([&](mlir::qc::MeasureOp measure) { + auto destination = measurementDestination(measure); + const auto index = mlir::getConstantIntValue(destination.getIndex()); + if (!index) { + throw std::runtime_error( + "QC measurement uses a dynamic classical destination"); + } + if (!isFusableMeasurementStore(measure, destination)) { + throw std::runtime_error("QC measurement destination must follow the " + "measurement in the same block"); + } + if (auto* definition = destination.getIndex().getDefiningOp(); + definition && definition->getBlock() == measure->getBlock() && + measure->isBeforeInBlock(definition)) { + mlir::OpBuilder builder(measure); + destination.getIndexMutable().assign(mlir::arith::ConstantIndexOp::create( + builder, measure.getLoc(), *index)); + } + destination->moveAfter(measure); + }); +} + [[nodiscard]] static ClassicalVariable declareLocal(mlir::Type type, ExportedCircuit& circuit, ExportState& state) { Expression expression; @@ -2666,11 +2681,6 @@ collectSwitch(mlir::scf::IndexSwitchOp switchOp, ExportedCircuit& containing, throw std::runtime_error( "QC measurement uses a dynamic classical destination"); } - if (!isFusableMeasurementStore(measure, destination)) { - throw std::runtime_error( - "QC measurement destination must follow the measurement in the " - "same block"); - } const auto checked = checkedIndex(*index, "classical-bit"); if (checked >= info->second.size) { throw std::runtime_error( @@ -3005,6 +3015,7 @@ nb::object exportCircuit(const mlir::QCProgram& program, "target qubit count"); } collectResources(function, state, target); + prepareMeasurementStores(function); indexWrites(function.getBody().front(), state.writes); auto circuit = collectBlock(function.getBody().front(), state, 0U); for (const auto& [reg, info] : state.classicalRegisterInfo) { diff --git a/test/python/test_mlir_qiskit_translation.py b/test/python/test_mlir_qiskit_translation.py index 20c3c6aa52..be43030bd2 100644 --- a/test/python/test_mlir_qiskit_translation.py +++ b/test/python/test_mlir_qiskit_translation.py @@ -2594,6 +2594,69 @@ def test_delayed_measurement_store_across_quantum_operations(gate: str, *, via_q assert sample(restored, shots=1, seed=1) == {"1": 1} +@pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) +@pytest.mark.parametrize( + ("intervening", "expected", "variables"), + [ + pytest.param( + "scf.if %first { qc.x %q1 : !qc.qubit }\n cbit.store %first, %c[%zero] : !cbit.reg<2>", + "11", + 1, + id="measurement-result-control", + ), + pytest.param( + "%old = cbit.load %c[%one] : !cbit.reg<2>\n" + " cbit.store %first, %c[%zero] : !cbit.reg<2>\n" + " scf.if %old { qc.x %q1 : !qc.qubit }", + "01", + 0, + id="disjoint-bit-snapshot", + ), + pytest.param( + "%late = arith.constant 0 : index\n cbit.store %first, %c[%late] : !cbit.reg<2>", + "01", + 0, + id="late-destination-index", + ), + ], +) +def test_delayed_measurement_store_preserves_scalar_uses( + intervening: str, expected: str, variables: int, *, via_qco: bool +) -> None: + """Preserve measured and loaded bits without redundant scalar snapshots.""" + program = QCProgram.from_mlir_str( + f"""module {{ + func.func @main() -> !cbit.reg<2> attributes {{mqt.entry_point}} {{ + %q0 = qc.alloc : !qc.qubit + %q1 = qc.alloc : !qc.qubit + %c = cbit.alloc(#cbit.init) : !cbit.reg<2> + %zero = arith.constant 0 : index + %one = arith.constant 1 : index + qc.x %q0 : !qc.qubit + %first = qc.measure %q0 : !qc.qubit -> i1 + {intervening} + %second = qc.measure %q1 : !qc.qubit -> i1 + cbit.store %second, %c[%one] : !cbit.reg<2> + qc.dealloc %q0 : !qc.qubit + qc.dealloc %q1 : !qc.qubit + return %c : !cbit.reg<2> + }} +}} +""" + ) + if via_qco: + program = program.to_qco().to_qc() + original = str(program) + + restored = program.to_qiskit() + + assert str(program) == original + assert restored.num_clbits == 2 + assert restored.num_vars == variables + assert sample(program, shots=1, seed=1) == {expected: 1} + assert sample(QCProgram.from_qiskit(restored), shots=1, seed=1) == {expected: 1} + + @pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) @pytest.mark.parametrize("case", ["same-register", "distinct-registers", "reverse-stores", "repeated-qubit"]) def test_delayed_measurement_store_across_measurements(case: str, *, via_qco: bool) -> None: @@ -2768,8 +2831,10 @@ def test_delayed_measurement_store_across_allocation(*, allocate_destination: bo @pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) -def test_grouped_measurements_reject_shared_destination(*, via_qco: bool) -> None: - """Do not silently reverse writes when two measurements share a destination.""" +@pytest.mark.parametrize("reverse_stores", [False, True], ids=["ordered", "reversed"]) +def test_grouped_measurements_preserve_shared_destination_order(*, via_qco: bool, reverse_stores: bool) -> None: + """Fuse a shared destination only when measurement and store order agree.""" + values = ["second", "first"] if reverse_stores else ["first", "second"] program = _single_qubit_program( [ '%classical = cbit.alloc(#cbit.init) {mqt.register_name = "c"} : !cbit.reg<1>', @@ -2778,16 +2843,21 @@ def test_grouped_measurements_reject_shared_destination(*, via_qco: bool) -> Non "%first = qc.measure %q : !qc.qubit -> i1", "qc.x %q : !qc.qubit", "%second = qc.measure %q : !qc.qubit -> i1", - "cbit.store %second, %classical[%zero] : !cbit.reg<1>", - "cbit.store %first, %classical[%zero] : !cbit.reg<1>", + *(f"cbit.store %{value}, %classical[%zero] : !cbit.reg<1>" for value in values), ], returns_classical=True, ) if via_qco: program = program.to_qco().to_qc() - with pytest.raises(RuntimeError, match="destination must follow the measurement"): - program.to_qiskit() + if reverse_stores: + with pytest.raises(RuntimeError, match="destination must follow the measurement"): + program.to_qiskit() + else: + restored = program.to_qiskit() + assert restored.num_clbits == 1 + assert sample(program, shots=1, seed=1) == {"0": 1} + assert sample(QCProgram.from_qiskit(restored), shots=1, seed=1) == {"0": 1} @pytest.mark.parametrize( @@ -2829,8 +2899,8 @@ def test_delayed_measurement_store_rejects_intervening_write(write: str) -> None @pytest.mark.parametrize("via_qco", [False, True], ids=["qc", "qco"]) @pytest.mark.parametrize("consume_before_store", [True, False], ids=["before-store", "after-store"]) -def test_delayed_measurement_store_is_rejected(*, consume_before_store: bool, via_qco: bool) -> None: - """Reject a delayed write that would change a captured bit snapshot.""" +def test_delayed_measurement_store_preserves_snapshot(*, consume_before_store: bool, via_qco: bool) -> None: + """Capture a bit before its destination is written by a fused measurement.""" consumer = "scf.if %old { qc.x %controlled_qubit : !qc.qubit }" program = QCProgram.from_mlir_str( f"""module {{ @@ -2845,6 +2915,8 @@ def test_delayed_measurement_store_is_rejected(*, consume_before_store: bool, vi {consumer if consume_before_store else ""} cbit.store %measured, %classical[%zero] : !cbit.reg<1> {"" if consume_before_store else consumer} + %final = qc.measure %controlled_qubit : !qc.qubit -> i1 + cbit.store %final, %classical[%zero] : !cbit.reg<1> qc.dealloc %measured_qubit : !qc.qubit qc.dealloc %controlled_qubit : !qc.qubit return %classical : !cbit.reg<1> @@ -2855,8 +2927,10 @@ def test_delayed_measurement_store_is_rejected(*, consume_before_store: bool, vi if via_qco: program = program.to_qco().to_qc() - with pytest.raises(RuntimeError, match="cannot preserve a stale classical snapshot"): - program.to_qiskit() + restored = program.to_qiskit() + assert restored.num_clbits == 1 + assert sample(program, shots=1, seed=1) == {"0": 1} + assert sample(QCProgram.from_qiskit(restored), shots=1, seed=1) == {"0": 1} def test_multi_result_boolean_select_round_trip() -> None: