dialects: Add qssa dialect

[alexarice]

Add a simple quantum ssa dialect and a simple roundtrip filecheck test. Thought it would be better to split the work I was doing into multiple pull requests to make it easier to review so this doesn't have any rewrites yet

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.78%. Comparing base (2d2e3e5) to head (625227c).
Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2746      +/-   ##
==========================================
- Coverage   89.87%   89.78%   -0.10%     
==========================================
  Files         367      370       +3     
  Lines       47442    47617     +175     
  Branches     7228     7299      +71     
==========================================
+ Hits        42638    42751     +113     
- Misses       3694     3730      +36     
- Partials     1110     1136      +26     

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

[AntonLydike]

Nice stuff! 🦄

[alexarice]

Am I good to merge or should I ask for more reviews?

[AntonLydike]

Maybe @n-io can give a thumbs up as well? Otherwise I don't know who'd care about a qssa dialect.

General rule of thumb is that you want to be somewhat sure that anyone interested or invested had the chance to say okay or no. If nobody said anything, that's not enough. Sasha goes along the lines of one check is enough, I usually try to get two or more.

[n-io]

Looks good, some minor comments (somehow didn't click submit review, apologies)

[n-io]

num, count, num_qubits or similar might be better for inferring the type from a point of readability, since the definition qubit = QubitAttr() would suggest that qubits : Sequence[QubitAttr]

[AntonLydike]

Actually, looking at this again, I don't think we need this attribute at all. The number of qubits is simply the number of results. So the attribute can be dropped entirely

[n-io]

I agree. I'd generally encourage to print some type information in the mlir, though.

[n-io]

From the semantics, this reads like it allocates a series of single qubits, rather than an array of qubits as the op is described in the paper. You could consider parameterising QubitAttr similar to IntegerAttr, or maybe @AntonLydike could advise?

[AntonLydike]

What I was saying is that these two ops are both completely valid, just one has redundant information:

@irdl_op_definition
class QubitAllocOp(QubitBase):
    name = "qssa.alloc"

    qubits = prop_def(IntegerAttr)

    res: VarOpResult = var_result_def(qubit)

and

@irdl_op_definition
class QubitAllocOp(QubitBase):
    name = "qssa.alloc"

    res: VarOpResult = var_result_def(qubit)

Simply because we know that qbits == len(res). Therefore we don't need to carry this additional information at all. The resulting IR would look like this:

%qbit1, %qbit2 = qssa.alloc
%qbit3 = qssa.alloc

So you just specify how many qbits you allocate by the number of results.

[n-io]

For reference: 3.3.1 Allocation. qssa.alloc allocates an array of qubits of fixed or variable length. %q = qssa.alloc : qubit<10>

[AntonLydike]

I think @alexarice does not talk about qbit arrays, but models each qbit as a single SSA value. But that's for him to answer 😄

[n-io]

I seem to remember that semantically, an array of qubits may be understood to mean something different as compared to n single qubits, but I may be mistaken here.

[alexarice]

This is not directly based on the qssa dialect from the paper and is currently representing each individual qubit as a separate ssa value. I'd be open to renaming this dialect if this will cause confusion in the future

[alexarice]

The rest of the discussion didn't load for me on github, but I believe dropping the parameter is not ideal as the parser does not have access to the number of results (at least from my testing)

[n-io]

Suggestions on how to avoid storing the number of qubits explicitly, while still printing and parsing the number as before.

[alexarice]

Suggestions on how to avoid storing the number of qubits explicitly.

Is there a reason this is desirable? It feels strange to me to have a disconnect between the printed data and the data stored on the operation

[AntonLydike]

I would recommend to not print the count at all. Just rely on people being able to count the number of results?

[AntonLydike]

Suggestions on how to avoid storing the number of qubits explicitly.

Is there a reason this is desirable? It feels strange to me to have a disconnect between the printed data and the data stored on the operation

I generally agree, I think printing derived (redundant) data is not the best idea... I'd just not print the count at all

[alexarice]

I would recommend to not print the count at all. Just rely on people being able to count the number of results?

you then can't parse the operation though

[AntonLydike]

I would recommend to not print the count at all. Just rely on people being able to count the number of results?

you then can't parse the operation though

I think you should be able to. I'll come over

[AntonLydike]

@alexarice and I looked into the parser internals and spoke with Markus on how MLIR handles it, and sadly we have to print redundant information here (the number of results), as MLIR allows one to write qssa.alloc<4> without specifying the results, if they are not used. This is imo not great, and makes me wrong in saying "I think you should be able to. I'll come over"

[n-io]

Looks good :)