From 6e8708ca147227efc7d9f3f0436b4fb5bb04fbce Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Thu, 3 Sep 2026 16:30:31 +0200 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=90=9B=20Preserve=20mapped=20feed-f?= =?UTF-8?q?orward=20control?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep routing SWAPs before crossed structured feed-forward, preserve direct measurement destinations, and retain concrete register-effect order during topological repair. Assisted-by: GPT-5.6 Sol via Codex Signed-off-by: Simon Hofmann --- .../preserve-mapped-classical-control.md | 72 +++++ .../QCO/Transforms/Mapping/Mapping.cpp | 122 ++++++-- mlir/lib/Dialect/QCO/Utils/Sorting.cpp | 119 ++++++-- .../QCO/Transforms/Mapping/CMakeLists.txt | 1 + .../QCO/Transforms/Mapping/test_mapping.cpp | 271 +++++++++++++++++- 5 files changed, 539 insertions(+), 46 deletions(-) create mode 100644 .agent/plans/preserve-mapped-classical-control.md diff --git a/.agent/plans/preserve-mapped-classical-control.md b/.agent/plans/preserve-mapped-classical-control.md new file mode 100644 index 0000000000..49981e3c9a --- /dev/null +++ b/.agent/plans/preserve-mapped-classical-control.md @@ -0,0 +1,72 @@ +# Preserve classical control during target mapping + +Status: complete. The implementation and regressions are rebased onto current +`main` and validated through the Core and Benchpress paths. + +## Goal and scope + +Target mapping must preserve measurement-fed classical control while inserting +routing SWAPs and repairing block order. Before this change, routing could +create a backward quantum dependency through later structured control, and the +topological repair could move a classical-register read before the measurement +result was stored. Cleanup could then remove the conditional quantum work. +Routing could also split a measurement from its direct classical destination, +which native Qiskit export rejects. + +The implementation is confined to +`mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp` and +`mlir/lib/Dialect/QCO/Utils/Sorting.cpp`. Five regressions in +`mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp` cover the +routing frontier, mutable-register ordering, static indexed loads, repeated +measurement destinations, and the native measurement-destination contract. No +public API or dependency changes are required. + +## Decisions + +- Track direct, statically indexed CBit loads and stores per register element. + Effects on distinct constant indices do not conflict. Whole-register, + dynamic-index, and indirect effects remain conservative barriers that alias + every element. This per-index refinement removes false cross-index + dependencies without creating a dependency cycle. +- Select the earliest original block-position operation from the topological + sorter's ready set. This retains source order and measurement/store adjacency + whenever dependencies permit it. +- Retain the mapper's existing decrement, insert, and increment protocol. For + the first use of each SWAP endpoint, rewind only when its wire crossed a + `qco.if`, `qco.index_switch`, `scf.for`, or `scf.while` beyond the earliest + unresolved frontier. Rewinding every endpoint breaks valid pure-quantum + routing state. +- When a SWAP consumes a measured qubit, delay it until after the first later + direct memory-write consumer of the measured bit. Stop at structured control + and never move the insertion point earlier than the normal definition anchor. +- Represent register conditions with the current fixed-width model: `cbit.read` + followed by standard `arith` operations. The sorter orders the store before + the read; pure comparisons do not own the memory dependency. + +## Validation + +- The five focused mapping regressions passed 25 consecutive repetitions each: + 125 of 125 test executions. +- The complete mapping unit-test binary passed all 99 tests. +- The nine target-compilation compiler regressions passed. +- A wheel built from this source passed the full affected Benchpress matrix when + combined with the independent target-pipeline inlining from Core PR #2344: all + 31 control cases exported to native Qiskit, preserving all 3,461 conditionals + and satisfying target validation. +- Without that independent inlining, this PR passes 23 of 31 control cases and + preserves 3,422 conditionals. The remaining eight fail before this mapping + logic because reusable quantum functions reach target compilation. +- The BV100 regression exported natively with all 99 measurement destinations + intact. +- Repository lint, Markdown lint, formatting, and whitespace checks passed. The + exact C++ lint session is delegated to PR CI because this machine does not + have the required clang-tidy 23 executable. + +## Outcome + +Routing no longer creates a backward dependency through later structured +control, sorting preserves only potentially aliasing mutable-register effects, +and routing keeps measurements with their direct classical destinations. Core +pull request 2344 remains independently necessary for the eight current-`main` +cases whose reusable quantum functions must be inlined before target +compilation; it is not part of this repair. diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 72ddfb0aa3..c918816612 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -782,8 +783,9 @@ struct MappingPass : impl::MappingPassBase { return newWhileOp; } - /// Return the value whose wire edge crosses a composite in block order. - static Value valueBeforeBoundary(WireIterator iterator, Operation* boundary) { + /// Return an iterator to the wire edge crossing a boundary in block order. + static WireIterator iteratorBeforeBoundary(WireIterator iterator, + Operation* boundary) { assert(boundary != nullptr && boundary->getBlock() != nullptr); // Independent wires can advance beyond `boundary`. Rewind to the qubit @@ -808,7 +810,47 @@ struct MappingPass : impl::MappingPassBase { assert((consumer == boundary || boundary->isBeforeInBlock(consumer) || isa(consumer)) && "selected qubit value does not cross composite boundary"); - return value; + return iterator; + } + + /// Rewind only when a wire has advanced through structured classical + /// control beyond an earlier routing frontier. + static WireIterator iteratorBeforeCrossedControl(WireIterator iterator, + Operation* boundary) { + WireIterator insertionPoint = iterator; + while (iterator != std::default_sentinel && + iterator.operation() != nullptr && + !iterator.operation()->isBeforeInBlock(boundary)) { + Operation* operation = iterator.operation(); + assert(operation->getBlock() == boundary->getBlock()); + --iterator; + if (isa(operation)) { + insertionPoint = iterator; + } + } + return insertionPoint; + } + + /// Return the first direct memory-write destination of a measured qubit, + /// unless doing so would move routing across structured control. + static Operation* measurementDestination(Value qubit) { + auto measurement = qubit.getDefiningOp(); + if (!measurement) { + return nullptr; + } + + for (Operation* operation = measurement->getNextNode(); + operation != nullptr; operation = operation->getNextNode()) { + if (isa(operation)) { + return nullptr; + } + if (llvm::is_contained(operation->getOperands(), + measurement.getResult()) && + hasEffect(operation)) { + return operation; + } + } + return nullptr; } /// Execute `ntrials` many (parallel) initial layout refinement trials and @@ -1143,11 +1185,15 @@ struct MappingPass : impl::MappingPassBase { /// Insert SWAP operations, exchanging two qubits, virtually /// (`RoutingMode::Cold`) or into the IR (`RoutingMode::Hot`). The function - /// expects that each wire points at the correct insertion point. + /// expects that each wire points at the correct insertion point. In hot mode, + /// `boundary` prevents a touched wire from crossing earlier classical + /// control. template static void insertSWAPs(ArrayRef swaps, RoutingBundle& bundle, - Statistics& stats, IRRewriter* rewriter) { + Statistics& stats, IRRewriter* rewriter, + Operation* boundary = nullptr) { auto& [wires, infos, layout] = bundle; + DenseSet adjusted; for (const auto& [hw0, hw1] : swaps) { const auto [prog0, prog1] = layout.getProgramIndices(hw0, hw1); @@ -1159,11 +1205,36 @@ struct MappingPass : impl::MappingPassBase { auto& w0 = wires[i0]; auto& w1 = wires[i1]; - + if (boundary != nullptr) { + for (const auto& [index, wire] : + {std::pair{i0, &w0}, std::pair{i1, &w1}}) { + if (adjusted.insert(index).second) { + *wire = iteratorBeforeCrossedControl(*wire, boundary); + } + } + } auto in0 = w0.qubit(); auto in1 = w1.qubit(); - rewriter->setInsertionPointAfterValue(in0); // Valid bc. Hot => Forward. + Operation* destination = nullptr; + for (Value input : {in0, in1}) { + Operation* candidate = measurementDestination(input); + if (candidate != nullptr && + (destination == nullptr || + destination->isBeforeInBlock(candidate))) { + destination = candidate; + } + } + Operation* in0Definition = in0.getDefiningOp(); + if (destination != nullptr && + (in0Definition == nullptr || + (in0Definition->getBlock() == destination->getBlock() && + in0Definition->isBeforeInBlock(destination)))) { + rewriter->setInsertionPointAfter(destination); + } else { + // Valid because hot routing only runs in the forward direction. + rewriter->setInsertionPointAfterValue(in0); + } auto swapOp = SWAPOp::create(*rewriter, in0.getLoc(), in0, in1); auto out0 = swapOp.getQubit0Out(); @@ -1333,7 +1404,7 @@ struct MappingPass : impl::MappingPassBase { allIndices, [&](const size_t i) { return !included.contains(i); })); const SmallVector addons(map_range(excluded, [&](const size_t i) { - return valueBeforeBoundary(parent.wires[i], composite.op); + return iteratorBeforeBoundary(parent.wires[i], composite.op).qubit(); })); composite = CompositeUnitary{ @@ -1706,26 +1777,31 @@ struct MappingPass : impl::MappingPassBase { if constexpr (Mode == RoutingMode::Hot) { - // At this point the wire iterators point to sink-like operations - // (e.g. SinkOp, YieldOp), measurements, or two-qubit gate of the - // subsequent layer. Decrementing once ensures that the wire iterators - // point at the input qubits of those operations. + // Remember the earliest unresolved operation before moving each wire + // to its usual SWAP insertion point. If an endpoint has advanced + // through structured classical control beyond that frontier, rewind + // just that endpoint before inserting its first SWAP. + Operation* boundary = nullptr; + for (WireIterator& wire : wires) { + Operation* operation = wire.operation(); + assert(operation != nullptr && + "expected an operation at the frontier"); + assert((boundary == nullptr || + operation->getBlock() == boundary->getBlock()) && + "expected a single-block routing frontier"); + if (boundary == nullptr || operation->isBeforeInBlock(boundary)) { + boundary = operation; + } + } + assert(boundary != nullptr && "expected a non-empty routing frontier"); for_each(wires, [](auto& it) { std::ranges::advance(it, -1); }); - } - - insertSWAPs(*swaps, bundle, stats, rewriter); - - if constexpr (Mode == RoutingMode::Hot) { - // After SWAP insertion, a wire is either untouched by the SWAP - // insertion or pointing at a SWAP operation. If the former is the - // case, incrementing the wire iterator will undo the previous - // decrement, leaving it at the same position as before the SWAP - // insertion. Otherwise, an increment will move the iterator past the - // inserted SWAP operation. + insertSWAPs(*swaps, bundle, stats, rewriter, boundary); for_each(wires, [](auto& it) { std::ranges::advance(it, 1); }); + } else { + insertSWAPs(*swaps, bundle, stats, rewriter); } } diff --git a/mlir/lib/Dialect/QCO/Utils/Sorting.cpp b/mlir/lib/Dialect/QCO/Utils/Sorting.cpp index 63141e993a..d3ee3cdae0 100644 --- a/mlir/lib/Dialect/QCO/Utils/Sorting.cpp +++ b/mlir/lib/Dialect/QCO/Utils/Sorting.cpp @@ -10,16 +10,46 @@ #include "mlir/Dialect/QCO/Utils/Sorting.h" +#include "mlir/Dialect/CBit/IR/CBitOps.h" + #include +#include #include #include +#include #include #include #include +#include #include +#include +#include + using namespace mlir; +namespace { +struct RegisterEffects { + Operation* latestRegisterBarrier = nullptr; + DenseMap latestIndexedEffects; +}; +} // namespace + +/// Return the static index of a direct CBit element access. Dynamic and +/// indirect accesses conservatively alias the complete register. +static std::optional getDirectCBitIndex(Operation* operation, + Value reg) { + if (auto load = dyn_cast(operation); + load && load.getReg() == reg) { + return getConstantIntValue(load.getIndex()); + } + if (auto store = dyn_cast(operation); + store && store.getReg() == reg) { + return getConstantIntValue(store.getIndex()); + } + return std::nullopt; +} + /// Find the nearest neighbour in a given block. static Operation* findParentInBlock(Operation* op, Block& block) { Operation* parent = op->getParentOp(); @@ -42,23 +72,34 @@ void reorderTopologically(Block& block, IRRewriter& rewriter) { // Construct unresolved map: The dependencies of each operation. DenseMap inDegree; + DenseMap blockOrder; DenseMap> successors; DenseMap> predecessors; + const auto addDependency = [&](Operation* predecessor, Operation* successor) { + if (predecessor == successor || + !predecessors[successor].insert(predecessor).second) { + return; + } + ++inDegree[successor]; + successors[predecessor].insert(successor); + }; + + DenseMap registerEffects; + for (Operation& op : block) { // Collect the in-block dependencies of the current operation. - auto& succs = successors[&op]; - auto& pres = predecessors[&op]; + successors.try_emplace(&op); + predecessors.try_emplace(&op); inDegree.try_emplace(&op, 0); + blockOrder.try_emplace(&op, blockOrder.size()); for (Value v : op.getOperands()) { Operation* def = v.getDefiningOp(); - if (def != nullptr && v.getParentBlock() == &block && - !pres.contains(def)) { - pres.insert(def); - ++inDegree[&op]; + if (def != nullptr && v.getParentBlock() == &block) { + addDependency(def, &op); } } @@ -69,50 +110,86 @@ void reorderTopologically(Block& block, IRRewriter& rewriter) { for (Operation* user : op.getUsers()) { if (user->getBlock() == &block) { - if (!succs.contains(user)) { - succs.insert(user); - } + addDependency(&op, user); continue; } if (Operation* parent = findParentInBlock(user, block); parent != nullptr) { + addDependency(&op, parent); + } + } + + // SSA use-def chains do not capture ordering constraints on mutable CBit + // registers. Preserve the original order of effects that may alias the + // same register element. Direct, statically indexed accesses to distinct + // elements do not alias; whole-register and dynamic accesses + // conservatively alias every element. Value-less effects cannot safely + // impose block-order dependencies here: routing may temporarily require + // those operations to move while repairing SSA order. + const auto effects = getEffectsRecursively(&op); + if (!effects) { + continue; + } - auto& parentPre = predecessors[parent]; - if (!parentPre.contains(&op)) { - parentPre.insert(&op); - ++inDegree[parent]; + llvm::SmallDenseSet affectedValues; + for (const auto& effect : *effects) { + Value value = effect.getValue(); + if (!value || !affectedValues.insert(value).second) { + continue; + } + if (isa(value.getType())) { + auto& state = registerEffects[value]; + if (const auto index = getDirectCBitIndex(&op, value)) { + if (state.latestRegisterBarrier != nullptr) { + addDependency(state.latestRegisterBarrier, &op); + } + if (Operation* previous = state.latestIndexedEffects.lookup(*index)) { + addDependency(previous, &op); + } + state.latestIndexedEffects[*index] = &op; + continue; } - if (!succs.contains(parent)) { - succs.insert(parent); + if (state.latestRegisterBarrier != nullptr) { + addDependency(state.latestRegisterBarrier, &op); + } + for (const auto& indexed : state.latestIndexedEffects) { + addDependency(indexed.second, &op); } + state.latestIndexedEffects.clear(); + state.latestRegisterBarrier = &op; } } } assert((inDegree.size() == range_size(block))); - SmallVector worklist; - worklist.reserve(range_size(block)); + const auto laterInBlock = [&blockOrder](Operation* lhs, Operation* rhs) { + return blockOrder.lookup(lhs) > blockOrder.lookup(rhs); + }; + llvm::PriorityQueue, + decltype(laterInBlock)> + worklist(laterInBlock); for (Operation& op : block) { if (inDegree.lookup(&op) == 0) { - worklist.emplace_back(&op); + worklist.push(&op); } } Block* newBlock = rewriter.createBlock(&block, block.getArgumentTypes(), getArgumentLocs(block)); - for (size_t cursor = 0; cursor < worklist.size(); ++cursor) { - Operation* ready = worklist[cursor]; + while (!worklist.empty()) { + Operation* ready = worklist.top(); + worklist.pop(); rewriter.moveOpBefore(ready, newBlock, newBlock->end()); for (Operation* user : successors[ready]) { inDegree[user]--; if (inDegree[user] == 0) { - worklist.push_back(user); + worklist.push(user); } } } diff --git a/mlir/unittests/Dialect/QCO/Transforms/Mapping/CMakeLists.txt b/mlir/unittests/Dialect/QCO/Transforms/Mapping/CMakeLists.txt index 49b90f6ac3..05f8c1c765 100644 --- a/mlir/unittests/Dialect/QCO/Transforms/Mapping/CMakeLists.txt +++ b/mlir/unittests/Dialect/QCO/Transforms/Mapping/CMakeLists.txt @@ -12,6 +12,7 @@ add_executable(${target_name} test_mapping.cpp) target_link_libraries( ${target_name} PRIVATE GTest::gtest_main + MLIRCBitDialect MLIRParser MLIRMQTDialect MQTCompilerTarget diff --git a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp index dbd4786c9c..f89e14ce81 100644 --- a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp +++ b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp @@ -9,6 +9,8 @@ */ #include "mlir/Compiler/Target.h" +#include "mlir/Dialect/CBit/IR/CBitDialect.h" +#include "mlir/Dialect/CBit/IR/CBitOps.h" #include "mlir/Dialect/MQT/IR/MQTDialect.h" #include "mlir/Dialect/QCO/Builder/QCOProgramBuilder.h" #include "mlir/Dialect/QCO/IR/QCODialect.h" @@ -16,6 +18,7 @@ #include "mlir/Dialect/QCO/IR/QCOOps.h" #include "mlir/Dialect/QCO/Transforms/Mapping/Mapping.h" #include "mlir/Dialect/QCO/Transforms/Passes.h" +#include "mlir/Dialect/QCO/Utils/Sorting.h" #include "mlir/Dialect/QTensor/IR/QTensorDialect.h" #include "mlir/Dialect/QTensor/IR/QTensorOps.h" #include "mlir/Support/Passes.h" @@ -304,8 +307,9 @@ class MappingPassFixture : public testing::Test { protected: void SetUp() override { DialectRegistry registry; - registry.insert(); + registry.insert(); context = std::make_unique(); context->appendDialectRegistry(registry); context->loadAllAvailableDialects(); @@ -430,6 +434,269 @@ TEST_F(MappingPassFixture, EXPECT_TRUE(isa(*measurement.getQubitOut().getUsers().begin())); } +TEST_F(MappingPassFixture, RouteBeforeLaterClassicalControl) { + const auto target = llvm::cantFail(CompilerTarget::create( + 4, Connectivity::fromCouplings({{0, 1}, {0, 2}, {0, 3}}), + NativeOperations::unrestricted())); + + QCOProgramBuilder builder(context.get()); + builder.initialize(); + + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + auto ancilla = builder.allocQubit(); + + std::tie(q0, ancilla) = builder.cx(q0, ancilla); + std::tie(q1, ancilla) = builder.cx(q1, ancilla); + std::tie(q0, q2) = builder.cx(q0, q2); + + Value condition; + std::tie(ancilla, condition) = builder.measure(ancilla); + q0 = builder.qcoIf(condition, q0, + [&](Value qubit) { return builder.x(qubit); }); + + builder.sink(q0); + builder.sink(q1); + builder.sink(q2); + builder.sink(ancilla); + + auto moduleOp = builder.finalize(); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(runPass(moduleOp.get(), target, + MappingPassOptions{.ntrials = 4, .seed = 42}) + .succeeded()); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(moduleOp.get()), target)); + + IfOp conditional; + size_t numControlledGates = 0; + moduleOp->walk([&](IfOp candidate) { conditional = candidate; }); + ASSERT_TRUE(conditional); + + size_t numSwaps = 0; + size_t numSwapsBeforeControl = 0; + moduleOp->walk([&](SWAPOp swap) { + ++numSwaps; + if (swap->getBlock() == conditional->getBlock() && + swap->isBeforeInBlock(conditional)) { + ++numSwapsBeforeControl; + } + }); + conditional->walk([&](XOp) { ++numControlledGates; }); + EXPECT_GT(numSwaps, 0); + EXPECT_EQ(numSwapsBeforeControl, numSwaps); + EXPECT_EQ(numControlledGates, 1); +} + +TEST_F(MappingPassFixture, KeepMeasurementDestinationAdjacentDuringRouting) { + const auto target = llvm::cantFail(CompilerTarget::create( + 4, Connectivity::fromCouplings({{0, 1}, {1, 2}, {2, 3}}), + NativeOperations::unrestricted())); + + QCOProgramBuilder builder(context.get()); + builder.initialize(); + + auto reg = builder.allocClassicalBitRegister(1); + SmallVector qubits(4); + for (Value& qubit : qubits) { + qubit = builder.allocQubit(); + } + + std::tie(qubits[1], qubits[2]) = builder.cx(qubits[1], qubits[2]); + std::tie(qubits[3], qubits[2]) = builder.cx(qubits[3], qubits[2]); + std::tie(qubits[1], std::ignore) = builder.measure(qubits[1], reg, 0); + std::tie(qubits[3], qubits[1]) = builder.cx(qubits[3], qubits[1]); + + for (Value qubit : qubits) { + builder.sink(qubit); + } + + auto moduleOp = builder.finalize(); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE( + runPass(moduleOp.get(), target, MappingPassOptions{}).succeeded()); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(moduleOp.get()), target)); + + cbit::StoreOp store; + moduleOp->walk([&](cbit::StoreOp candidate) { store = candidate; }); + ASSERT_TRUE(store); + auto measurement = store.getValue().getDefiningOp(); + ASSERT_TRUE(measurement); + EXPECT_EQ(store->getPrevNode(), measurement.getOperation()); + + size_t numSwaps = 0; + moduleOp->walk([&](SWAPOp) { ++numSwaps; }); + EXPECT_GT(numSwaps, 0); +} + +TEST_F(MappingPassFixture, KeepMeasurementStoresOrderedDuringRepair) { + for (int64_t secondIndex : {int64_t{1}, int64_t{0}}) { + SCOPED_TRACE(secondIndex); + + QCOProgramBuilder builder(context.get()); + builder.initialize(); + + auto reg = builder.allocClassicalBitRegister(2); + Value index0 = arith::ConstantIndexOp::create(builder, 0); + Value index1 = arith::ConstantIndexOp::create(builder, secondIndex); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto delayedDefinition = builder.x(q0); + + Value bit0; + std::tie(q0, bit0) = builder.measure(delayedDefinition); + builder.storeClassicalBit(bit0, reg, index0); + + Value bit1; + std::tie(q1, bit1) = builder.measure(q1); + builder.storeClassicalBit(bit1, reg, index1); + builder.sink(q0); + builder.sink(q1); + + auto moduleOp = builder.finalize(); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + + SmallVector stores; + moduleOp->walk([&](cbit::StoreOp store) { stores.push_back(store); }); + ASSERT_EQ(stores.size(), 2); + + delayedDefinition.getDefiningOp()->moveAfter(stores.back()); + + auto entryPoint = getEntryPoint(moduleOp.get()); + IRRewriter rewriter(context.get()); + reorderTopologically(entryPoint.getFunctionBody().front(), rewriter); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + + if (secondIndex == 0) { + EXPECT_TRUE(stores.front()->isBeforeInBlock(stores.back())); + continue; + } + for (cbit::StoreOp store : stores) { + auto measurement = store.getValue().getDefiningOp(); + ASSERT_TRUE(measurement); + EXPECT_EQ(store->getPrevNode(), measurement.getOperation()); + } + } +} + +TEST_F(MappingPassFixture, KeepRegisterBarrierBeforeIndexedLoadDuringRepair) { + QCOProgramBuilder builder(context.get()); + builder.initialize(); + + auto reg = builder.allocClassicalBitRegister(2); + Value index = arith::ConstantIndexOp::create(builder, 0); + Value value = arith::ConstantIntOp::create(builder, 0, 2); + auto write = cbit::WriteOp::create(builder, value, reg); + Value loaded = builder.loadClassicalBit(reg, index); + auto load = loaded.getDefiningOp(); + + auto moduleOp = builder.finalize(); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + + value.getDefiningOp()->moveAfter(load); + + auto entryPoint = getEntryPoint(moduleOp.get()); + IRRewriter rewriter(context.get()); + reorderTopologically(entryPoint.getFunctionBody().front(), rewriter); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(write->isBeforeInBlock(load)); +} + +TEST_F(MappingPassFixture, PreserveStoredRegisterControlDuringRouting) { + constexpr StringLiteral source = R"mlir( + module { + func.func @main() attributes {mqt.entry_point} { + %c0 = arith.constant 0 : index + %reg = cbit.alloc(#cbit.init) : !cbit.reg<1> + %q0 = qco.alloc : !qco.qubit + %q1 = qco.alloc : !qco.qubit + %q2 = qco.alloc : !qco.qubit + %measured, %bit = qco.measure %q0 : !qco.qubit + cbit.store %bit, %reg[%c0] : !cbit.reg<1> + // Keep an independent ready chain between the store and read to expose + // source-order instability during topological repair. + %one = arith.constant 1 : i64 + %two = arith.addi %one, %one : i64 + %snapshot = cbit.read %reg : !cbit.reg<1> -> i1 + %expected = arith.constant 1 : i1 + %condition = arith.cmpi eq, %snapshot, %expected : i1 + %controlled1 = qco.if %condition args(%arg = %q1) -> (!qco.qubit) { + %flipped = qco.x %arg : !qco.qubit -> !qco.qubit + qco.yield %flipped : !qco.qubit + } else args(%arg = %q1) { + qco.yield %arg : !qco.qubit + } + %next1, %next2 = qco.swap %controlled1, %q2 + : !qco.qubit, !qco.qubit -> !qco.qubit, !qco.qubit + qco.sink %measured : !qco.qubit + qco.sink %next1 : !qco.qubit + qco.sink %next2 : !qco.qubit + return + } + } + )mlir"; + + auto moduleOp = parseSourceString(source, context.get()); + ASSERT_TRUE(moduleOp); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + + const auto target = llvm::cantFail( + CompilerTarget::create(3, Connectivity::fromCouplings({{0, 1}, {1, 2}}), + NativeOperations::unrestricted())); + PassManager mappingPm(context.get()); + mappingPm.addPass( + createMappingPass(target, MappingPassOptions{.ntrials = 1})); + ASSERT_TRUE(succeeded(mappingPm.run(moduleOp.get()))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(moduleOp.get()), target)); + + cbit::StoreOp mappedStore; + cbit::ReadOp mappedSnapshot; + arith::CmpIOp mappedComparison; + IfOp mappedConditional; + moduleOp->walk([&](cbit::StoreOp candidate) { mappedStore = candidate; }); + moduleOp->walk([&](cbit::ReadOp candidate) { mappedSnapshot = candidate; }); + moduleOp->walk( + [&](arith::CmpIOp candidate) { mappedComparison = candidate; }); + moduleOp->walk([&](IfOp candidate) { mappedConditional = candidate; }); + ASSERT_TRUE(mappedStore); + ASSERT_TRUE(mappedSnapshot); + ASSERT_TRUE(mappedComparison); + ASSERT_TRUE(mappedConditional); + auto mappedMeasurement = mappedStore.getValue().getDefiningOp(); + ASSERT_TRUE(mappedMeasurement); + EXPECT_EQ(mappedStore->getPrevNode(), mappedMeasurement.getOperation()); + EXPECT_EQ(mappedStore.getReg(), mappedSnapshot.getReg()); + EXPECT_TRUE(mappedStore->isBeforeInBlock(mappedSnapshot)); + EXPECT_EQ(mappedSnapshot.getResult(), mappedComparison.getLhs()); + EXPECT_EQ(mappedComparison.getPredicate(), arith::CmpIPredicate::eq); + EXPECT_EQ(mappedComparison.getResult(), mappedConditional.getCondition()); + + PassManager cleanupPm(context.get()); + cleanupPm.addPass(createCanonicalizerPass()); + ASSERT_TRUE(succeeded(cleanupPm.run(moduleOp.get()))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + + cbit::StoreOp store; + cbit::ReadOp snapshot; + IfOp conditional; + size_t numControlledGates = 0; + moduleOp->walk([&](cbit::StoreOp candidate) { store = candidate; }); + moduleOp->walk([&](cbit::ReadOp candidate) { snapshot = candidate; }); + moduleOp->walk([&](IfOp candidate) { conditional = candidate; }); + moduleOp->walk([&](XOp) { ++numControlledGates; }); + + ASSERT_TRUE(store); + ASSERT_TRUE(snapshot); + ASSERT_TRUE(conditional); + EXPECT_EQ(store.getReg(), snapshot.getReg()); + EXPECT_TRUE(store->isBeforeInBlock(snapshot)); + EXPECT_EQ(snapshot.getResult(), conditional.getCondition()); + EXPECT_EQ(numControlledGates, 1); +} + TEST_F(MappingPassFixture, PreserveNoncontiguousTargetSiteIds) { constexpr int64_t size = 3; From 8936bc2ab28ffc35cf8ade169e0779e04fac62f7 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Mon, 7 Sep 2026 22:18:42 +0200 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=93=9D=20Record=20mapped=20control?= =?UTF-8?q?=20validation=20and=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: GPT-5 via Codex --- .../preserve-mapped-classical-control.md | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/.agent/plans/preserve-mapped-classical-control.md b/.agent/plans/preserve-mapped-classical-control.md index cf0544349f..2ec4c4ac34 100644 --- a/.agent/plans/preserve-mapped-classical-control.md +++ b/.agent/plans/preserve-mapped-classical-control.md @@ -1,6 +1,7 @@ # Preserve classical control during target mapping -Status: validation in progress on main `3f801880a`. +Status: complete on main `3f801880a`. The routing cycle is fixed; separate +Benchpress integration and exporter gaps remain. ## Goal and scope @@ -30,11 +31,25 @@ whole-register write followed by an indexed load during dominance repair. - Leave the existing composite-boundary helper unchanged. No public API or dependency changes are required. -## Validation and remaining work +## Validation -- Confirm that the isolated routing regression still fails on main, then passes - with the routing fix. -- Run the mapping, QCO utility, and compiler tests, plus repeated focused cases. -- Recheck the known Benchpress gaps with the refreshed branch. Do not restart - the full corpus suite. -- Run repository lint and the whole-changed-file C++ lint session before push. +- The isolated routing regression aborts at the sorter's cyclic-dependency + assertion on main. With this fix, it passes 25 consecutive repetitions. +- All 96 mapping, 185 QCO utility, and 167 compiler tests pass. +- A fresh Python wheel passes all 306 Qiskit translation tests and the two + one-qubit synthesis regressions. All 68 Benchpress integration tests pass. +- Repository lint and whole-changed-file C++ lint pass without findings. + +## Remaining Benchpress gaps + +The constrained feed-forward matrix is not fully enabled by this change. Six of +31 guarded profiles pass unchanged; 25 stop at the integration's strict textual +event-order check. The check rejects reordered independent events, so these +failures alone do not establish a semantic regression. The small deterministic +feed-forward counterexample now preserves its measured result. + +BV100 still fails native Qiskit export when mapping groups measurements before +their stores. The exporter supports intervening quantum operations, but not +another measurement. This restriction belongs in the exporter, not SWAP +placement. Keep the integration guards and export fallback until their separate +contracts are resolved; no full Benchpress corpus result is claimed here. From a4c571289a1dea34ec96147a0f1e9e72ea2de0de Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 8 Sep 2026 09:46:56 +0200 Subject: [PATCH 03/11] Apply trivial fix --- .../QCO/Transforms/Mapping/Mapping.cpp | 79 +++++-------------- 1 file changed, 21 insertions(+), 58 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index d3c5d2dbff..832c7f8266 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -11,6 +11,7 @@ #include "mlir/Dialect/QCO/Transforms/Mapping/Mapping.h" #include "mlir/Compiler/Target.h" +#include "mlir/Dialect/CBit/IR/CBitOps.h" #include "mlir/Dialect/MQT/IR/MQTDialect.h" #include "mlir/Dialect/QCO/IR/QCODialect.h" #include "mlir/Dialect/QCO/IR/QCOInterfaces.h" @@ -28,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -811,24 +811,6 @@ struct MappingPass : impl::MappingPassBase { return value; } - /// Rewind only when a wire has advanced through structured classical - /// control beyond an earlier routing frontier. - static WireIterator iteratorBeforeCrossedControl(WireIterator iterator, - Operation* boundary) { - WireIterator insertionPoint = iterator; - while (iterator != std::default_sentinel && - iterator.operation() != nullptr && - !iterator.operation()->isBeforeInBlock(boundary)) { - Operation* operation = iterator.operation(); - assert(operation->getBlock() == boundary->getBlock()); - --iterator; - if (isa(operation)) { - insertionPoint = iterator; - } - } - return insertionPoint; - } - /// Execute `ntrials` many (parallel) initial layout refinement trials and /// return the heuristically best one. /// @@ -1161,15 +1143,11 @@ struct MappingPass : impl::MappingPassBase { /// Insert SWAP operations, exchanging two qubits, virtually /// (`RoutingMode::Cold`) or into the IR (`RoutingMode::Hot`). The function - /// expects that each wire points at the correct insertion point. In hot mode, - /// `boundary` prevents a touched wire from crossing earlier classical - /// control. + /// expects that each wire points at the correct insertion point. template static void insertSWAPs(ArrayRef swaps, RoutingBundle& bundle, - Statistics& stats, IRRewriter* rewriter, - Operation* boundary = nullptr) { + Statistics& stats, IRRewriter* rewriter) { auto& [wires, infos, layout] = bundle; - DenseSet adjusted; for (const auto& [hw0, hw1] : swaps) { const auto [prog0, prog1] = layout.getProgramIndices(hw0, hw1); @@ -1181,14 +1159,7 @@ struct MappingPass : impl::MappingPassBase { auto& w0 = wires[i0]; auto& w1 = wires[i1]; - if (boundary != nullptr) { - for (const auto& [index, wire] : - {std::pair{i0, &w0}, std::pair{i1, &w1}}) { - if (adjusted.insert(index).second) { - *wire = iteratorBeforeCrossedControl(*wire, boundary); - } - } - } + auto in0 = w0.qubit(); auto in1 = w1.qubit(); @@ -1270,7 +1241,7 @@ struct MappingPass : impl::MappingPassBase { getForwardSlice(bit, &slice); return any_of(slice, [](Operation* op) { return isa(op); + UnitaryOpInterface, cbit::StoreOp>(op); }); }) .template Case( @@ -1280,9 +1251,6 @@ struct MappingPass : impl::MappingPassBase { [](auto&) { return Direction == WireDirection::Backward; }) .template Case( [&](auto&) { - if (indices.size() == 1) { - return true; - } if (visited.insert(op).second) { composites.emplace_back(op, indices); } @@ -1735,31 +1703,26 @@ struct MappingPass : impl::MappingPassBase { if constexpr (Mode == RoutingMode::Hot) { - // Remember the earliest unresolved operation before moving each wire - // to its usual SWAP insertion point. If an endpoint has advanced - // through structured classical control beyond that frontier, rewind - // just that endpoint before inserting its first SWAP. - Operation* boundary = nullptr; - for (WireIterator& wire : wires) { - Operation* operation = wire.operation(); - assert(operation != nullptr && - "expected an operation at the frontier"); - assert((boundary == nullptr || - operation->getBlock() == boundary->getBlock()) && - "expected a single-block routing frontier"); - if (boundary == nullptr || operation->isBeforeInBlock(boundary)) { - boundary = operation; - } - } - assert(boundary != nullptr && "expected a non-empty routing frontier"); + // At this point the wire iterators point to sink-like operations + // (e.g. SinkOp, YieldOp), measurements, or two-qubit gate of the + // subsequent layer. Decrementing once ensures that the wire iterators + // point at the input qubits of those operations. for_each(wires, [](auto& it) { std::ranges::advance(it, -1); }); + } + + insertSWAPs(*swaps, bundle, stats, rewriter); + + if constexpr (Mode == RoutingMode::Hot) { - insertSWAPs(*swaps, bundle, stats, rewriter, boundary); + // After SWAP insertion, a wire is either untouched by the SWAP + // insertion or pointing at a SWAP operation. If the former is the + // case, incrementing the wire iterator will undo the previous + // decrement, leaving it at the same position as before the SWAP + // insertion. Otherwise, an increment will move the iterator past the + // inserted SWAP operation. for_each(wires, [](auto& it) { std::ranges::advance(it, 1); }); - } else { - insertSWAPs(*swaps, bundle, stats, rewriter); } } @@ -1780,4 +1743,4 @@ std::unique_ptr createMappingPass(const CompilerTarget& target, return std::make_unique(target, options); } -} // namespace mlir::qco +} // namespace mlir::qco \ No newline at end of file From b31f07aaf83b3eb3949ac08df747bad02be62384 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:15:35 +0000 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=8E=A8=20pre-commit=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 832c7f8266..e221c85cd7 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -1743,4 +1743,4 @@ std::unique_ptr createMappingPass(const CompilerTarget& target, return std::make_unique(target, options); } -} // namespace mlir::qco \ No newline at end of file +} // namespace mlir::qco From 05fe7f2007228d79ef4e31ea98c578b1981f29f1 Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 8 Sep 2026 10:53:44 +0200 Subject: [PATCH 05/11] Fix base profile requirements --- .../QCO/Transforms/Mapping/Mapping.cpp | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 832c7f8266..a4c50f0630 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -1232,16 +1232,41 @@ struct MappingPass : impl::MappingPassBase { Value bit = m.getResult(); assert(qubit.hasOneUse()); - Operation* user = *qubit.user_begin(); - if (!isa(user)) { + if (!isa(*qubit.user_begin())) { return true; } + // Verify side-effect dependencies: Does an operation exist + // which reads this value after write? If so, this is an + // adaptive-profile program. + + if (bit.hasOneUse()) { + if (auto store = + dyn_cast(*bit.user_begin())) { + return any_of( + store.getReg().getUsers(), [&](Operation* op) { + if (op == store || + op->getBlock() != store->getBlock() || + !store->isBeforeInBlock(op)) { + return false; + } + return TypeSwitch(op) + .Case( + [&](auto ls) { + return ls.getIndex() == store.getIndex(); + }) + .template Case( + [](auto) { return true; }) + .Default([](Operation*) { return false; }); + }); + } + } + SetVector slice; getForwardSlice(bit, &slice); return any_of(slice, [](Operation* op) { return isa(op); + UnitaryOpInterface>(op); }); }) .template Case( From b6a733265cb22d60ce03b1d4c9c7d8cb978b63ab Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 8 Sep 2026 15:33:08 +0200 Subject: [PATCH 06/11] Add defer helper lambda --- .../QCO/Transforms/Mapping/Mapping.cpp | 89 ++++++++----------- 1 file changed, 36 insertions(+), 53 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index ba3b855e4e..db627af3f2 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -782,35 +782,6 @@ struct MappingPass : impl::MappingPassBase { return newWhileOp; } - /// Return the value whose wire edge crosses a composite in block order. - static Value valueBeforeBoundary(WireIterator iterator, Operation* boundary) { - assert(boundary != nullptr && boundary->getBlock() != nullptr); - - // Independent wires can advance beyond `boundary`. Rewind to the qubit - // value that crosses it so extending the composite does not move later - // operations before the boundary. - if (iterator == std::default_sentinel) { - --iterator; - } - - while (iterator.operation() != nullptr && - !iterator.operation()->isBeforeInBlock(boundary)) { - assert(iterator.operation()->getBlock() == boundary->getBlock()); - --iterator; - } - - Value value = iterator.qubit(); - assert(value && "expected a qubit value before the composite boundary"); - assert(value.hasOneUse() && "expected linear qubit use at boundary"); - Operation* consumer = boundary->getBlock()->findAncestorOpInBlock( - *value.use_begin()->getOwner()); - assert(consumer != nullptr && "expected consumer in boundary block"); - assert((consumer == boundary || boundary->isBeforeInBlock(consumer) || - isa(consumer)) && - "selected qubit value does not cross composite boundary"); - return value; - } - /// Execute `ntrials` many (parallel) initial layout refinement trials and /// return the heuristically best one. /// @@ -1196,6 +1167,36 @@ struct MappingPass : impl::MappingPassBase { DenseSet visited; SmallVector composites; + // The walkProgramGraph driver currently only respects quantum semantics; it + // does not follow classical def-use (side-effect) chains (TODO!). + // Consequently, whenever an operation using non-qubit values is ready there + // may still be classical dependencies. As of now, the easiest solution is + // to defer such a candidate operation until all wires either point at a + // sink (backward: allocs), the candidate itself, or any operation + // after (backward: before) the candidate (from an IR perspective). Hence, + // this function returns true if any of these cases is not fulfilled. + + const auto defer = [&wires](Operation* candidate) { + return any_of(wires, [&](WireIterator& it) { + assert(it != std::default_sentinel); + + Operation* op = it.operation(); + if (op == nullptr || op == candidate) { + return false; + } + + if (isa(op)) { + return false; + } + + if constexpr (Direction == WireDirection::Forward) { + return op->isBeforeInBlock(candidate); + } + + return candidate->isBeforeInBlock(op); + }); + }; + // Advance wires past all executable gates and push composite unitaries // and the respective wire indices of their inputs onto the vector. @@ -1275,8 +1276,8 @@ struct MappingPass : impl::MappingPassBase { scf::YieldOp, scf::ConditionOp>( [](auto&) { return Direction == WireDirection::Backward; }) .template Case( - [&](auto&) { - if (visited.insert(op).second) { + [&](auto& cf) { + if (!defer(cf) && visited.insert(op).second) { composites.emplace_back(op, indices); } return false; @@ -1305,26 +1306,6 @@ struct MappingPass : impl::MappingPassBase { return lhs.op->isBeforeInBlock(rhs.op); }); - // Defer a composite while another active wire points to an operation that - // precedes it in the traversal direction. Otherwise, dispatch would remove - // that operation from the routing frontier. - llvm::erase_if(composites, [&](const CompositeUnitary& composite) { - return llvm::any_of(wires, [&](const WireIterator& iterator) { - if (iterator == std::default_sentinel) { - return false; - } - Operation* operation = iterator.operation(); - if (operation == nullptr || operation == composite.op) { - return false; - } - assert(operation->getBlock() == composite.op->getBlock()); - if constexpr (Direction == WireDirection::Forward) { - return operation->isBeforeInBlock(composite.op); - } - return composite.op->isBeforeInBlock(operation); - }); - }); - return composites; } @@ -1355,7 +1336,10 @@ struct MappingPass : impl::MappingPassBase { allIndices, [&](const size_t i) { return !included.contains(i); })); const SmallVector addons(map_range(excluded, [&](const size_t i) { - return valueBeforeBoundary(parent.wires[i], composite.op); + // Make sure the qubits point to an already processed operation. + const auto& it = std::prev( + parent.wires[i], parent.wires[i] == std::default_sentinel ? 2 : 1); + return it.qubit(); })); composite = CompositeUnitary{ @@ -1684,7 +1668,6 @@ struct MappingPass : impl::MappingPassBase { auto& [wires, infos, layout] = bundle; Statistics stats; - while (true) { while (true) { auto composites = advance(wires, infos, layout); From 6b8632ea30b23f88ffbbff6186e82a5b68fd1ea7 Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 8 Sep 2026 15:37:43 +0200 Subject: [PATCH 07/11] Accept overwrites --- mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index db627af3f2..e8ce768ed8 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -1175,7 +1175,7 @@ struct MappingPass : impl::MappingPassBase { // sink (backward: allocs), the candidate itself, or any operation // after (backward: before) the candidate (from an IR perspective). Hence, // this function returns true if any of these cases is not fulfilled. - + const auto defer = [&wires](Operation* candidate) { return any_of(wires, [&](WireIterator& it) { assert(it != std::default_sentinel); @@ -1252,11 +1252,10 @@ struct MappingPass : impl::MappingPassBase { return false; } return TypeSwitch(op) - .Case( - [&](auto ls) { - return ls.getIndex() == store.getIndex(); - }) - .template Case( + .Case([&](auto ls) { + return ls.getIndex() == store.getIndex(); + }) + .template Case( [](auto) { return true; }) .Default([](Operation*) { return false; }); }); From 96af9c840c27edf1e23d1f04ed70e37df43bd9e0 Mon Sep 17 00:00:00 2001 From: matthias Date: Tue, 8 Sep 2026 16:00:31 +0200 Subject: [PATCH 08/11] Fix lint --- mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index e8ce768ed8..12ec7951ce 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -1252,11 +1252,11 @@ struct MappingPass : impl::MappingPassBase { return false; } return TypeSwitch(op) - .Case([&](auto ls) { + .Case([&](cbit::LoadOp ls) { return ls.getIndex() == store.getIndex(); }) .template Case( - [](auto) { return true; }) + [](cbit::ReadOp) { return true; }) .Default([](Operation*) { return false; }); }); } From f1a5090ab79aca62b7162ac6f7c252be26ac9b14 Mon Sep 17 00:00:00 2001 From: Simon Hofmann Date: Tue, 8 Sep 2026 17:41:55 +0200 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=90=9B=20Fix=20measurement-dependen?= =?UTF-8?q?t=20routing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow SSA uses and register-effect order before advancing measurements. Keep output-only measurements terminal and preserve routing progress. Assisted-by: Codex --- .../preserve-mapped-classical-control.md | 50 +++--- .../QCO/Transforms/Mapping/Mapping.cpp | 153 +++++++++++------- .../QCO/Transforms/Mapping/test_mapping.cpp | 149 ++++++++++++++++- 3 files changed, 260 insertions(+), 92 deletions(-) diff --git a/.agent/plans/preserve-mapped-classical-control.md b/.agent/plans/preserve-mapped-classical-control.md index 2ec4c4ac34..cbb83b8de2 100644 --- a/.agent/plans/preserve-mapped-classical-control.md +++ b/.agent/plans/preserve-mapped-classical-control.md @@ -1,7 +1,6 @@ # Preserve classical control during target mapping -Status: complete on main `3f801880a`. The routing cycle is fixed; separate -Benchpress integration and exporter gaps remain. +Status: complete. The routing regressions and required local checks pass. ## Goal and scope @@ -23,33 +22,26 @@ whole-register write followed by an indexed load during dominance repair. repair a cycle introduced by routing. - Do not require measurement/store adjacency in the mapper. Native Qiskit export supports intervening quantum operations since #2439. -- Retain the mapper's decrement, insert, and increment protocol. Before the - first use of each SWAP endpoint, rewind its wire only if it crossed a - `qco.if`, `qco.index_switch`, `scf.for`, or `scf.while` beyond the earliest - unresolved frontier. Rewinding every endpoint changes valid pure-quantum - routing state. -- Leave the existing composite-boundary helper unchanged. No public API or - dependency changes are required. +- Defer composites while an earlier wire operation still needs routing. Terminal + sinks and output-only measurements must not block independent quantum work. +- Use one measurement classification for advancement and composite deferral. + Follow all SSA result uses and the sorter's whole-register effect order until + reaching quantum work. Output-only loads and overwrites remain terminal; + register accesses inside quantum composites still impose ordering. +- Keep consecutive measurements in scope so an earlier measurement does not hide + a later result used for quantum control. Reuse LLVM slice analysis and a + bounded worklist; no public API or dependency changes are required. ## Validation -- The isolated routing regression aborts at the sorter's cyclic-dependency - assertion on main. With this fix, it passes 25 consecutive repetitions. -- All 96 mapping, 185 QCO utility, and 167 compiler tests pass. -- A fresh Python wheel passes all 306 Qiskit translation tests and the two - one-qubit synthesis regressions. All 68 Benchpress integration tests pass. -- Repository lint and whole-changed-file C++ lint pass without findings. - -## Remaining Benchpress gaps - -The constrained feed-forward matrix is not fully enabled by this change. Six of -31 guarded profiles pass unchanged; 25 stop at the integration's strict textual -event-order check. The check rejects reordered independent events, so these -failures alone do not establish a semantic regression. The small deterministic -feed-forward counterexample now preserves its measured result. - -BV100 still fails native Qiskit export when mapping groups measurements before -their stores. The exporter supports intervening quantum operations, but not -another measurement. This restriction belongs in the exporter, not SWAP -placement. Keep the integration guards and export fallback until their separate -contracts are resolved; no full Benchpress corpus result is claimed here. +Build the release mapping, QCO utility, and compiler unit-test targets. The +mapping binary passes all 100 tests, the QCO utility binary passes all 187, and +the compiler binary passes all 171. The focused mapping tests cover terminal +measurements and sinks before independent control, consecutive measurements, +multiple result users, output-only register reads and overwrites, and register +writes inside a quantum conditional. + +Run `uvx nox -s lint` and `uvx nox -s cpp-lint -- ` for the change. +The repository hooks pass. Full-file C++ lint passes for all three C++ files in +the PR, using main base `b75b02fa9`. These are local checks, not hosted CI or a +full Benchpress corpus run. diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 12ec7951ce..fbee459f73 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -1155,26 +1157,98 @@ struct MappingPass : impl::MappingPassBase { stats.nswaps += swaps.size(); } - /// Advance past all executable gates and return operations with nested - /// regions and the respective wire indices. Stops when no more executable - /// gates are found. The function positions each wire on a non-executable - /// two-qubit gate or a composite unitary, if possible. The function never - /// advances past sink-like operation and thus, each wire will never reach the - /// sentinel state. + /// Return whether quantum work follows a measurement through its wire, SSA + /// results, or the register-effect order preserved by the topological sorter. + static bool measurementNeedsRouting(MeasureOp measurement) { + SetVector worklist; + const auto addSlice = [&](Operation* root) { + SetVector slice; + ForwardSliceOptions options; + options.inclusive = true; + options.filter = [&](Operation* op) { return !worklist.contains(op); }; + getForwardSlice(root, &slice, options); + worklist.insert(slice.begin(), slice.end()); + }; + + // A later measurement on this wire can feed quantum work even when the + // first result is only returned to the caller. + for (auto current = measurement;;) { + auto qubit = current.getQubitOut(); + assert(qubit.hasOneUse()); + Operation* next = *qubit.getUsers().begin(); + if (!isa(next)) { + return true; + } + for (Operation* user : current.getResult().getUsers()) { + addSlice(user); + } + if (isa(next)) { + break; + } + current = cast(next); + } + + Block* block = measurement->getBlock(); + DenseMap firstEffect; + for (size_t index = 0; index < worklist.size(); ++index) { + Operation* op = worklist[index]; + if (isa(op) || + (isa(op) && + any_of(op->getResultTypes(), + [](Type type) { return isa(type); }))) { + return true; + } + + // Captures and nested register accesses also constrain their enclosing + // composite, whose placement threads every physical wire through it. + Operation* ancestor = block->findAncestorOpInBlock(*op); + if (ancestor != op) { + if (ancestor != nullptr) { + addSlice(ancestor); + } + continue; + } + + const auto effects = getEffectsRecursively(op); + if (!effects) { + continue; + } + for (const auto& effect : *effects) { + auto reg = effect.getValue(); + if (!reg || !isa(reg.getType())) { + continue; + } + auto [it, inserted] = firstEffect.try_emplace(reg, op); + if (!inserted && !op->isBeforeInBlock(it->second)) { + continue; + } + it->second = op; + + // Match the sorter's whole-register effect order, including overwrites. + // Output-only reads and writes do not require quantum work. + for (Operation* user : reg.getUsers()) { + Operation* next = block->findAncestorOpInBlock(*user); + if (next != nullptr && next != op && op->isBeforeInBlock(next)) { + addSlice(next); + } + } + } + } + return false; + } + + /// Advance past executable gates and return ready composite operations. + /// Leave wires at non-executable gates, composites, terminal measurements, + /// or sink-like operations, never at the sentinel. template SmallVector advance(Wires& wires, const WireInfos& infos, const Layout& layout) { DenseSet visited; SmallVector composites; - // The walkProgramGraph driver currently only respects quantum semantics; it - // does not follow classical def-use (side-effect) chains (TODO!). - // Consequently, whenever an operation using non-qubit values is ready there - // may still be classical dependencies. As of now, the easiest solution is - // to defer such a candidate operation until all wires either point at a - // sink (backward: allocs), the candidate itself, or any operation - // after (backward: before) the candidate (from an IR perspective). Hence, - // this function returns true if any of these cases is not fulfilled. + // The wire traversal does not follow classical dependencies. Defer a + // composite until earlier routing work is complete, but let independent + // composites pass terminal wires. Reverse block order for backward routing. const auto defer = [&wires](Operation* candidate) { return any_of(wires, [&](WireIterator& it) { @@ -1188,8 +1262,11 @@ struct MappingPass : impl::MappingPassBase { if (isa(op)) { return false; } - if constexpr (Direction == WireDirection::Forward) { + if (auto measurement = dyn_cast(op); + measurement && !measurementNeedsRouting(measurement)) { + return false; + } return op->isBeforeInBlock(candidate); } @@ -1223,51 +1300,7 @@ struct MappingPass : impl::MappingPassBase { return true; } - /// Only advance past measurements in adaptive-profile - /// scenarios, where a qubit is used after measurement - /// (multiple subsequent measurements are fine) or a bit is - /// used to determine a subsequent chain of unitaries. - /// The forward slice follows SSA def-use chains only. - - Value qubit = m.getQubitOut(); - Value bit = m.getResult(); - - assert(qubit.hasOneUse()); - if (!isa(*qubit.user_begin())) { - return true; - } - - // Verify side-effect dependencies: Does an operation exist - // which reads this value after write? If so, this is an - // adaptive-profile program. - - if (bit.hasOneUse()) { - if (auto store = - dyn_cast(*bit.user_begin())) { - return any_of( - store.getReg().getUsers(), [&](Operation* op) { - if (op == store || - op->getBlock() != store->getBlock() || - !store->isBeforeInBlock(op)) { - return false; - } - return TypeSwitch(op) - .Case([&](cbit::LoadOp ls) { - return ls.getIndex() == store.getIndex(); - }) - .template Case( - [](cbit::ReadOp) { return true; }) - .Default([](Operation*) { return false; }); - }); - } - } - - SetVector slice; - getForwardSlice(bit, &slice); - return any_of(slice, [](Operation* op) { - return isa(op); - }); + return measurementNeedsRouting(m); }) .template Case( [](auto&) { return Direction == WireDirection::Forward; }) diff --git a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp index 1500a7d705..0422c4b9b0 100644 --- a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp +++ b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp @@ -491,6 +491,147 @@ TEST_F(MappingPassFixture, EXPECT_TRUE(isa(*measurement.getQubitOut().getUsers().begin())); } +TEST_F(MappingPassFixture, RouteIndependentControlAfterTerminalWire) { + const auto target = llvm::cantFail(CompilerTarget::create( + 3, Connectivity::fromCouplings({{0, 1}, {1, 2}, {0, 2}}), + NativeOperations::unrestricted())); + + for (const bool measure : {false, true}) { + SCOPED_TRACE(measure); + QCOProgramBuilder builder(context.get()); + builder.initialize(); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + if (measure) { + q0 = builder.measure(q0).first; + } + builder.sink(q0); + q1 = builder.qcoIf(true, q1, [&](Value qubit) { return builder.x(qubit); }); + std::tie(q1, q2) = builder.cx(q1, q2); + builder.sink(q1); + builder.sink(q2); + auto moduleOp = builder.finalize(); + + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded(runPass( + *moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 42}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); + } +} + +TEST_F(MappingPassFixture, RouteControlAfterConsecutiveMeasurements) { + const auto target = llvm::cantFail(CompilerTarget::create( + 3, Connectivity::fromCouplings({{0, 1}, {1, 2}, {0, 2}}), + NativeOperations::unrestricted())); + QCOProgramBuilder builder(context.get()); + builder.initialize(); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + q0 = builder.measure(q0).first; + Value bit; + std::tie(q0, bit) = builder.measure(q0); + builder.sink(q0); + q1 = builder.qcoIf(bit, q1, [&](Value qubit) { return builder.x(qubit); }); + std::tie(q1, q2) = builder.cx(q1, q2); + builder.sink(q1); + builder.sink(q2); + auto moduleOp = builder.finalize(); + + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded(runPass(*moduleOp, target, + MappingPassOptions{.ntrials = 1, .seed = 42}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); +} + +TEST_F(MappingPassFixture, KeepMeasurementStoreBeforeConditionalOverwrite) { + const auto target = llvm::cantFail(CompilerTarget::create( + 3, Connectivity::fromCouplings({{0, 1}, {1, 2}, {0, 2}}), + NativeOperations::unrestricted())); + QCOProgramBuilder builder(context.get()); + builder.initialize(); + auto reg = builder.allocClassicalBitRegister(1); + Value index = arith::ConstantIndexOp::create(builder, 0); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + Value bit; + std::tie(q0, bit) = builder.measure(q0); + builder.storeClassicalBit(bit, reg, index); + builder.sink(q0); + q1 = builder.qcoIf(true, q1, [&](Value qubit) { + Value zero = arith::ConstantIntOp::create(builder, 0, 1); + builder.storeClassicalBit(zero, reg, index); + return builder.x(qubit); + }); + std::tie(q1, q2) = builder.cx(q1, q2); + builder.sink(q1); + builder.sink(q2); + auto moduleOp = builder.finalize(); + + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded(runPass(*moduleOp, target, + MappingPassOptions{.ntrials = 1, .seed = 42}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + auto entry = getEntryPoint(*moduleOp); + EXPECT_TRUE(isExecutable(entry, target)); + auto store = *entry.getOps().begin(); + auto conditional = *entry.getOps().begin(); + EXPECT_TRUE(store->isBeforeInBlock(conditional)); +} + +TEST_F(MappingPassFixture, KeepOutputOnlyRegisterMeasurementsTerminal) { + const auto target = llvm::cantFail( + CompilerTarget::create(3, Connectivity::fromCouplings({{0, 1}, {1, 2}}), + NativeOperations::unrestricted())); + + for (const bool returnRead : {false, true}) { + SCOPED_TRACE(returnRead); + QCOProgramBuilder builder(context.get()); + Type resultType = builder.getI1Type(); + if (!returnRead) { + resultType = cbit::RegisterType::get(context.get(), 1); + } + builder.initialize({resultType}); + auto reg = builder.allocClassicalBitRegister(1); + Value index = arith::ConstantIndexOp::create(builder, 0); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + std::tie(q0, q1) = builder.cx(q0, q1); + std::tie(q0, q2) = builder.cx(q0, q2); + Value bit; + std::tie(q0, bit) = builder.measure(q0); + builder.storeClassicalBit(bit, reg, index); + builder.sink(q0); + std::tie(q1, q2) = builder.cx(q1, q2); + std::tie(q1, bit) = builder.measure(q1); + builder.storeClassicalBit(bit, reg, index); + builder.sink(q1); + builder.sink(q2); + Value output = reg; + if (returnRead) { + output = cbit::ReadOp::create(builder, builder.getI1Type(), reg); + } + auto moduleOp = builder.finalize(output); + + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded(runPass( + *moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 0}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); + moduleOp->walk([](MeasureOp measurement) { + ASSERT_TRUE(measurement.getQubitOut().hasOneUse()); + // Output-only measurements must remain valid for the QIR base profile. + EXPECT_TRUE((isa( + *measurement.getQubitOut().getUsers().begin()))); + }); + } +} + TEST_F(MappingPassFixture, PreserveNoncontiguousTargetSiteIds) { constexpr int64_t size = 3; @@ -706,14 +847,17 @@ TEST_F(MappingPassFixture, KeepWorkspaceSparseOnLargeTarget) { TEST_F(MappingPassFixture, PreserveStoredRegisterControlDuringRouting) { constexpr StringLiteral source = R"mlir( module { - func.func @main() attributes {mqt.entry_point} { + func.func @main() -> !cbit.reg<1> attributes {mqt.entry_point} { %c0 = arith.constant 0 : index %reg = cbit.alloc(#cbit.init) : !cbit.reg<1> + %extra = cbit.alloc(#cbit.init) : !cbit.reg<1> %q0 = qco.alloc : !qco.qubit %q1 = qco.alloc : !qco.qubit %q2 = qco.alloc : !qco.qubit %measured, %bit = qco.measure %q0 : !qco.qubit cbit.store %bit, %reg[%c0] : !cbit.reg<1> + cbit.store %bit, %extra[%c0] : !cbit.reg<1> + qco.sink %measured : !qco.qubit %one = arith.constant 1 : i64 %two = arith.addi %one, %one : i64 %snapshot = cbit.read %reg : !cbit.reg<1> -> i1 @@ -734,10 +878,9 @@ TEST_F(MappingPassFixture, PreserveStoredRegisterControlDuringRouting) { } %next1, %next2 = qco.swap %controlled1, %q2 : !qco.qubit, !qco.qubit -> !qco.qubit, !qco.qubit - qco.sink %measured : !qco.qubit qco.sink %next1 : !qco.qubit qco.sink %next2 : !qco.qubit - return + return %extra : !cbit.reg<1> } } )mlir"; From 1432818b43f2ebc6d877f310396946b22a89c465 Mon Sep 17 00:00:00 2001 From: matthias Date: Wed, 9 Sep 2026 08:47:00 +0200 Subject: [PATCH 10/11] Final touches on implementation --- .../QCO/Transforms/Mapping/Mapping.cpp | 91 ++++++++++--------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 0945613b79..7920e81b3c 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -12,6 +12,7 @@ #include "mlir/Compiler/Target.h" #include "mlir/Compiler/TargetEnvironment.h" +#include "mlir/Dialect/CBit/IR/CBitDialect.h" #include "mlir/Dialect/CBit/IR/CBitOps.h" #include "mlir/Dialect/MQT/IR/MQTDialect.h" #include "mlir/Dialect/QCO/IR/QCODialect.h" @@ -1154,10 +1155,12 @@ struct MappingPass : impl::MappingPassBase { } /// Return whether quantum work follows a measurement through its wire, SSA - /// results, or the register-effect order preserved by the topological sorter. + /// results, or any register effects. This function assumes the direction + /// WireDirection::Forward. static bool measurementNeedsRouting(MeasureOp measurement) { - SetVector worklist; - const auto addSlice = [&](Operation* root) { + Block* const block = measurement->getBlock(); + + const auto addSlice = [](Operation* root, SetVector& worklist) { SetVector slice; ForwardSliceOptions options; options.inclusive = true; @@ -1166,72 +1169,77 @@ struct MappingPass : impl::MappingPassBase { worklist.insert(slice.begin(), slice.end()); }; - // A later measurement on this wire can feed quantum work even when the - // first result is only returned to the caller. - for (auto current = measurement;;) { - auto qubit = current.getQubitOut(); - assert(qubit.hasOneUse()); - Operation* next = *qubit.getUsers().begin(); - if (!isa(next)) { - return true; - } - for (Operation* user : current.getResult().getUsers()) { - addSlice(user); + SetVector worklist; + + // Find adaptive-profile scenarios, where a measured qubit is fed into + // further quantum work (multiple consecutive measurements are fine). + + WireIterator it(measurement.getQubitOut()); + for (; it != std::default_sentinel; ++it) { + if (auto meas = dyn_cast(it.operation())) { + Value bit = meas.getResult(); + for_each(bit.getUsers(), + [&](Operation* user) { addSlice(user, worklist); }); + continue; } - if (isa(next)) { + + if (isa(it.operation())) { break; } - current = cast(next); + + return true; } - Block* block = measurement->getBlock(); - DenseMap firstEffect; - // addSlice can append work, so do not retain iterators or cache the end. - size_t index = 0; - while (index < worklist.size()) { - Operation* op = worklist[index++]; - if (isa(op) || - (isa(op) && - any_of(op->getResultTypes(), - [](Type type) { return isa(type); }))) { + DenseSet> processed; + + for (size_t cursor = 0; cursor < worklist.size(); ++cursor) { + Operation* op = worklist[cursor]; + + // If any transitive dependency of the measurement consumes qubits, the + // program fulfills the adaptive profile. + + if (any_of(op->getOperandTypes(), + [](auto type) { return isa(type); })) { return true; } // Captures and nested register accesses also constrain their enclosing // composite, whose placement threads every physical wire through it. - Operation* ancestor = block->findAncestorOpInBlock(*op); - if (ancestor != op) { - if (ancestor != nullptr) { - addSlice(ancestor); + + if (op->getBlock() != block) { + if (Operation* ancestor = block->findAncestorOpInBlock(*op); + ancestor != nullptr) { + addSlice(ancestor, worklist); } - continue; } const auto effects = getEffectsRecursively(op); if (!effects) { continue; } + for (const auto& effect : *effects) { - auto reg = effect.getValue(); - if (!reg || !isa(reg.getType())) { + auto value = effect.getValue(); + auto reg = dyn_cast_if_present>(value); + if (!reg) { continue; } - auto [it, inserted] = firstEffect.try_emplace(reg, op); - if (!inserted && !op->isBeforeInBlock(it->second)) { + + auto [it, inserted] = processed.insert(reg); + if (!inserted) { continue; } - it->second = op; - // Match the sorter's whole-register effect order, including overwrites. - // Output-only reads and writes do not require quantum work. for (Operation* user : reg.getUsers()) { - Operation* next = block->findAncestorOpInBlock(*user); - if (next != nullptr && next != op && op->isBeforeInBlock(next)) { - addSlice(next); + if (user == op) { + continue; } + + addSlice(user, worklist); } } } + return false; } @@ -1330,6 +1338,7 @@ struct MappingPass : impl::MappingPassBase { // become ready at once. Hot routing threads every qubit through each // composite, so processing a later operation first could introduce a // use-before-definition for an earlier operation. + llvm::sort(composites, [](const CompositeUnitary& lhs, const CompositeUnitary& rhs) { assert(lhs.op->getBlock() == rhs.op->getBlock()); From aa1b13cf83e9fe12c9852abcaa4512b3f0331b79 Mon Sep 17 00:00:00 2001 From: Lukas Burgholzer Date: Wed, 9 Sep 2026 08:07:21 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=90=9B=20Preserve=20mapped=20contro?= =?UTF-8?q?l=20boundaries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Handle exhausted nested wires during backward traversal and select composite inputs before the composite's block-order boundary. Keep earlier register accesses from forcing measured-qubit reuse. Cache consecutive measurement classifications during each unchanged traversal to avoid repeated suffix and register scans. Assisted-by: Codex --- .../preserve-mapped-classical-control.md | 43 +++-- .../QCO/Transforms/Mapping/Mapping.cpp | 159 ++++++++++-------- .../QCO/Transforms/Mapping/test_mapping.cpp | 159 ++++++++++++++++++ 3 files changed, 280 insertions(+), 81 deletions(-) diff --git a/.agent/plans/preserve-mapped-classical-control.md b/.agent/plans/preserve-mapped-classical-control.md index 362e3510ca..d1bebb610e 100644 --- a/.agent/plans/preserve-mapped-classical-control.md +++ b/.agent/plans/preserve-mapped-classical-control.md @@ -1,6 +1,7 @@ # Preserve classical control during target mapping -Status: complete. The routing regressions and required local checks pass. +Status: complete. Routing, terminal-measurement, and performance regressions +pass. ## Goal and scope @@ -25,26 +26,38 @@ whole-register write followed by an indexed load during dominance repair. - Defer composites while an earlier wire operation still needs routing. Terminal sinks and output-only measurements must not block independent quantum work. - Use one measurement classification for advancement and composite deferral. - Follow all SSA result uses and the sorter's whole-register effect order until - reaching quantum work. Output-only loads and overwrites remain terminal; + Follow SSA result uses and forward whole-register effects until reaching + quantum work. Register accesses before the measurement run do not require + measured-qubit reuse. Output-only loads and overwrites remain terminal; register accesses inside quantum composites still impose ordering. - Keep consecutive measurements in scope so an earlier measurement does not hide a later result used for quantum control. Reuse LLVM slice analysis and a bounded worklist. Traverse it by index because discovering more effects can append entries and invalidate iterators. No public API or dependency changes are required. +- Cache consecutive-measurement suffix classifications within each `advance` + invocation, sharing slice and register work across the run. Discard the cache + before hot routing mutates the graph. +- Backward traversal of an idle nested block argument can reach its sentinel; + that exhausted wire does not defer another composite. +- Select the wire value crossing the composite's block-order boundary when + extending it. Another wire may already have advanced through a gate that + consumes a classical result of that composite; its current value would + introduce an SSA cycle. +- Prefer terminal measurements even when the target accepts adaptive programs. + Target permission does not replace program dependencies or placement + readiness. ## Validation -Build the release mapping, QCO utility, and compiler unit-test targets. The -mapping binary passes all 96 tests, the QCO utility binary passes all 192, and -the compiler binary passes all 180. The focused mapping tests cover terminal -measurements and sinks before independent control, consecutive measurements, -multiple result users, output-only register reads and overwrites, and register -writes inside a quantum conditional. All six focused regressions also pass 25 -consecutive repetitions. - -Run `uvx nox -s lint` and `uvx nox -s cpp-lint -- ` for the change. -The repository hooks pass. Full-file C++ lint passes for all three C++ files in -the PR, using main base `4c5e45855`. These are local checks, not hosted CI or a -full Benchpress corpus run. +Release unit tests pass: 100 mapping tests, 192 QCO utility tests, and 181 +compiler tests. The added cases cover idle nested wires, a conditional angle +consumed on another wire, terminal measurements after earlier register control, +and consecutive measurements with first/last result control and shared stores. + +The consecutive-measurement probe at 4,000 measurements improved from 327.85 ms +to 2.398 ms before the boundary and earlier-access fixes. This measures mapping +alone on a synthetic workload, not a whole-program or corpus speedup. + +Repository lint and full-file C++ lint against fixed base `ce608b082` pass. +These are local checks, not hosted CI or a full Benchpress corpus run. diff --git a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp index 7920e81b3c..5a07f6378e 100644 --- a/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp +++ b/mlir/lib/Dialect/QCO/Transforms/Mapping/Mapping.cpp @@ -27,6 +27,7 @@ #include "mlir/Dialect/QTensor/IR/QTensorOps.h" #include "mlir/Dialect/QTensor/Utils/TensorIterator.h" +#include #include #include #include @@ -781,6 +782,18 @@ struct MappingPass : impl::MappingPassBase { return newWhileOp; } + /// Return the wire value before a composite, even if advancement passed it. + static Value valueBeforeBoundary(WireIterator iterator, Operation* boundary) { + if (iterator == std::default_sentinel) { + --iterator; + } + while (iterator.operation() != nullptr && + !iterator.operation()->isBeforeInBlock(boundary)) { + --iterator; + } + return iterator.qubit(); + } + /// Execute `ntrials` many (parallel) initial layout refinement trials and /// return the heuristically best one. /// @@ -1154,10 +1167,27 @@ struct MappingPass : impl::MappingPassBase { stats.nswaps += swaps.size(); } - /// Return whether quantum work follows a measurement through its wire, SSA - /// results, or any register effects. This function assumes the direction - /// WireDirection::Forward. - static bool measurementNeedsRouting(MeasureOp measurement) { + /// Classify a consecutive measurement run from its end, so each suffix is + /// visited once. The cache is valid only while the IR remains unchanged. + static bool measurementNeedsRouting(MeasureOp measurement, + DenseMap& cache) { + SmallVector measurements; + bool needsRouting = false; + WireIterator it(measurement.getQubitOut()); + for (; it != std::default_sentinel; ++it) { + Operation* op = it.operation(); + if (const auto cached = cache.find(op); cached != cache.end()) { + needsRouting = cached->second; + break; + } + if (auto next = dyn_cast(op)) { + measurements.push_back(next); + continue; + } + needsRouting = !isa(op); + break; + } + Block* const block = measurement->getBlock(); const auto addSlice = [](Operation* root, SetVector& worklist) { @@ -1171,94 +1201,93 @@ struct MappingPass : impl::MappingPassBase { SetVector worklist; - // Find adaptive-profile scenarios, where a measured qubit is fed into - // further quantum work (multiple consecutive measurements are fine). - - WireIterator it(measurement.getQubitOut()); - for (; it != std::default_sentinel; ++it) { - if (auto meas = dyn_cast(it.operation())) { - Value bit = meas.getResult(); - for_each(bit.getUsers(), - [&](Operation* user) { addSlice(user, worklist); }); - continue; - } - - if (isa(it.operation())) { - break; - } - - return true; - } - DenseSet> processed; - for (size_t cursor = 0; cursor < worklist.size(); ++cursor) { - Operation* op = worklist[cursor]; - - // If any transitive dependency of the measurement consumes qubits, the - // program fulfills the adaptive profile. - - if (any_of(op->getOperandTypes(), - [](auto type) { return isa(type); })) { - return true; - } + size_t cursor = 0; + const auto resultNeedsRouting = [&](MeasureOp next) { + for_each(next.getResult().getUsers(), + [&](Operation* user) { addSlice(user, worklist); }); + for (; cursor < worklist.size(); ++cursor) { + Operation* op = worklist[cursor]; - // Captures and nested register accesses also constrain their enclosing - // composite, whose placement threads every physical wire through it. + /// Quantum consumers require the measurement to advance, independent + /// of the selected target's permission to reuse measured qubits. - if (op->getBlock() != block) { - if (Operation* ancestor = block->findAncestorOpInBlock(*op); - ancestor != nullptr) { - addSlice(ancestor, worklist); + if (any_of(op->getOperandTypes(), + [](auto type) { return isa(type); })) { + return true; } - } - const auto effects = getEffectsRecursively(op); - if (!effects) { - continue; - } + // Captures and nested register accesses also constrain their enclosing + // composite, whose placement threads every physical wire through it. - for (const auto& effect : *effects) { - auto value = effect.getValue(); - auto reg = dyn_cast_if_present>(value); - if (!reg) { - continue; + if (op->getBlock() != block) { + if (Operation* ancestor = block->findAncestorOpInBlock(*op); + ancestor != nullptr) { + addSlice(ancestor, worklist); + } } - auto [it, inserted] = processed.insert(reg); - if (!inserted) { + const auto effects = getEffectsRecursively(op); + if (!effects) { continue; } - for (Operation* user : reg.getUsers()) { - if (user == op) { + for (const auto& effect : *effects) { + auto value = effect.getValue(); + auto reg = dyn_cast_if_present>(value); + if (!reg) { continue; } - addSlice(user, worklist); + auto [it, inserted] = processed.insert(reg); + if (!inserted) { + continue; + } + + for (Operation* user : reg.getUsers()) { + Operation* ancestor = block->findAncestorOpInBlock(*user); + if (user == op || + (ancestor && !measurement->isBeforeInBlock(ancestor))) { + continue; + } + + addSlice(user, worklist); + } } } - } - return false; + return false; + }; + + for (auto next : llvm::reverse(measurements)) { + needsRouting = needsRouting || resultNeedsRouting(next); + cache.try_emplace(next, needsRouting); + } + return needsRouting; } /// Advance past executable gates and return ready composite operations. /// Leave wires at non-executable gates, composites, terminal measurements, - /// or sink-like operations, never at the sentinel. + /// or sink-like operations. Backward traversal can exhaust block arguments. template SmallVector advance(Wires& wires, const WireInfos& infos, const Layout& layout) { DenseSet visited; SmallVector composites; + /// Advancement only moves iterators. Discard classifications before routing + /// inserts SWAPs or replaces composites. + DenseMap measurementRouting; // The wire traversal does not follow classical dependencies. Defer a // composite until earlier routing work is complete, but let independent // composites pass terminal wires. Reverse block order for backward routing. - const auto defer = [&wires](Operation* candidate) { + const auto defer = [&wires, &measurementRouting](Operation* candidate) { return any_of(wires, [&](WireIterator& it) { - assert(it != std::default_sentinel); + if (it == std::default_sentinel) { + return false; + } Operation* op = it.operation(); if (op == nullptr || op == candidate) { @@ -1270,7 +1299,8 @@ struct MappingPass : impl::MappingPassBase { } if constexpr (Direction == WireDirection::Forward) { if (auto measurement = dyn_cast(op); - measurement && !measurementNeedsRouting(measurement)) { + measurement && + !measurementNeedsRouting(measurement, measurementRouting)) { return false; } return op->isBeforeInBlock(candidate); @@ -1301,12 +1331,12 @@ struct MappingPass : impl::MappingPassBase { return target->areAdjacent(hw0, hw1); }) .Case([](ResetOp&) { return true; }) - .Case([](MeasureOp& m) { + .Case([&](MeasureOp& m) { if (Direction == WireDirection::Backward) { return true; } - return measurementNeedsRouting(m); + return measurementNeedsRouting(m, measurementRouting); }) .template Case( [](auto&) { return Direction == WireDirection::Forward; }) @@ -1375,10 +1405,7 @@ struct MappingPass : impl::MappingPassBase { allIndices, [&](const size_t i) { return !included.contains(i); })); const SmallVector addons(map_range(excluded, [&](const size_t i) { - // Make sure the qubits point to an already processed operation. - const auto& it = std::prev( - parent.wires[i], parent.wires[i] == std::default_sentinel ? 2 : 1); - return it.qubit(); + return valueBeforeBoundary(parent.wires[i], composite.op); })); composite = CompositeUnitary{ diff --git a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp index 04401e257e..3ad277a997 100644 --- a/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp +++ b/mlir/unittests/Dialect/QCO/Transforms/Mapping/test_mapping.cpp @@ -622,6 +622,165 @@ TEST_F(MappingPassFixture, RouteControlAfterConsecutiveMeasurements) { EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); } +TEST_F(MappingPassFixture, MapNestedControlWithIdleWire) { + const auto target = llvm::cantFail( + CompilerTarget::create(2, Connectivity::fromCouplings({{0, 1}}), + NativeOperations::unrestricted())); + auto moduleOp = parseSourceString(R"mlir( +module { + func.func @main(%c: i1, %innerCondition: i1) attributes {mqt.entry_point} { + %q0 = qco.alloc : !qco.qubit + %q1 = qco.alloc : !qco.qubit + %out0, %out1 = qco.if %c args(%a = %q0, %b = %q1) -> (!qco.qubit, !qco.qubit) { + %inner = qco.if %innerCondition args(%i = %a) -> (!qco.qubit) { + %x = qco.x %i : !qco.qubit -> !qco.qubit + qco.yield %x : !qco.qubit + } else args(%i = %a) { + qco.yield %i : !qco.qubit + } + qco.yield %inner, %b : !qco.qubit, !qco.qubit + } else args(%a = %q0, %b = %q1) { + qco.yield %a, %b : !qco.qubit, !qco.qubit + } + qco.sink %out0 : !qco.qubit + qco.sink %out1 : !qco.qubit + return + } +} + )mlir", + context.get()); + ASSERT_TRUE(moduleOp); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded( + runPass(*moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 0}))); + EXPECT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); +} + +TEST_F(MappingPassFixture, PreserveConditionalGateParameterDependency) { + const auto target = llvm::cantFail( + CompilerTarget::create(2, Connectivity::fromCouplings({{0, 1}}), + NativeOperations::unrestricted())); + auto moduleOp = parseSourceString(R"mlir( +module { + func.func @main(%condition: i1) -> (i1, i1) attributes {mqt.entry_point} { + %q0 = qco.alloc : !qco.qubit + %q1 = qco.alloc : !qco.qubit + %angle, %out = qco.if %condition args(%a = %q0) -> (f64, !qco.qubit) { + %v = arith.constant 1.0 : f64 + %x = qco.x %a : !qco.qubit -> !qco.qubit + qco.yield %v, %x : f64, !qco.qubit + } else args(%a = %q0) { + %v = arith.constant 2.0 : f64 + %h = qco.h %a : !qco.qubit -> !qco.qubit + qco.yield %v, %h : f64, !qco.qubit + } + %rotated = qco.rx(%angle) %q1 : !qco.qubit -> !qco.qubit + %done0, %bit0 = qco.measure %out : !qco.qubit + %done1, %bit1 = qco.measure %rotated : !qco.qubit + qco.sink %done0 : !qco.qubit + qco.sink %done1 : !qco.qubit + return %bit0, %bit1 : i1, i1 + } +} + )mlir", + context.get()); + ASSERT_TRUE(moduleOp); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded( + runPass(*moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 0}))); + EXPECT_TRUE(succeeded(verify(*moduleOp))); + EXPECT_TRUE(isExecutable(getEntryPoint(*moduleOp), target)); +} + +TEST_F(MappingPassFixture, + KeepMeasurementsTerminalAfterEarlierRegisterControl) { + const auto target = llvm::cantFail( + CompilerTarget::create(3, Connectivity::fromCouplings({{0, 1}, {1, 2}}), + NativeOperations::unrestricted())); + QCOProgramBuilder builder(context.get()); + builder.initialize({cbit::RegisterType::get(context.get(), 1)}); + auto reg = builder.allocClassicalBitRegister(1); + Value index = arith::ConstantIndexOp::create(builder, 0); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + auto q2 = builder.allocQubit(); + auto oldBit = builder.loadClassicalBit(reg, index); + q0 = builder.qcoIf(oldBit, q0, [&](Value q) { return builder.x(q); }); + std::tie(q0, q1) = builder.cx(q0, q1); + std::tie(q0, q2) = builder.cx(q0, q2); + Value bit; + std::tie(q0, bit) = builder.measure(q0); + builder.storeClassicalBit(bit, reg, index); + builder.sink(q0); + std::tie(q1, q2) = builder.cx(q1, q2); + std::tie(q1, bit) = builder.measure(q1); + builder.storeClassicalBit(bit, reg, index); + builder.sink(q1); + builder.sink(q2); + auto moduleOp = builder.finalize(reg); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded( + runPass(*moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 0}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + moduleOp->walk([](MeasureOp measurement) { + EXPECT_TRUE(( + isa(*measurement.getQubitOut().getUsers().begin()))); + }); +} + +TEST_F(MappingPassFixture, RouteControlFromConsecutiveMeasurementResults) { + const auto target = llvm::cantFail( + CompilerTarget::create(2, Connectivity::fromCouplings({{0, 1}}), + NativeOperations::unrestricted())); + constexpr size_t numMeasurements = 128; + for (const bool storeResults : {false, true}) { + SCOPED_TRACE(storeResults); + for (const size_t controlIndex : {size_t{0}, numMeasurements - 1}) { + SCOPED_TRACE(controlIndex); + QCOProgramBuilder builder(context.get()); + builder.initialize({cbit::RegisterType::get(context.get(), 1)}); + auto reg = builder.allocClassicalBitRegister(1); + Value index = arith::ConstantIndexOp::create(builder, 0); + auto q0 = builder.allocQubit(); + auto q1 = builder.allocQubit(); + Value condition; + for (size_t i = 0; i < numMeasurements; ++i) { + Value bit; + std::tie(q0, bit) = builder.measure(q0); + if (storeResults) { + builder.storeClassicalBit(bit, reg, index); + } + if (i == controlIndex) { + condition = bit; + } + } + q1 = builder.qcoIf(condition, q1, + [&](Value qubit) { return builder.x(qubit); }); + builder.sink(q0); + builder.sink(q1); + auto moduleOp = builder.finalize(reg); + + ASSERT_TRUE(succeeded(verify(*moduleOp))); + ASSERT_TRUE(succeeded(runPass( + *moduleOp, target, MappingPassOptions{.ntrials = 1, .seed = 0}))); + ASSERT_TRUE(succeeded(verify(*moduleOp))); + auto entry = getEntryPoint(*moduleOp); + EXPECT_TRUE(isExecutable(entry, target)); + auto measurements = llvm::to_vector(entry.getOps()); + ASSERT_EQ(measurements.size(), numMeasurements); + auto conditional = *entry.getOps().begin(); + EXPECT_EQ(conditional.getCondition(), condition); + if (controlIndex == 0) { + /// A dependency of the first result must not make later measurements + /// nonterminal. + EXPECT_TRUE( + isa(*measurements.back().getQubitOut().getUsers().begin())); + } + } + } +} + TEST_F(MappingPassFixture, KeepMeasurementStoreBeforeConditionalOverwrite) { const auto target = llvm::cantFail(CompilerTarget::create( 3, Connectivity::fromCouplings({{0, 1}, {1, 2}, {0, 2}}),