Fix data-flow analysis in control-flow jump/mark #1666
Merged
Conversation
This file contains 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
During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information. The tree-like form within `QuantumCircuit` implicitly creates a basic-block structure, which groups the whole control-flow body into a single block that must be executed atomicly (as seen by the outer circuit). When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places, nor allow unrelated instructions to appear between jump/mark pairs if they were no originally in that control-flow block. There are two components here; the former is solved by ensuring the jump/mark instructions span the full data width including clbits, and the latter is solved by placing full circuit width barriers around each control-flow logical component.
jakelishman
added
the
Changelog: Bugfix
hhorii
approved these changes
Nov 25, 2022
There was a problem hiding this comment.
During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information.
Decomposing an instruction that has a sub-circuit is more complicated than I expected. Thank you for your fix.
When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places,
This also makes sense to me.
hhorii
added
the
stable-backport-potential
|
Yeah, I didn't think about it at first either. It's on my mind now because I'd been discussing with Kevin (@kdk) about better data structures for representing control-flow graphs alongside data-flow ones, and this was one of the major problems we identified. |
hhorii
pushed a commit
to hhorii/qiskit-aer
that referenced
this pull request
Nov 28, 2022
* Fix data-flow analysis in control-flow jump/mark During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information. The tree-like form within `QuantumCircuit` implicitly creates a basic-block structure, which groups the whole control-flow body into a single block that must be executed atomicly (as seen by the outer circuit). When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places, nor allow unrelated instructions to appear between jump/mark pairs if they were no originally in that control-flow block. There are two components here; the former is solved by ensuring the jump/mark instructions span the full data width including clbits, and the latter is solved by placing full circuit width barriers around each control-flow logical component. * Fix lint
hhorii
pushed a commit
to hhorii/qiskit-aer
that referenced
this pull request
Nov 29, 2022
* Fix data-flow analysis in control-flow jump/mark During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information. The tree-like form within `QuantumCircuit` implicitly creates a basic-block structure, which groups the whole control-flow body into a single block that must be executed atomicly (as seen by the outer circuit). When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places, nor allow unrelated instructions to appear between jump/mark pairs if they were no originally in that control-flow block. There are two components here; the former is solved by ensuring the jump/mark instructions span the full data width including clbits, and the latter is solved by placing full circuit width barriers around each control-flow logical component. * Fix lint
hhorii
pushed a commit
to hhorii/qiskit-aer
that referenced
this pull request
Nov 29, 2022
* Fix data-flow analysis in control-flow jump/mark During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information. The tree-like form within `QuantumCircuit` implicitly creates a basic-block structure, which groups the whole control-flow body into a single block that must be executed atomicly (as seen by the outer circuit). When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places, nor allow unrelated instructions to appear between jump/mark pairs if they were no originally in that control-flow block. There are two components here; the former is solved by ensuring the jump/mark instructions span the full data width including clbits, and the latter is solved by placing full circuit width barriers around each control-flow logical component. * Fix lint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
During the inlining of control-flow into jump/mark instructions, the data-flow graph was losing information. The tree-like form within
QuantumCircuitimplicitly creates a basic-block structure, which groups the whole control-flow body into a single block that must be executed atomicly (as seen by the outer circuit). When inlining the body, this is no longer the case, and we have to take more care to ensure that topological iteration through the data-flow graph does not accidentally move jump/mark instructions into incorrect places, nor allow unrelated instructions to appear between jump/mark pairs if they were no originally in that control-flow block.There are two components here; the former is solved by ensuring the jump/mark instructions span the full data width including clbits, and the latter is solved by placing full circuit width barriers around each control-flow logical component.
Details and comments
Fix #1665.