MLIR mqt dialect conversion

[li-mingbao]

This PR introduces the dialect conversion between the MQTOpt dialect and the MQTDyn dialect of mqtcore in both directions.

Checklist:

• The pull request only contains commits that are focused and relevant to this change.
• I have added appropriate tests that cover the new/changed functionality.
• I have updated the documentation to reflect these changes.
• I have added entries to the changelog for any noteworthy additions, changes, fixes or removals.
• I have added migration instructions to the upgrade guide (if needed).
• The changes follow the project's style guidelines and introduce no new warnings.
• The changes are fully tested and pass the CI checks.
• I have reviewed my own code changes.

[ystade]

Thanks, @li-mingbao for all your efforts that you put in here. This already looks very good. Thank you as well for already adding tests for both directions. Nevertheless, I have some minor comments that are inline. You'll find them below and in the "Files Changed" tab.

One thing that I did not mention in the comments in the code and what I want to mention here is the following: For the direction from the Dyn to Opt Dialect, the inputs of the subsequent operations of an operation get updated multiple times, leading to a complexity of O(n^2). However, in theory it should suffice, to update the inputs for every operation just once. I think it would be worth to invest some more hours here to try whether this is possible in MLIR.

Finally, thanks again for all your efforts again and feel free to reach out if you have any questions.

[li-mingbao]

Thanks @ystade for the detailed feedback. I appreciate it.
I applied most of the requested changes but I am not done yet (still missing the descriptions and the tests). I hope I understood them correctly, otherwise just let me know.
The main part that I changed is how the mqtdyn to mqtopt conversion works.
Instead of the loops I created Densemaps to store the dynQubits and the reference to its latest optQubit. The same thing applies for the qubit registers. Since I need to keep track of some additional informations for the opt insertOperation later on, I also created a small container struct to store additional data for qubits but I am not sure if this is the best approach.
Also, since Densemap which apparently is the preferred way to store mlir::Values, are unordered maps and I iterate over them to create the mqtopt insert operation the order of insertOperations is non-deterministic and I am not sure how to write test for that. Besides that it should work and the operands are set correctly.
I will gladly take any feedback on that.

Edit: The mqtdyn-to-mqtopt conversion doesnt work in the ci and has an unrealized conversion but it seems to work for me locally. Edit2: Should be fixed now. It was an issue with llvm-19 that is not present in llvm-20.

[ystade]

@li-mingbao thanks a lot for the changes. I did not review everything in detail again. I will do that latest on Monday. May I ask you to click on the resolve button for every conversation you think it is resolved. Since most of them are outdated, it is for me quite tedious to check whether they are resolved or not. Since you know what you did, you should be able to quickly scroll through them and resolve everything that you worked on.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

[ystade]

Hi @li-mingbao ,

Sorry, for the delay on my end, I said I will do it yesterday but I did not get around to. Thanks for all the changes. In my opinion, the conversions improve a lot since last iteration. In particular the use of the macros and the modifed substituion of qubits in the conversion from dynamic to opt. I have some minor questions and comments that you find inline. If you find a comment only in one conversion please also check whether the same comment may apply for the other conversion as well. Thanks again for your efforts!

[ystade]

Out of curiosity, have you seen that somewhere else or did you come up with these getters on your own?

[li-mingbao]

I initially tried to do it with just static variables but this didnt work, so I looked up the best way to create global variables and this is what I found. I am not sure if this is the best way and I will gladly take any feedback on how to do it better.

[ystade]

For static variables, that is definitely the best way. However, I find it still not the most object-oriented way. I was wondering whether LLVM and MLIR in particular provide some framework/infrastructure to facilitate the same functionality as you/we are definitely not the first ones who need to maintain information across different passes. So I was wondering whether you found this in another MLIR conversion as I would be interested whether there exist some best practices one could make use of.

[DRovara]

Running the risk of making a misleading suggestion again:
Rather than using global/static variables which would likely break if we tried to run a conversion multiple times(?), wouldn't it make sense to store references to these maps as member variables, where the references are passed in the constructor?

The mqt-core roundtrip pass does something similar:
https://github.com/munich-quantum-toolkit/core/blob/main/mlir/lib/Dialect/MQTOpt/Transforms/ToQuantumComputationPattern.cpp#L44
Here, a reference to a qc::QuantumComputation is passed to the constructor of the RewritePattern. The reference can then be modified inside the pattern and then used outside.

In this case, maybe one can pass references to all required maps in the constructors of the corresponding ConversionPatterns to have shared instances without using global variables.

[ystade]

