-
-
Notifications
You must be signed in to change notification settings - Fork 73
✨ Add an unroll-modifiers pass for unrolling multi-operation modifiers
#2015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
denialhaag
wants to merge
38
commits into
main
Choose a base branch
from
unroll-modifiers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+2,171
−259
Open
Changes from 8 commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
eee1742
✨ Add an `unroll-modifiers` pass for multi-operation modifiers
denialhaag 0cf9047
Address the Rabbit's comments
denialhaag 7707cba
Reject StaticOps in modifier bodies
denialhaag 61cb3d0
Clean up a bit
denialhaag 9ff2e9d
Log modifiers that cannot be unrolled
denialhaag 050511d
Fix linter errors
denialhaag 491e802
Share the unrolled ctrl and inv programs
denialhaag a52566b
Fix typo
denialhaag 9cb5201
Update changelog
denialhaag c505ade
Merge branch 'main' into unroll-modifiers
denialhaag d043d9b
✅ Reject qubit captures in QCO modifiers
burgholzer 65d12fc
🎨 Follow fixed-width integer style
burgholzer 7170cea
📝 Document QCO modifier capture rules
burgholzer ac7b14e
Clean up namespace qualifiers
denialhaag 31794d4
Merge remote-tracking branch 'origin/main' into unroll-modifiers
denialhaag 3f9ce38
Unroll pow modifiers acting on disjoint qubits
denialhaag 6a2a6f8
Implement RegionBranchOpInterface for QC and QCO modifiers
denialhaag b0cba26
Fix dangling values when inlining modifier bodies
denialhaag 8559d15
Revert "Implement RegionBranchOpInterface for QC and QCO modifiers"
denialhaag 216524b
Add canonicalization patterns for removing unused qubit block arguments
denialhaag da18dbc
Merge remote-tracking branch 'origin/main' into unroll-modifiers
denialhaag b76d5ac
Fix linter errors
denialhaag c3665b5
Add interface targets for the shared MLIR test headers
denialhaag 67be78c
Merge remote-tracking branch 'origin/main' into unroll-modifiers
denialhaag 18192a8
Merge origin/main into PR #2015 remediation branch
burgholzer 8f34843
Reduce unrelated test namespace churn
burgholzer 1f27829
Construct narrowed modifiers during unrolling
burgholzer 2901113
Reject nested non-unitary QCO modifier bodies
burgholzer cc2169a
Preserve QCO wire order when folding powers
burgholzer be8d32a
Avoid leaking QIR control state past no-op bodies
burgholzer 889041b
Assert unroll-modifier structure before cleanup
burgholzer c5e5815
Format modifier remediation changes
burgholzer 01e4c8e
Clean up modifier remediation includes
burgholzer 3701773
Merge current main into PR #2015 remediation branch
burgholzer 93c8cda
Preserve QCO wire order across all power folds
burgholzer 6344d70
Merge remote-tracking branch 'origin/main' into unroll-modifiers
denialhaag 5d77e5d
Clean up a bit
denialhaag 69e60fe
Fix linter errors
denialhaag File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit too narrow. The cases where this simplification holds are well known. IIRC, this holds for (positive?) integer powers. That should be easy to verify though with an LLM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something fundamental right now,$(XY)^2 = XYXY \neq XXYY = X^2Y^2$ , where $X$ and $Y$ are the respective Pauli matrices.
pow(r) { a; b } = pow(r) { a }; pow(r) { b }requires at least thataandbcommute, no? This should already be a simple enough counterexample:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can always expand
pow(r) { a; b }ifaandbare acting on different qubits, as they definitely commute then. 🤔 Should be easy enough to check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. I was definitely looking at this too quickly. You are right, and I would not overcomplicate the current implementation with Commutation Analysis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I encountered a related problem while extending
MergeSingleQubitRotationGatesto fixed gates in #1639.For the OpenQASM program
the merge pass turns the
h; sbody into auoperation plus a separategphase. This is unitary-equivalent outside a modifier, but insidepowit changes the body fromto roughly
That currently breaks the standard QC-to-QIR pipeline because the resulting multi-operation
powcannot be legalized.For an integer exponent, the phase could theoretically be handled with
but that would require an explicitly Pow-aware global-phase rewrite, as it is not the same as distributing the power over arbitrary body operations.
Is that something that should be taken care of in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3f9ce38 adds a pattern that unrolls a
PowOpif all body operations are disjoint.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably supersedes #2041, could you check if that is similar to what is covered by your pattern?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation here only splits
pow(2) { gphase(..); u(...) }intopow(2) { gphase(..) }; pow(2) { u(...) }, but it doesn't then inlinepow(2) { u(...) }. If I'm not missing something right now, I still see the value in #2041. 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay great, then I will keep that open 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 AI text below 🤖
Implemented the bounded version at
93c8cdabd:unroll-modifierssplits only body operations whose qubit sets are disjoint. Overlapping operations and non-integral powers remain untouched; no commutation analysis or generalpow @ usynthesis was added. Structural tests assert both the split and preservation cases before cleanup.