Make SabreSwap account for clbits in predecessors

qiskit/transpiler/passes/routing/sabre_swap.py

@@ -18,7 +18,6 @@
 import numpy as np
 
 from qiskit.circuit.library.standard_gates import SwapGate
-from qiskit.circuit.quantumregister import Qubit
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.exceptions import TranspilerError
 from qiskit.transpiler.layout import Layout
@@ -276,15 +275,14 @@ def _reset_qubits_decay(self):
         self.qubits_decay = {k: 1 for k in self.qubits_decay.keys()}
 
     def _successors(self, node, dag):
-        for _, successor, edge_data in dag.edges(node):
+        for _, successor, _ in dag.edges(node):
             if not isinstance(successor, DAGOpNode):
                 continue
-            if isinstance(edge_data, Qubit):
-                yield successor
+            yield successor
 
     def _is_resolved(self, node):
         """Return True if all of a node's predecessors in dag are applied."""
-        return self.applied_predecessors[node] == len(node.qargs)
+        return self.applied_predecessors[node] == len(node.qargs) + len(node.cargs)
 
     def _obtain_extended_set(self, dag, front_layer):
         """Populate extended_set by looking ahead a fixed number of gates.

releasenotes/notes/fix-sabreswap-clbits-428eb5f3a46063da.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixed :class:`.SabreSwap`, and by extension :func:`.transpile` with
+    ``optimization_level=3``, occasionally re-ordering measurements invalidly.
+    Previously, if two measurements wrote to the same classical bit,
+    :class:`.SabreSwap` could (depending on the coupling map) re-order them to
+    produce a non-equivalent circuit.  This behaviour was stochastic, so may
+    not have appeared reliably.

test/python/transpiler/test_sabre_swap.py

@@ -13,6 +13,7 @@
 """Test the Sabre Swap pass"""
 
 import unittest
+from qiskit.circuit.library import CCXGate, HGate, Measure
 from qiskit.transpiler.passes import SabreSwap
 from qiskit.transpiler import CouplingMap, PassManager
 from qiskit import QuantumRegister, QuantumCircuit
@@ -95,6 +96,34 @@ def test_do_not_change_cm(self):
 
         self.assertEqual(set(cm_edges), set(coupling.get_edges()))
 
+    def test_do_not_reorder_measurements(self):
+        """Test that SabreSwap doesn't reorder measurements to the same classical bit.
+
+        With the particular coupling map used in this test and the 3q ccx gate, the routing would
+        invariably the measurements if the classical successors are not accurately tracked.
+        Regression test of gh-7950."""
+        coupling = CouplingMap([(0, 2), (2, 0), (1, 2), (2, 1)])
+        qc = QuantumCircuit(3, 1)
+        qc.compose(CCXGate().definition, [0, 1, 2], [])  # Unroll CCX to 2q operations.
+        qc.h(0)
+        qc.barrier()
+        qc.measure(0, 0)  # This measure is 50/50 between the Z states.
+        qc.measure(1, 0)  # This measure always overwrites with 0.
+        passmanager = PassManager(SabreSwap(coupling))
+        transpiled = passmanager.run(qc)
+
+        last_h, h_qubits, _ = transpiled.data[-4]
+        self.assertIsInstance(last_h, HGate)
+        first_measure, first_measure_qubits, _ = transpiled.data[-2]
+        second_measure, second_measure_qubits, _ = transpiled.data[-1]
+        self.assertIsInstance(first_measure, Measure)
+        self.assertIsInstance(second_measure, Measure)
+        # Assert that the first measure is on the same qubit that the HGate was applied to, and the
+        # second measurement is on a different qubit (though we don't care which exactly - that
+        # depends a little on the randomisation of the pass).
+        self.assertEqual(h_qubits, first_measure_qubits)
+        self.assertNotEqual(h_qubits, second_measure_qubits)
+
 
 if __name__ == "__main__":
     unittest.main()