Thanks @ystade for the detailed feedback. I appreciate it. I applied most of the requested changes but I am not done yet (still missing the descriptions and the tests). I hope I understood them correctly, otherwise just let me know. The main part that I changed is how the mqtdyn to mqtopt conversion works. Instead of the loops I created Densemaps to store the dynQubits and the reference to its latest optQubit. The same thing applies for the qubit registers. Since I need to keep track of some additional informations for the opt insertOperation later on, I also created a small container struct to store additional data for qubits but I am not sure if this is the best approach. Also, since Densemap which apparently is the preferred way to store mlir::Values, are unordered maps and I iterate over them to create the mqtopt insert operation the order of insertOperations is non-deterministic and I am not sure how to write test for that. Besides that it should work and the operands are set correctly. I will gladly take any feedback on that.

Edit: The mqtdyn-to-mqtopt conversion doesnt work in the ci and has an unrealized conversion but it seems to work for me locally. Edit2: Should be fixed now. It was an issue with llvm-19 that is not present in llvm-20.

In principle, inserting the insert operations unordered is not a big deal. I would think that there must be a way to write LIT tests that test whether a function exists somewhere ignoring the particular order. I will have a look later this week whether I find something and will let you know. If you are faster that's also fine ;)

[li-mingbao]

Thanks again for the feedback @ystade. The test for mqtdyn-to-mqtopt should work now although the solution is not optimal. Also, I set the pr to draft as it needs more work than I expected after some testing.
The issue is that the conversion approach from mqtdyn is still not working as I want. With the revisioned approach I match the current dyn operation. look up the opt operands in the map, replace it with the opt operation and update the map. This causes issues with call and returnlike operations as I do not explicitly match them and replace their operands. Also, there are some cases in the opt dialect where a result is used more than once, for example if you have an if else branch without a block argument and therefore the entries in the map should not be updated.
This makes the conversion not as trivial as I thought and I have to reconsider my approach again.

[ystade]

Oh, that is actually a bigger issue that you encountered there. For now, I would propose to follow a simple appraoch that only works for programs not containing any function calls where quantum resources are involved. Do you think that is feasible with the current appraoch or small modifications?

For reference: The issue in the conversion of functions when converting from the dyn to opt dialect is illustrated by the following example pseudo code:

fun foo(q : QubitRef):
    x(q)

fun main():
    ...
    q = extract(qreg, 0)
    foo(q)
    h(q)
    ...

This simple program in the dyn dialect would need to be converted to:

fun foo(q : Qubit):
    q' = x(q)
    return q'

fun main():
    ...
    qreg', q = extract(qreg, 0)
    q' = foo(q)
    q'' = h(q')
    ...

However, this conversion, i.e., changing the return type of the function foo is non-trivial. Especially, if the function foo already has other return values. @DRovara @flowerthrower Do you have any thoughts on that issue?

[DRovara]

However, this conversion, i.e., changing the return type of the function foo is non-trivial. Especially, if the function foo already has other return values. @DRovara @flowerthrower Do you have any thoughts on that issue?

That's a good point. In my opinion, we will have to consider this situation for function calls and similar stuff for the MQTOpt dialect anyways. Consider for instance this situation in the quantum-sink transformation.
It isn't working directly with function calls, but it uses labels and jump operations for a similar idea.
Here, certain optimisations may also lead to changes in required parameters.

So, while it is not entirely the same situation, I believe discussing/finding a solution for these general cases may also help us decide what to do in the conversions.

[li-mingbao]

The mqtdyn conversion should work for programs that only use quantum types for the defined mqtdyn operations as these operations are matched. So the approach works for the mqtdyn programs that are currently in the tests. Do I have to consider any additional cases that uses any other structures for now? Once any other operation like func.return or cf.cond_br use quantum operands, the conversion fails.
For example this code here will have the same issue as when you make a function call:

    cf.cond_br %i, ^bb1, ^bb2
  ^bb1: 
    mqtdyn.x() %q1
    cf.br ^bb3
  ^bb2:  
    mqtdyn.y() %q1
    cf.br ^bb3
  ^bb3: 

In the opt dialect these jumps would need the qubits as arguments and the block arguments also needs to be added and the operand for the operation needs to be the block argument.

[ystade]

@li-mingbao Please finish this PR for the simple case for now. We will discuss internally how we want to solve the more complicated issues in general next week. For you, @li-mingbao , I think it makes more sense to focus on the simple case here and then set up the translation from and to QIR. I hope that sounds good to you. Let me know if you want to investigate something else. We are quite flexible here.

[ystade]

@li-mingbao to be precise, you can ignore branching and function calls for now and just assume one main function with one code block containing quantum instructions.