-
Notifications
You must be signed in to change notification settings - Fork 169
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
Draft of dependency graph generation algorithm #1848
Conversation
CLA Assistant Lite bot All Contributors have signed the CLA. |
I have read the Contributor License Agreement and I hereby accept the Terms. |
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'll have to try this out.
…er AST to reflect these reuses
Can be used like so: The additional The |
Command Bot: Processing... |
…m into dependency_analysis
…ncludes some fixes for bugs revealed by validation.
ConversionTarget target(*ctx); | ||
target.addLegalDialect<quake::QuakeDialect>(); | ||
target.addDynamicallyLegalOp<quake::NullWireOp>( | ||
[&](quake::NullWireOp alloc) { return alloc->hasAttr("qid"); }); |
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 attribute approach is brittle as other transformations will not necessarily keep the information. We should really use WireSet, BorrowWire, ReturnWire for explicitly mapping the identity of a qubit from an infinite space (virtual registers) to a finite space (physical registers).
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.
Makes sense, right now Assign IDs and Dependency Analysis are very tangled passes, so they pretty much have to be run one right after another, and Dependency Analysis has to check that the attributes are present.
Command Bot: Processing... |
… cases * quake.discriminate was incorrectly classified as a quantum op and would add a cycle * Reusing a wire didn't properly handle the case where the wire wasn't the first result * DependencyNodes were incorrectly put into the same graphs based on non-quantum-dependent classical values * Successors were ordered lists, but classical values are not linear types, so successors became sets Also removes the validate method for DependencyNodes as the remaining check are already asserted elsewhere.
…m into dependency_analysis
Closing in lieu of #2163 |
Description
This is a draft of an analysis to determine dependency information between qubit operations in a quake program.