Skip to content
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

dialects: Add qref dialect #2769

Merged
merged 1 commit into from
Jun 25, 2024
Merged

dialects: Add qref dialect #2769

merged 1 commit into from
Jun 25, 2024

Conversation

alexarice
Copy link
Collaborator

Adds a qref dialect to xdsl, a variant of the qssa dialect which stores qubits as references rather than values.

@AntonLydike if you are interested

@alexarice alexarice added the dialects Changes on the dialects label Jun 24, 2024
@alexarice alexarice requested a review from superlopuh June 24, 2024 16:28
@alexarice alexarice self-assigned this Jun 24, 2024
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.80%. Comparing base (0f6558a) to head (6358532).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2769      +/-   ##
==========================================
- Coverage   89.81%   89.80%   -0.01%     
==========================================
  Files         372      373       +1     
  Lines       47817    47884      +67     
  Branches     7331     7340       +9     
==========================================
+ Hits        42945    43004      +59     
- Misses       3736     3742       +6     
- Partials     1136     1138       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice and simple, I like it!

I wonder if it'd be possible to generate the op definitions from some sort of spec in the future. How many different operations are there in quantum? is it just the handful of operations in this dialect, or are there tens or hundreds of them?

@alexarice
Copy link
Collaborator Author

I wonder if it'd be possible to generate the op definitions from some sort of spec in the future. How many different operations are there in quantum? is it just the handful of operations in this dialect, or are there a tens or hundreds of them?

I did wonder if it would be worth generating the operations in tandem, but @superlopuh advised against doing this.

There will likely be more operations than this, and I'm not certain on the best way to represent these yet (i.e. should they each be a separate operations or the same operation that refers to some symbol?)

I think this should be considered in the future but is likely not too relevant for this pr?

@n-io
Copy link
Collaborator

n-io commented Jun 24, 2024

There will likely be more operations than this, and I'm not certain on the best way to represent these yet (i.e. should they each be a separate operations or the same operation that refers to some symbol?)

I think this should be considered in the future but is likely not too relevant for this pr?

It depends what and how much they share? The arith and csl dialects have plenty of 1-line ops with shared base classes. The idea is to have one class per emitted op.

@AntonLydike
Copy link
Collaborator

I think this should be considered in the future but is likely not too relevant for this pr?

💯 Absolutely! Just a random thought of mine!

@n-io n-io self-requested a review June 24, 2024 16:54
Copy link
Collaborator

@n-io n-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having had a very quick look, it appears that these might be the same ops as defined on qssa, but operating on different data types. Why have this as a separate dialect, rather than supporting e.g. an additional QRefAttr type on the ops?

@alexarice
Copy link
Collaborator Author

Having had a very quick look, it appears that these might be the same ops as defined on qssa, but operating on different data types. Why have this as a separate dialect, rather than supporting e.g. an additional QRefAttr type on the ops?

The (gate-like) operations in this dialect also have no results. In future a verify pass could(/should) be added to qssa to check that the qubits there are used linearly, but such a verification should not apply to this dialect.

@superlopuh
Copy link
Member

I'm not sure how well the unification would work, and how much it might slow down making additions in the future. I think this is a great first thing to merge.

Copy link
Collaborator

@n-io n-io left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (gate-like) operations in this dialect also have no results.

That's a good point. Shared semantics or not, the prospects of having the same 'if' in every method does indeed sound undesirable.

In future a verify pass could(/should) be added to qssa to check that the qubits there are used linearly, but such a verification should not apply to this dialect.

Is there any way the two flavours can co-exist in a manner not intended, and it leading to undesirable side-effects?

@alexarice
Copy link
Collaborator Author

Is there any way the two flavours can co-exist in a manner not intended, and it leading to undesirable side-effects?

Not with the current registered operations I believe

@alexarice alexarice merged commit ea6f4d9 into main Jun 25, 2024
10 checks passed
@alexarice alexarice deleted the alexarice/qref-dialect branch June 25, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants