Updates from main and disable C# generation when QIR is generated #896
Conversation
* [WIP] Started updating VS LSP client. * A few more nullability fixes. * Use explicit 0:0 range instead of relying on constructor. * Propagate nullability metadata to F# callers. * Avoid calling unsafe constructors. * Avoid deprecated Assert.Equals. * Set DocumentRangeFormattingProvider to false. * Fix to AssertCapability. * Assertions should work more reliabily when capabilities are null. * Disable nullable reference checking when copying unsound types. * Check if Range is null, since nullability metadata can be violated. Co-authored-by: Ricardo Espinoza <ricardoe@microsoft.com>
bettinaheim
changed the title
bettinaheim
requested review from
ScottCarda-MS,
bamarsha,
cesarzc,
kuzminrobin and
swernli
| ``` | ||
| <QirGeneration>true</QirGeneration> | ||
| ``` | ||
| If the project builds successfully, the .ll file containing QIR can be found in `qir` folder in the project folder. Alternatively, the folder path can be specified via the `QirOutputPath` project property. The project file should look similar to this: |
There was a problem hiding this comment.
Very minor: right now we are actually producing both the human readable .ll file and the more tightly packed .bc (bitcode) file. I'm not sure if this is worth calling out in the readme, but we may eventually want to generate just one by default and have the the other as an option, so perhaps introducing the concepts now would be good.
There was a problem hiding this comment.
Hm, I am not sure what you are referring to; I believe we are currently only generating .ll, though I agree that we should be generating bitcode in the near future, possibly with an option to keep with human readable.
There was a problem hiding this comment.
Hmm... the update to also produce bitcode was in the feature branch but seems to have been lost in a merge somewhere. Here's the commit that introduced it: 722ec70#diff-d72a80363d839b02d5bf562da75e9641615d62dce924b53e85790cde9d1d8179. That commit is still in the history, but the code isn't in the file anymore, and I can't seem to find the commit that removed it. Was that intentionally dropped somewhere along the way?
There was a problem hiding this comment.
Ah, I found it. #889 included enough updates to the file Context.cs that the diff registered it as an entire deletion of the existing Emit function and replacing with a new Emit function. As a result, the merge automatically lost the change to produce bitcode. It's not a big deal, since as the diff I linked above shows it's extremely easy to produce, so we can add it back in when we are ready. I double checked the various repos and no test infrastructure is taking any dependencies on the bitcode being present, so there is no need to introduce any more churn to the feature branch while we are trying to merge to main.
| # QIR Emission - Preview Feature | ||
|
|
There was a problem hiding this comment.
If not explained earlier, you may want to explain in this file
- what the current implementation is (without QIR)
- what problems are there with the current implementation
- what solution is being worked on, what QIR stands for and how it solves the current problems
|
|
||
| ## Limitations | ||
|
|
||
| Please be aware that as of this time, it is not possible to both QIR and C#. |
There was a problem hiding this comment.
it is not possible to <generate?> both QIR and C#
|
|
||
| Please be aware that as of this time, it is not possible to both QIR and C#. | ||
| Open issues related to QIR emission can be found [here](https://github.com/microsoft/qsharp-compiler/issues?q=is%3Aopen+is%3Aissue+label%3A%22area%3A+QIR%22). | ||
| The emitted QIR does currently not contain any debug information. We are planning to add better support in the future. If you are interested in contributing, please indicate your interest on [this issue](https://github.com/microsoft/qsharp-compiler/issues/637). |
There was a problem hiding this comment.
I see
The emitted QIR does currently not contain
I suspect you meant
The emitted QIR
doescurrently does not contain
| The entry point wrapper performs some translation between standard | ||
| C types and QIR types. |
There was a problem hiding this comment.
Consider
performs some translation between
standardC types and QIR types.
| ## QIR Specification | ||
|
|
||
| The QIR specification is on the [Q# language repository](https://github.com/microsoft/qsharp-language/tree/main/Specifications/QIR). | ||
|
|
There was a problem hiding this comment.
(Minor)
If a very broad audience is expected to read this file, then I encourage the English native speakers to proofread/double-check the following fragments/prepositions:
-
please indicate your interest on this issue
-
The QIR specification is on the Q# language repository
| For Q# operation decorated with the `@EntryPoint` attribute, the QIR generation | ||
| will create an additional C-callable wrapper function. | ||
| The name of this wrapper is the same as the full, namespace-qualified name | ||
| of the entry point, with periods replaced by double underscores. | ||
| The entry point wrapper function gets tagged with an custom LLVM "EntryPoint" attribute. |
There was a problem hiding this comment.
If the concept of the entry points is introduced in some earlier piece of documentation then placing, in the beginning of this paragraph, a link to that introduction would help.
Otherwise, despite the fact that this paragraph talks about the entry points, strictly speaking this paragraph still does not define what the entry point is. In this case you might want to consider something like changing the first sentence to the following:
Entry point is a Q# operation decorated with the
@EntryPointattribute. The Q# function can also be made an entry point (by decorating it with the@EntryPointattribute), but it will make little sense since such a function will not be able to call the operations (see details in <Functions vs. Operations>).
For the entry points the Q# compiler will generate an additional LLVM IR wrapper function callable from C code.
Also includes latest updates from main, and updates the readme.
Takes care of #869.