Avoid operator creation in transpiler (#12826) (#12875)

* Avoid operator creation in transpiler

This removes very nearly all of the use of `DAGOpNode.op` in the default
transpiler paths.  The sole exception is in `InverseCancellation`, which
currently would involve some quite awkward gymnastics for little
near-term benefit. The pass should move fully to Rust soon, making it
not worth the effort.

Most of the tricks here involve using the knowledge that most operations
will involve only Rust-space standard gates, and that these cannot be
control-flow operations.

* Fix `HighLevelSynthesis` fast path

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
(cherry picked from commit 0afb06e)

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

crates/circuit/src/circuit_instruction.rs

@@ -22,7 +22,9 @@ use pyo3::{intern, IntoPy, PyObject, PyResult};
 
 use smallvec::SmallVec;
 
-use crate::imports::{GATE, INSTRUCTION, OPERATION, WARNINGS_WARN};
+use crate::imports::{
+    CONTROLLED_GATE, CONTROL_FLOW_OP, GATE, INSTRUCTION, OPERATION, WARNINGS_WARN,
+};
 use crate::operations::{
     Operation, OperationRef, Param, PyGate, PyInstruction, PyOperation, StandardGate,
 };
@@ -266,11 +268,36 @@ impl CircuitInstruction {
             .and_then(|attrs| attrs.unit.as_deref())
     }
 
-    #[getter]
+    /// Is the :class:`.Operation` contained in this instruction a Qiskit standard gate?
     pub fn is_standard_gate(&self) -> bool {
         self.operation.try_standard_gate().is_some()
     }
 
+    /// Is the :class:`.Operation` contained in this instruction a subclass of
+    /// :class:`.ControlledGate`?
+    pub fn is_controlled_gate(&self, py: Python) -> PyResult<bool> {
+        match self.operation.view() {
+            OperationRef::Standard(standard) => Ok(standard.num_ctrl_qubits() != 0),
+            OperationRef::Gate(gate) => gate
+                .gate
+                .bind(py)
+                .is_instance(CONTROLLED_GATE.get_bound(py)),
+            _ => Ok(false),
+        }
+    }
+
+    /// Is the :class:`.Operation` contained in this node a directive?
+    pub fn is_directive(&self) -> bool {
+        self.op().directive()
+    }
+
+    /// Is the :class:`.Operation` contained in this instruction a control-flow operation (i.e. an
+    /// instance of :class:`.ControlFlowOp`)?
+    pub fn is_control_flow(&self) -> bool {
+        self.op().control_flow()
+    }
+
+    /// Does this instruction contain any :class:`.ParameterExpression` parameters?
     pub fn is_parameterized(&self) -> bool {
         self.params
             .iter()
@@ -557,6 +584,7 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
                 clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?,
                 params: params.len() as u32,
                 op_name: ob.getattr(intern!(py, "name"))?.extract()?,
+                control_flow: ob.is_instance(CONTROL_FLOW_OP.get_bound(py))?,
                 instruction: ob.into_py(py),
             });
             return Ok(OperationFromPython {

crates/circuit/src/dag_node.rs

@@ -291,10 +291,6 @@ impl DAGOpNode {
         self.instruction.params = val;
     }
 
-    pub fn is_parameterized(&self) -> bool {
-        self.instruction.is_parameterized()
-    }
-
     #[getter]
     fn matrix(&self, py: Python) -> Option<PyObject> {
         let matrix = self.instruction.op().matrix(&self.instruction.params);
@@ -333,11 +329,32 @@ impl DAGOpNode {
             .and_then(|attrs| attrs.unit.as_deref())
     }
 
-    #[getter]
+    /// Is the :class:`.Operation` contained in this node a Qiskit standard gate?
     pub fn is_standard_gate(&self) -> bool {
         self.instruction.is_standard_gate()
     }
 
+    /// Is the :class:`.Operation` contained in this node a subclass of :class:`.ControlledGate`?
+    pub fn is_controlled_gate(&self, py: Python) -> PyResult<bool> {
+        self.instruction.is_controlled_gate(py)
+    }
+
+    /// Is the :class:`.Operation` contained in this node a directive?
+    pub fn is_directive(&self) -> bool {
+        self.instruction.is_directive()
+    }
+
+    /// Is the :class:`.Operation` contained in this node a control-flow operation (i.e. an instance
+    /// of :class:`.ControlFlowOp`)?
+    pub fn is_control_flow(&self) -> bool {
+        self.instruction.is_control_flow()
+    }
+
+    /// Does this node contain any :class:`.ParameterExpression` parameters?
+    pub fn is_parameterized(&self) -> bool {
+        self.instruction.is_parameterized()
+    }
+
     #[setter]
     fn set_label(&mut self, val: Option<String>) {
         match self.instruction.extra_attrs.as_mut() {

crates/circuit/src/imports.rs

@@ -61,6 +61,8 @@ pub static OPERATION: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.opera
 pub static INSTRUCTION: ImportOnceCell =
     ImportOnceCell::new("qiskit.circuit.instruction", "Instruction");
 pub static GATE: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.gate", "Gate");
+pub static CONTROL_FLOW_OP: ImportOnceCell =
+    ImportOnceCell::new("qiskit.circuit.controlflow", "ControlFlowOp");
 pub static QUBIT: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.quantumregister", "Qubit");
 pub static CLBIT: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.classicalregister", "Clbit");
 pub static PARAMETER_EXPRESSION: ImportOnceCell =

crates/circuit/src/operations.rs

@@ -2016,6 +2016,7 @@ pub struct PyInstruction {
     pub clbits: u32,
     pub params: u32,
     pub op_name: String,
+    pub control_flow: bool,
     pub instruction: PyObject,
 }
 
@@ -2033,7 +2034,7 @@ impl Operation for PyInstruction {
         self.params
     }
     fn control_flow(&self) -> bool {
-        false
+        self.control_flow
     }
     fn matrix(&self, _params: &[Param]) -> Option<Array2<Complex64>> {
         None

crates/circuit/src/packed_instruction.rs

@@ -283,6 +283,7 @@ impl PackedOperation {
                 qubits: instruction.qubits,
                 clbits: instruction.clbits,
                 params: instruction.params,
+                control_flow: instruction.control_flow,
                 op_name: instruction.op_name.clone(),
             }
             .into()),
@@ -316,6 +317,7 @@ impl PackedOperation {
                 qubits: instruction.qubits,
                 clbits: instruction.clbits,
                 params: instruction.params,
+                control_flow: instruction.control_flow,
                 op_name: instruction.op_name.clone(),
             })
             .into()),

qiskit/circuit/commutation_checker.py

@@ -66,11 +66,11 @@ def commute_nodes(
         """Checks if two DAGOpNodes commute."""
         qargs1 = op1.qargs
         cargs1 = op2.cargs
-        if not op1.is_standard_gate:
+        if not op1.is_standard_gate():
             op1 = op1.op
         qargs2 = op2.qargs
         cargs2 = op2.cargs
-        if not op2.is_standard_gate:
+        if not op2.is_standard_gate():
             op2 = op2.op
         return self.commute(op1, qargs1, cargs1, op2, qargs2, cargs2, max_num_qubits)
 

qiskit/dagcircuit/dagcircuit.py

@@ -974,15 +974,26 @@ def _reject_new_register(reg):
             elif isinstance(nd, DAGOpNode):
                 m_qargs = [edge_map.get(x, x) for x in nd.qargs]
                 m_cargs = [edge_map.get(x, x) for x in nd.cargs]
-                op = nd.op.copy()
-                if (condition := getattr(op, "condition", None)) is not None:
-                    if not isinstance(op, ControlFlowOp):
-                        op = op.c_if(*variable_mapper.map_condition(condition, allow_reorder=True))
+                inst = nd._to_circuit_instruction(deepcopy=True)
+                m_op = None
+                if inst.condition is not None:
+                    if inst.is_control_flow():
+                        m_op = inst.operation
+                        m_op.condition = variable_mapper.map_condition(
+                            inst.condition, allow_reorder=True
+                        )
                     else:
-                        op.condition = variable_mapper.map_condition(condition, allow_reorder=True)
-                elif isinstance(op, SwitchCaseOp):
-                    op.target = variable_mapper.map_target(op.target)
-                dag.apply_operation_back(op, m_qargs, m_cargs, check=False)
+                        m_op = inst.operation.c_if(
+                            *variable_mapper.map_condition(inst.condition, allow_reorder=True)
+                        )
+                elif inst.is_control_flow() and isinstance(inst.operation, SwitchCaseOp):
+                    m_op = inst.operation
+                    m_op.target = variable_mapper.map_target(m_op.target)
+                if m_op is None:
+                    inst = inst.replace(qubits=m_qargs, clbits=m_cargs)
+                else:
+                    inst = inst.replace(operation=m_op, qubits=m_qargs, clbits=m_cargs)
+                dag._apply_op_node_back(DAGOpNode.from_instruction(inst), check=False)
             else:
                 raise DAGCircuitError(f"bad node type {type(nd)}")
 
@@ -1460,11 +1471,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None, propagate_condit
         reverse_wire_map = {b: a for a, b in wire_map.items()}
         # It doesn't make sense to try and propagate a condition from a control-flow op; a
         # replacement for the control-flow op should implement the operation completely.
-        if (
-            propagate_condition
-            and not isinstance(node.op, ControlFlowOp)
-            and (op_condition := getattr(node.op, "condition", None)) is not None
-        ):
+        if propagate_condition and not node.is_control_flow() and node.condition is not None:
             in_dag = input_dag.copy_empty_like()
             # The remapping of `condition` below is still using the old code that assumes a 2-tuple.
             # This is because this remapping code only makes sense in the case of non-control-flow
@@ -1473,7 +1480,7 @@ def substitute_node_with_dag(self, node, input_dag, wires=None, propagate_condit
             # in favour of the new-style conditional blocks.  The extra logic in here to add
             # additional wires into the map as necessary would hugely complicate matters if we tried
             # to abstract it out into the `VariableMapper` used elsewhere.
-            target, value = op_condition
+            target, value = node.condition
             if isinstance(target, Clbit):
                 new_target = reverse_wire_map.get(target, Clbit())
                 if new_target not in wire_map:
@@ -1593,25 +1600,31 @@ def edge_weight_map(wire):
         for old_node_index, new_node_index in node_map.items():
             # update node attributes
             old_node = in_dag._multi_graph[old_node_index]
-            if isinstance(old_node.op, SwitchCaseOp):
+            m_op = None
+            if not old_node.is_standard_gate() and isinstance(old_node.op, SwitchCaseOp):
                 m_op = SwitchCaseOp(
                     variable_mapper.map_target(old_node.op.target),
                     old_node.op.cases_specifier(),
                     label=old_node.op.label,
                 )
-            elif getattr(old_node.op, "condition", None) is not None:
+            elif old_node.condition is not None:
                 m_op = old_node.op
-                if not isinstance(old_node.op, ControlFlowOp):
+                if old_node.is_control_flow():
+                    m_op.condition = variable_mapper.map_condition(m_op.condition)
+                else:
                     new_condition = variable_mapper.map_condition(m_op.condition)
                     if new_condition is not None:
                         m_op = m_op.c_if(*new_condition)
-                else:
-                    m_op.condition = variable_mapper.map_condition(m_op.condition)
-            else:
-                m_op = old_node.op
             m_qargs = [wire_map[x] for x in old_node.qargs]
             m_cargs = [wire_map[x] for x in old_node.cargs]
-            new_node = DAGOpNode(m_op, qargs=m_qargs, cargs=m_cargs, dag=self)
+            old_instruction = old_node._to_circuit_instruction()
+            if m_op is None:
+                new_instruction = old_instruction.replace(qubits=m_qargs, clbits=m_cargs)
+            else:
+                new_instruction = old_instruction.replace(
+                    operation=m_op, qubits=m_qargs, clbits=m_cargs
+                )
+            new_node = DAGOpNode.from_instruction(new_instruction)
             new_node._node_id = new_node_index
             self._multi_graph[new_node_index] = new_node
             self._increment_op(new_node.name)
@@ -1840,11 +1853,18 @@ def op_nodes(self, op=None, include_directives=True):
             list[DAGOpNode]: the list of node ids containing the given op.
         """
         nodes = []
+        filter_is_nonstandard = getattr(op, "_standard_gate", None) is None
         for node in self._multi_graph.nodes():
             if isinstance(node, DAGOpNode):
-                if not include_directives and getattr(node.op, "_directive", False):
+                if not include_directives and node.is_directive():
                     continue
-                if op is None or isinstance(node.op, op):
+                if op is None or (
+                    # This middle catch is to avoid Python-space operation creation for most uses of
+                    # `op`; we're usually just looking for control-flow ops, and standard gates
+                    # aren't control-flow ops.
+                    not (filter_is_nonstandard and node.is_standard_gate())
+                    and isinstance(node.op, op)
+                ):
                     nodes.append(node)
         return nodes
 
@@ -1864,7 +1884,7 @@ def named_nodes(self, *names):
         """Get the set of "op" nodes with the given name."""
         named_nodes = []
         for node in self._multi_graph.nodes():
-            if isinstance(node, DAGOpNode) and node.op.name in names:
+            if isinstance(node, DAGOpNode) and node.name in names:
                 named_nodes.append(node)
         return named_nodes
 
@@ -2070,14 +2090,11 @@ def layers(self, *, vars_mode: _VarsMode = "captures"):
             new_layer = self.copy_empty_like(vars_mode=vars_mode)
 
             for node in op_nodes:
-                # this creates new DAGOpNodes in the new_layer
-                new_layer.apply_operation_back(node.op, node.qargs, node.cargs, check=False)
+                new_layer._apply_op_node_back(node, check=False)
 
             # The quantum registers that have an operation in this layer.
             support_list = [
-                op_node.qargs
-                for op_node in new_layer.op_nodes()
-                if not getattr(op_node.op, "_directive", False)
+                op_node.qargs for op_node in new_layer.op_nodes() if not op_node.is_directive()
             ]
 
             yield {"graph": new_layer, "partition": support_list}
@@ -2129,11 +2146,7 @@ def collect_runs(self, namelist):
         """
 
         def filter_fn(node):
-            return (
-                isinstance(node, DAGOpNode)
-                and node.op.name in namelist
-                and getattr(node.op, "condition", None) is None
-            )
+            return isinstance(node, DAGOpNode) and node.name in namelist and node.condition is None
 
         group_list = rx.collect_runs(self._multi_graph, filter_fn)
         return {tuple(x) for x in group_list}
@@ -2366,7 +2379,7 @@ def _may_have_additional_wires(node) -> bool:
     #
     # If updating this, you most likely also need to update `_additional_wires`.
     return node.condition is not None or (
-        not node.is_standard_gate and isinstance(node.op, (ControlFlowOp, Store))
+        not node.is_standard_gate() and isinstance(node.op, (ControlFlowOp, Store))
     )
 
 

qiskit/transpiler/passes/basis/basis_translator.py

@@ -323,7 +323,7 @@ def _replace_node(self, dag, node, instr_map):
                     node.cargs[target_dag.find_bit(x).index] for x in inner_node.cargs
                 )
 
-                if not new_node.is_standard_gate:
+                if not new_node.is_standard_gate():
                     new_node.op = new_node.op.copy()
                 if any(isinstance(x, ParameterExpression) for x in inner_node.params):
                     new_params = []
@@ -342,7 +342,7 @@ def _replace_node(self, dag, node, instr_map):
                                 new_value = new_value.numeric()
                             new_params.append(new_value)
                     new_node.params = new_params
-                    if not new_node.is_standard_gate:
+                    if not new_node.is_standard_gate():
                         new_node.op.params = new_params
                 dag._apply_op_node_back(new_node)
 
@@ -533,7 +533,7 @@ def edge_cost(self, edge_data):
 
         cost_tot = 0
         for instruction in edge_data.rule.circuit:
-            key = Key(name=instruction.operation.name, num_qubits=len(instruction.qubits))
+            key = Key(name=instruction.name, num_qubits=len(instruction.qubits))
             cost_tot += self._opt_cost_map[key]
 
         return cost_tot - self._opt_cost_map[edge_data.source]

qiskit/transpiler/passes/layout/apply_layout.py

@@ -13,7 +13,7 @@
 """Transform a circuit with virtual qubits into a circuit with physical qubits."""
 
 from qiskit.circuit import QuantumRegister
-from qiskit.dagcircuit import DAGCircuit
+from qiskit.dagcircuit import DAGCircuit, DAGOpNode
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.exceptions import TranspilerError
 from qiskit.transpiler.layout import Layout
@@ -79,7 +79,12 @@ def run(self, dag):
             virtual_physical_map = layout.get_virtual_bits()
             for node in dag.topological_op_nodes():
                 qargs = [q[virtual_physical_map[qarg]] for qarg in node.qargs]
-                new_dag.apply_operation_back(node.op, qargs, node.cargs, check=False)
+                new_dag._apply_op_node_back(
+                    DAGOpNode.from_instruction(
+                        node._to_circuit_instruction().replace(qubits=qargs)
+                    ),
+                    check=False,
+                )
         else:
             # First build a new layout object going from:
             # old virtual -> old physical -> new virtual -> new physical
@@ -99,7 +104,12 @@ def run(self, dag):
             # Apply new layout to the circuit
             for node in dag.topological_op_nodes():
                 qargs = [q[new_virtual_to_physical[qarg]] for qarg in node.qargs]
-                new_dag.apply_operation_back(node.op, qargs, node.cargs, check=False)
+                new_dag._apply_op_node_back(
+                    DAGOpNode.from_instruction(
+                        node._to_circuit_instruction().replace(qubits=qargs)
+                    ),
+                    check=False,
+                )
             self.property_set["layout"] = full_layout
             if (final_layout := self.property_set["final_layout"]) is not None:
                 final_layout_mapping = {