qir-alliance / qat

QIR compiler tools and optimization passes for targeting QIR to different hardware backends
https://qir-alliance.github.io/qat/
MIT License
26 stars 14 forks source link

Qat doesn't inline a function call even with --always-inline and --profile base specified #52

Closed peter-campora closed 2 years ago

peter-campora commented 2 years ago

Hi,

It seems like the --always-inline flag doesn't always force inlining in the generated QIR. I ran the following command via qat: ./qat --always-inline --apply -S --profile base MagicStateDistillation-10X.ll > MagicStateDistillation-BaseProfile.10X.ll on the attached file and received the following output in MagicStateDistillation-BaseProfile.10X.ll (I changed the ll files to text files to attach them to the issue). The entry point function in the output file, @Microsoft__Quantum__Samples__MeasureDistilledTAtDepth10InX__Interop, calls out to a helper function @Microsoft__Quantum__Samples__Distill__body.4.

MagicStateDistillation-10X.txt MagicStateDistillation-BaseProfile.10X.txt

Let me know if I should try again with additional arguments to qat, or whether you need any more information.

swernli commented 2 years ago

Thanks for reporting, @peter-campora! We will investigate and see if we can find out what inline fails on that example. @troelsfr FYI.

troelsfr commented 2 years ago

@peter-campora thanks for reporting. I will investigate this further.

If this is blocking you in any way, you can immediately solve the issue by replacing --always-inline with -O3 which provides more aggreesive optimisation. The full command is:

./qir/qat/Apps/qat -O3 --apply -S --profile base ~/Downloads/MagicStateDistillation-10X.txt > MagicStateDistillation-BaseProfile.10X.ll

I've attached the output MagicStateDistillation-BaseProfile.10X.txt. This should work with the master branch, but just in case that it does not, please try this one.

@swernli the main reason why this fails is because of our inline parameter which is used in the default pipeline, but was not added in the always-inline pass. I can add this, but I think we need a larger refactor of this section since always-inline implicitly disables the default pipeline. Let me know if you have any thoughts on this.

troelsfr commented 2 years ago

FYI, this issue should be solved in #53 - @swernli if you have time to review, then that would be much appreciated :)

swernli commented 2 years ago

Thanks @troelsfr for working on that! I'll review today.

troelsfr commented 2 years ago

53 is merged and I am closing this ticket. @peter-campora feel free to reopen if this does not solve the issue for you.

peter-campora commented 2 years ago

I'll take a look a look at the new version of qat and test it out! Thanks all! I found a new issue, so I'll see if any updates fix this new issue and if not, I'll file a new issue.