✨ Add native gate synthesis MLIR pass and decomposition stack#1665
Draft
simon1hofmann wants to merge 44 commits intomainfrom
Draft
✨ Add native gate synthesis MLIR pass and decomposition stack#1665simon1hofmann wants to merge 44 commits intomainfrom
simon1hofmann wants to merge 44 commits intomainfrom
Conversation
…use `std::exp` for complex exponentiation instead of `std::polar`.
… basis decomposer, Euler decomposition, and associated helper functions. This update introduces new headers and source files for managing gate sequences, unitary matrices, and decomposition strategies, enhancing the framework's capabilities for quantum circuit transformations. Co-authored-by: Tamino Bauknecht <dev@tb6.eu>
… and additional test cases for Euler and Weyl decompositions.
…, including new utility functions and validation for gate sequences.
…xamples and execution details.
…dual global phases.
… synthesis utilities
simon1hofmann
added
enhancement
… clarifying examples and improving readability.
…d results in native synthesis tests.
…re both output and input qubits match correctly.
…pointers and ensure operand count validation for RZ, RX, RY, U, P, and R operations in native synthesis tests.
…ubit outputs and inputs, ensuring correct operation merging in XXMinusYY and XXPlusYY patterns.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp (2)
256-261:⚠️ Potential issue | 🔴 CriticalUse-after-erase:
it0/it1are dereferenced aftercloseBlockerases their buckets.
closeBlock(it0->second)erasesblock.wireAandblock.wireBfromwireToBlock, and sincev0(the key ofit0) equals one of those wires,it0is invalidated. The subsequentit0->second != it1->secondandcloseBlock(it1->second)then read invalidated iterators — UB byDenseMap::erase's contract. Capture both indices into locals before any close call.🐛 Proposed fix
- if (tracked0) { - closeBlock(it0->second); - } - if (tracked1 && (!tracked0 || it0->second != it1->second)) { - closeBlock(it1->second); - } + const size_t idx0 = tracked0 ? it0->second : 0; + const size_t idx1 = tracked1 ? it1->second : 0; + if (tracked0) { + closeBlock(idx0); + } + if (tracked1 && (!tracked0 || idx0 != idx1)) { + closeBlock(idx1); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp` around lines 256 - 261, The code dereferences iterators it0/it1 after calling closeBlock, which erases entries from wireToBlock and invalidates those iterators; to fix, read and store the needed block indices (e.g., auto b0 = it0->second; auto b1 = it1->second) and any boolean flags (tracked0/tracked1) into local variables before invoking closeBlock, then call closeBlock(b0) and closeBlock(b1) using those locals and only compare locals (b0 != b1) instead of it0/it1; update the logic around tracked0/tracked1 and the closeBlock calls accordingly (functions/variables: closeBlock, it0, it1, tracked0, tracked1, wireToBlock, block.wireA, block.wireB).
78-106:⚠️ Potential issue | 🟡 MinorEmission failure in
materializeSingleTwoQubitBlockis still not propagated.
emitTwoQubitGateSequenceAtLocfailure on Lines 91-96 only emits a diagnostic and returns;materialize()(Lines 325-345) and the upstream pass driver inPass.cppnever see a failure signal, so the pass cannot reliablysignalPassFailure()for this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp` around lines 78 - 106, The helper materializeSingleTwoQubitBlock currently swallows emitTwoQubitGateSequenceAtLoc failures by only emitting a diagnostic; change materializeSingleTwoQubitBlock to return a LogicalResult (or bool) and return failure when emitTwoQubitGateSequenceAtLoc fails, then update the caller materialize to check that return value and propagate failure (return failure) up so the pass driver (Pass.cpp) can call signalPassFailure(); update all call sites of materializeSingleTwoQubitBlock accordingly and ensure the function signature and callers consistently use the LogicalResult type and/or LLVM succeeded/failed helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp`:
- Line 32: The code uses std::ranges::reverse_view and includes <ranges>;
replace that with LLVM's utility by including "llvm/ADT/STLExtras.h", remove the
<ranges> include, and change uses of std::ranges::reverse_view (in
PassTwoQubitWindows.cpp, e.g., around the loop/iteration that references
reverse_view) to use llvm::reverse or llvm::reverse(iterable) as provided by
STLExtras so the code follows MLIR/LLVM style and drops the STL ranges
dependency.
---
Duplicate comments:
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp`:
- Around line 256-261: The code dereferences iterators it0/it1 after calling
closeBlock, which erases entries from wireToBlock and invalidates those
iterators; to fix, read and store the needed block indices (e.g., auto b0 =
it0->second; auto b1 = it1->second) and any boolean flags (tracked0/tracked1)
into local variables before invoking closeBlock, then call closeBlock(b0) and
closeBlock(b1) using those locals and only compare locals (b0 != b1) instead of
it0/it1; update the logic around tracked0/tracked1 and the closeBlock calls
accordingly (functions/variables: closeBlock, it0, it1, tracked0, tracked1,
wireToBlock, block.wireA, block.wireB).
- Around line 78-106: The helper materializeSingleTwoQubitBlock currently
swallows emitTwoQubitGateSequenceAtLoc failures by only emitting a diagnostic;
change materializeSingleTwoQubitBlock to return a LogicalResult (or bool) and
return failure when emitTwoQubitGateSequenceAtLoc fails, then update the caller
materialize to check that return value and propagate failure (return failure) up
so the pass driver (Pass.cpp) can call signalPassFailure(); update all call
sites of materializeSingleTwoQubitBlock accordingly and ensure the function
signature and callers consistently use the LogicalResult type and/or LLVM
succeeded/failed helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e5ef0ac-1ce5-4ceb-a498-d80981852c34
📒 Files selected for processing (1)
mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp (1)
47-47: 🧹 Nitpick | 🔵 TrivialPrefer
llvm::reverseoverstd::ranges::reverse_view.Per the project's coding guideline for
mlir/, use LLVM utilities. The same comment was raised onPassTwoQubitWindows.cppand addressed there (now usesllvm::reverse); please apply the same change here so the<ranges>include can be dropped.♻️ Proposed refactor
-#include <ranges> `#include` <utility>- for (auto& op : std::ranges::reverse_view(run.ops)) { - Operation* toErase = op.getOperation(); + for (auto& op : llvm::reverse(run.ops)) { + Operation* toErase = op.getOperation(); rewriter.eraseOp(toErase); }As per coding guidelines: "PREFER LLVM data structures and methods in
mlir/(llvm::SmallVector,llvm::function_ref, etc.) over the STL".Also applies to: 164-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp` at line 47, Replace uses of std::ranges::reverse_view in this file with llvm::reverse (e.g., wrap the container or iterator range with llvm::reverse(...) or use llvm::make_range + llvm::reverse) and remove the `#include` <ranges> line; update the code locations around the current usages (noted near lines 164-167) to call llvm::reverse on the existing container/range so LLVM utilities are used instead of std::ranges::reverse_view.mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.cpp (1)
372-380:⚠️ Potential issue | 🟠 MajorU3 matrix synthesis still emits a spurious global phase of
e^(i(φ+λ)/2).This is the same issue raised on a prior commit (
d98c2f50) and does not appear to have been addressed in the latest revision. After projectingmatrixto SU(2),m = RZ(angles[1])·RY(angles[0])·RZ(angles[2])(pure ZYZ Euler form). However, emittingu(angles[0], angles[1], angles[2])producesU(θ,φ,λ) = e^(i(φ+λ)/2)·RZ(φ)·RY(θ)·RZ(λ) = e^(i(angles[1]+angles[2])/2) · mper the project's OpenQASM 2 U-gate convention. The currentemitGPhaseIfNonTrivial(rewriter, loc, phase)only restores the determinant phase but leaves the U-gate intrinsice^(i(φ+λ)/2)factor uncompensated, so the emitted sequence equals the original unitary only up to global phase — contrary to the comment on Lines 369–371.This is masked by tests using
isEquivalentUpToGlobalPhase, but becomes observable when the synthesized op is wrapped in aCtrlOp(controlled-version constructions). Per the established convention in this repo (seeQuaternionMergeRotationGates), GPhase corrections must be unconditional.🛡️ Proposed fix
const auto angles = decomposition::EulerDecomposition::anglesFromUnitary( m, decomposition::EulerBasis::ZYZ); - emitGPhaseIfNonTrivial(rewriter, loc, phase); + // U(θ,φ,λ) = e^(i(φ+λ)/2)·RZ(φ)·RY(θ)·RZ(λ); subtract its intrinsic + // phase so the emitted GPhase + UOp pair matches `matrix` exactly. + emitGPhaseIfNonTrivial(rewriter, loc, + phase - (angles[1] + angles[2]) / 2.0); return e.u(inQubit, angles[0], angles[1], angles[2]);Please verify that
anglesFromUnitary(m, ZYZ)returns pure Euler angles (not U-gate-aware angles) — if it already accounts for the U-gate intrinsic phase, the existing code would be correct:#!/bin/bash # Inspect anglesFromUnitary to determine its angle convention for ZYZ. fd -t f 'EulerDecomposition\.cpp' mlir/ --exec cat fd -t f 'EulerDecomposition\.h' mlir/ --exec cat # Look for tests that pin down the convention rg -nP --type=cpp -C5 'anglesFromUnitary.*ZYZ'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.cpp` around lines 372 - 380, After projecting to SU(2) you use EulerDecomposition::anglesFromUnitary(m, EulerBasis::ZYZ) which returns raw ZYZ rotation angles, but emitting the OpenQASM-style U gate via e.u(...) implicitly multiplies by e^(i(φ+λ)/2), so the current emitGPhaseIfNonTrivial(rewriter, loc, phase) only restores det(m) and leaves the U-gate intrinsic global phase uncorrected; fix by adding the U-gate intrinsic phase to the emitted global phase before the U emission (i.e., compute extra = (angles[1] + angles[2]) / 2.0 and call emitGPhaseIfNonTrivial(rewriter, loc, phase + extra) prior to return e.u(inQubit, angles[0], angles[1], angles[2]); also verify (or assert in a comment) that decomposition::EulerDecomposition::anglesFromUnitary(...) indeed returns pure ZYZ Euler angles and not U-gate-aware angles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.h`:
- Around line 23-26: The three-line Doxygen block currently sitting between the
includes and namespace should be converted into a proper file-level doc comment
or moved to the specific API docs: either change it to a "/// \\file" style
header (matching PassTwoQubitWindows.h) placed at the top of the file before the
includes, or delete the dangling block and fold its text into the doc comments
for the relevant functions: decomposeToZSXX,
computeSynthesizedSingleQubitLength, and emitSynthesizedSingleQubitFromMatrix;
ensure the final comment attaches to the intended symbol (file-level or the
three functions) rather than to namespace mlir::qco::native_synth and update
wording to match existing project Doxygen style.
In `@mlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/TwoQubit.h`:
- Around line 44-57: The SmallVector inline-storage is inconsistent: change
collectSingleQubitCandidates to explicitly use
llvm::SmallVector<SynthesisCandidate<SingleQubitRewritePlan>, 0> so its
signature matches the two-qubit collectors
(collectTwoQubitBasisCandidatesFromMatrix and collectTwoQubitBasisCandidates);
update the declaration of collectSingleQubitCandidates accordingly to pin N=0
for predictable stack footprint and consistent API.
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp`:
- Around line 661-673: The code conflates two failure modes: when
selectBestCandidate(...) returns nullptr and when emitTwoQubitGateSequence(...)
returns failure; update the control flow around selectBestCandidate,
emitTwoQubitGateSequence, and the ctrl.emitError call so each path emits a
distinct error message — e.g., if selectBestCandidate(...) is nullptr call
ctrl.emitError("controlled gate not allowed by selected profile"), else (a
candidate existed but emitTwoQubitGateSequence failed) call
ctrl.emitError("failed to emit two-qubit gate sequence for selected candidate")
— keep using rewriter.setInsertionPoint(ctrl) where needed and target the
emitTwoQubitGateSequence(...) failure branch to report the emission/synthesis
error separately.
In
`@mlir/unittests/Dialect/QCO/Transforms/Decomposition/test_decomposition_helpers.cpp`:
- Around line 11-18: Add a direct include of Eigen/Core to make the test
self-contained: update the includes in test_decomposition_helpers.cpp so that
Eigen::Matrix2cd (used in the test at the places referencing Matrix2cd) is
available even if Helpers.h stops including Eigen; specifically add an `#include`
for Eigen/Core near the top alongside the existing includes so uses of
Eigen::Matrix2cd compile independently of transitive includes from Helpers.h.
In
`@mlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_custom_menus.cpp`:
- Around line 169-172: The function onlyAllowsMenuNativeOps currently treats any
qco::RXOp as allowed if spec.allowX && spec.allowSX && spec.allowRZ are true;
tighten this by allowing non-allowRX rx operations only when the RXOp is exactly
the canonical X-equivalent (angle == ±π) — otherwise require spec.allowRX.
Update the RX check in onlyAllowsMenuNativeOps to inspect the qco::RXOp's angle
(use the RXOp accessor, e.g., getAngle()/angle attribute or operand representing
the rotation) and permit it under the X/SX/RZ fallback only when the angle
equals ±M_PI (or canonical ±π value representation used in the codebase); all
other RXOps must fail unless spec.allowRX is true.
In
`@mlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_multi_qubit.cpp`:
- Around line 71-107: In verifyEquivalentAcrossProfiles, hoist the repeated
ternary that builds the circuit-name message into a single local std::string
prefix at the start of the for-loop iteration (use circuitName != nullptr ?
std::string("circuit=") + circuitName + " " : "" to build it), then replace the
four inline ternaries used in the ASSERT_TRUE/EXPECT_TRUE messages (the checks
after computeNQubitUnitaryFromModule, runNativeSynthesis/isNative, and
isEquivalentUpToGlobalPhase) with this prefix + "native-gates=" <<
profileCase.nativeGates so each assertion uses the shared prefix variable
instead of duplicating the ternary.
---
Duplicate comments:
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp`:
- Line 47: Replace uses of std::ranges::reverse_view in this file with
llvm::reverse (e.g., wrap the container or iterator range with
llvm::reverse(...) or use llvm::make_range + llvm::reverse) and remove the
`#include` <ranges> line; update the code locations around the current usages
(noted near lines 164-167) to call llvm::reverse on the existing container/range
so LLVM utilities are used instead of std::ranges::reverse_view.
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.cpp`:
- Around line 372-380: After projecting to SU(2) you use
EulerDecomposition::anglesFromUnitary(m, EulerBasis::ZYZ) which returns raw ZYZ
rotation angles, but emitting the OpenQASM-style U gate via e.u(...) implicitly
multiplies by e^(i(φ+λ)/2), so the current emitGPhaseIfNonTrivial(rewriter, loc,
phase) only restores det(m) and leaves the U-gate intrinsic global phase
uncorrected; fix by adding the U-gate intrinsic phase to the emitted global
phase before the U emission (i.e., compute extra = (angles[1] + angles[2]) / 2.0
and call emitGPhaseIfNonTrivial(rewriter, loc, phase + extra) prior to return
e.u(inQubit, angles[0], angles[1], angles[2]); also verify (or assert in a
comment) that decomposition::EulerDecomposition::anglesFromUnitary(...) indeed
returns pure ZYZ Euler angles and not U-gate-aware angles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b833797-b55a-4d87-8557-b5c2c70d73fe
📒 Files selected for processing (28)
mlir/include/mlir/Compiler/CompilerPipeline.hmlir/include/mlir/Dialect/QCO/Transforms/Decomposition/GateSequence.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/NativeSpec.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/TwoQubit.hmlir/lib/Compiler/CMakeLists.txtmlir/lib/Compiler/CompilerPipeline.cppmlir/lib/Dialect/QCO/Transforms/CMakeLists.txtmlir/lib/Dialect/QCO/Transforms/Decomposition/BasisDecomposer.cppmlir/lib/Dialect/QCO/Transforms/Decomposition/EulerDecomposition.cppmlir/lib/Dialect/QCO/Transforms/Decomposition/Helpers.cppmlir/lib/Dialect/QCO/Transforms/Decomposition/UnitaryMatrices.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Policy.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/TwoQubit.cppmlir/tools/mqt-cc/mqt-cc.cppmlir/unittests/Dialect/QCO/Transforms/CMakeLists.txtmlir/unittests/Dialect/QCO/Transforms/Decomposition/decomposition_test_utils.hmlir/unittests/Dialect/QCO/Transforms/Decomposition/test_basis_decomposer.cppmlir/unittests/Dialect/QCO/Transforms/Decomposition/test_decomposition_helpers.cppmlir/unittests/Dialect/QCO/Transforms/Decomposition/test_weyl_decomposition.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_policy.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_custom_menus.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_multi_qubit.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_scoring.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp`:
- Around line 698-725: The XX±YY->RZZ branch unconditionally short-circuits when
rzz is allowed, preventing weights-based comparison with general candidates;
instead, construct a unified candidates list that includes the XxPlusMinusViaRzz
candidate (use xxPlusMinusYyRzzRewriteScoringMetrics and payload=true as before)
along with the results of collectTwoQubitBasisCandidates(unitary, spec), then
call selectBestCandidate(...) on that combined ArrayRef with weights to pick the
winner; after selection, set the insertion point and either call
rewriteXXPlusMinusYYViaRzz(rewriter, op) if the chosen CandidateClass is
XxPlusMinusViaRzz or emitTwoQubitGateSequence(...) using best->payload.sequence
for general two-qubit candidates so the specialization only wins when its
weighted score is best.
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp`:
- Around line 111-120: The check currently skips ops only when their immediate
parent is a CtrlOp; change it to skip any op that has a CtrlOp anywhere in its
ancestor chain by replacing the immediate-parent test with an ancestor lookup
(e.g. use op->getParentOfType<CtrlOp>() or similar) inside
collectUnitaryOpsInPreOrder (and the analogous filter around lines 161-168) so
any op nested under a CtrlOp (like qco.ctrl { qco.inv { ... } }) is ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89fb31fc-0ac8-42b3-89e2-4b33d9e9b009
📒 Files selected for processing (9)
mlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/SingleQubit.hmlir/include/mlir/Dialect/QCO/Transforms/NativeSynthesis/TwoQubit.hmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/TwoQubit.cppmlir/unittests/Dialect/QCO/Transforms/Decomposition/test_decomposition_helpers.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_custom_menus.cppmlir/unittests/Dialect/QCO/Transforms/NativeSynthesis/test_native_synthesis_pass_multi_qubit.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cpp`:
- Around line 484-499: The in-place mutation
rz1.getThetaMutable().assign(newTheta) must be wrapped with the rewriter's
modifyOpInPlace to trigger operation-listener notifications; locate the block
manipulating rz1 (symbols: rz1, getThetaMutable, newTheta) and replace the bare
assign with a call to rewriter.modifyOpInPlace(rz1, ...) that performs the
assign inside the provided lambda, keeping the same computation path
(valueToDouble checks, constantFromScalar or arith::AddFOp creation,
rewriter.replaceOp(partner, ...)) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8399e11a-f1da-4511-948b-ca34bde0a3ae
📒 Files selected for processing (4)
mlir/include/mlir/Dialect/QCO/Transforms/Decomposition/BasisDecomposer.hmlir/lib/Dialect/QCO/Transforms/Decomposition/BasisDecomposer.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/Pass.cppmlir/lib/Dialect/QCO/Transforms/NativeSynthesis/PassTwoQubitWindows.cpp
Contributor
Author
|
@burgholzer
Planned dependency chain: PR1 -> PR2 -> PR3 -> PR4 -> PR5 (each PR based on the previous one). Does this split and order look good, or would you prefer a different grouping/order? |
Member
That order looks reasonable to me. If it can be split up like that, I would certainly appreciate it! 👍🏼 |
11 tasks
…::polar for complex exponentiation, improving numerical stability and readability.
Signed-off-by: simon1hofmann <119581649+simon1hofmann@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.h`:
- Around line 42-43: Add a Doxygen comment above the public factory declaration
createNativeGateSynthesisPass describing its purpose (what the pass does),
documenting the parameter `@param` options (type NativeGateSynthesisOptions) and
any important semantics of those options, and the return value via `@return` (a
std::unique_ptr<Pass> owning the created pass). Keep wording consistent with
surrounding pass factory comments (brief one-line summary plus `@param/`@return
tags) and place it immediately above the declaration for
createNativeGateSynthesisPass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 468c7c0e-7e17-4ed8-8a53-dd3ca91801ad
📒 Files selected for processing (4)
mlir/include/mlir/Dialect/QCO/Transforms/Passes.hmlir/lib/Compiler/CompilerPipeline.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/XXMinusYYOp.cppmlir/lib/Dialect/QCO/IR/Operations/StandardGates/XXPlusYYOp.cpp
Comment on lines
+42
to
+43
| std::unique_ptr<Pass> | ||
| createNativeGateSynthesisPass(const NativeGateSynthesisOptions& options); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add Doxygen on the public pass factory declaration.
The new public declaration should be documented like the surrounding API surface.
Suggested patch
+/// Create the native gate synthesis pass configured with explicit options.
+///
+/// `@param` options Native gate menu and scoring weights.
+/// `@returns` A pass instance that lowers supported unitaries to the selected
+/// native gate set.
std::unique_ptr<Pass>
createNativeGateSynthesisPass(const NativeGateSynthesisOptions& options);As per coding guidelines, **/*.{cpp,cc,cxx,c,h,hpp,hxx}: MUST use Doxygen-style comments in C++ code.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| std::unique_ptr<Pass> | |
| createNativeGateSynthesisPass(const NativeGateSynthesisOptions& options); | |
| /// Create the native gate synthesis pass configured with explicit options. | |
| /// | |
| /// `@param` options Native gate menu and scoring weights. | |
| /// `@returns` A pass instance that lowers supported unitaries to the selected | |
| /// native gate set. | |
| std::unique_ptr<Pass> | |
| createNativeGateSynthesisPass(const NativeGateSynthesisOptions& options); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.h` around lines 42 - 43, Add
a Doxygen comment above the public factory declaration
createNativeGateSynthesisPass describing its purpose (what the pass does),
documenting the parameter `@param` options (type NativeGateSynthesisOptions) and
any important semantics of those options, and the return value via `@return` (a
std::unique_ptr<Pass> owning the created pass). Keep wording consistent with
surrounding pass factory comments (brief one-line summary plus `@param/`@return
tags) and place it immediately above the declaration for
createNativeGateSynthesisPass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Introduces a
native-gate-synthesistransform on QCO modules that lowers arbitrary supported unitaries to a comma-separated native gate menu, with scoring over two-qubit count, single-qubit count, and local depth.User-visible behavior
--native-gates=<menu>onmqt-cc(empty menu = no-op), aligned with pass options documented inPasses.td(native-gates,score-weight-twoq,score-weight-oneq,score-weight-depth).Fixes #(issue)
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.