📝 Getting Started With MLIR#1555
Conversation
|
@burgholzer @denialhaag @DRovara I am pretty sure everyone of you has an opinion on how this getting started guide should look - and should not look. Hence, I kindly asked for a review from each one of you. I hope this is okay! |
📝 WalkthroughWalkthroughAdds a new MLIR "Getting Started" guide at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
burgholzer
added
documentation
There was a problem hiding this comment.
Thanks a lot for the work, @MatthiasReumann! All in all, it already looks pretty nice. I still tried to give some (maybe controversial) opinions, just to make sure everything is clear. None of them are really set in stone, just suggestions.
That being said though: I believe there is still some room for discussion when it comes to who the target audience should be for this "Getting Started Guide":
- Probably it shouldn't be quantum algorithm engineers who just want to compile their code. They don't care what happens in the background.
- Then, one would assume it's compiler developers. But why would compiler developers care about the
qcdialect (which the majority of this tutorial is about).
All in all, this brings me to the big question: Is it really the "correct" approach to give the getting started guide in terms of the QC dialect? People are never supposed to write MLIR code anyways - we will likely have some other front-end or DSL for that. So the only time when the code will use QC is during the translation process (and for the output, if the user does not compile down to QIR). The only advantage that QC has over QCO is that it is simpler, but it is weird to argue, for a tutorial "we won't explain the important thing because that is too complicated, so instead we explain the less important thing".
What are everyone else's thoughts on that?
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Thanks @MatthiasReumann for getting this started 😄
Great to see someone push this forward!
I now also gave this a thorough read.
I added some comments inline as well, but much more important than that, I would like to pick up on Damians points below.
That being said though: I believe there is still some room for discussion when it comes to who the target audience should be for this "Getting Started Guide":
Probably it shouldn't be quantum algorithm engineers who just want to compile their code. They don't care what happens in the background.
Then, one would assume it's compiler developers. But why would compiler developers care about the qc dialect (which the majority of this tutorial is about).
All in all, this brings me to the big question: Is it really the "correct" approach to give the getting started guide in terms of the QC dialect? People are never supposed to write MLIR code anyways - we will likely have some other front-end or DSL for that. So the only time when the code will use QC is during the translation process (and for the output, if the user does not compile down to QIR). The only advantage that QC has over QCO is that it is simpler, but it is weird to argue, for a tutorial "we won't explain the important thing because that is too complicated, so instead we explain the less important thing".What are everyone else's thoughts on that?
Currently, the guide feels very compact. It touches on some topics, but mostly briefly, and for the most part probably not exhaustively.
I said in one of the comments that it is "nicht fisch, nicht fleisch", and I think this also is what Damian is highlighting above. At the moment the guide itself isn't sure who it is written for.
I think this could easily be split into three parts:
- For the people knowing quantum that have never heard of MLIR; those should start with an MLIR section that explains the concepts; potentially referring back to concepts that people might know from SDKs like Qiskit or Pennylane, or from languages like OpenQASM. People knowing MLIR may skip this section.
- For the people that know classical compilers (and MLIR), but that don't know quantum too well. Those should start at a section that step by step explains quantum concepts, but tries to refer back to classical compiler and MLIR terminology wherever suitable. People knowing quantum may skip this section.
- For the people familiar with both (or that have read both previous sections). Those people, we should tell what exactly we are doing as part of the project here. This should explain the difference between the two dialects, the compilation flow, etc. This is the meat of the tutorial; the previous sections are kind of the background for the relevant crowd. Within this section, it may again be interesting to provide some anecdotes that people from one of the backgrounds would find helpful ("the program structure in QCO is very similar to DAG structures people may be familiar with from Qiskit" for example)
Overall, this should really have an educational character for people and leave as little up for imagination as possible (Damian already did a great job in the comments highlighting a couple of places where this might not yet be the case).
That's all I got for now. I hope this makes sense and helps to navigate this into the right direction. Despite the flood of comments, this is still a really great start! 🎉
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/mlir/GettingStarted.md (1)
83-89:⚠️ Potential issue | 🟠 MajorFill in the core tutorial sections before merge.
The two key subsections are empty (
The QC and QCO Dialects,Compilation Flow), so the guide currently misses the main learning path (QC vs QCO semantics and QC→QCO flow) promised by this PR and linked issue.✍️ Minimal structure to add now
### The QC and QCO Dialects +QC uses reference semantics and models qubits as references to allocated resources. +QCO uses value semantics with linear types, where each qubit SSA value is consumed exactly once. +Show one small side-by-side snippet (QC and QCO) for the same Bell-state step to make this concrete. ### Compilation Flow +The typical flow is: OpenQASM -> QC -> QCO -> (optional) QIR. +Add one command sequence with `mqt-cc` and briefly explain where optimizations happen. +Include one short note about current limitations (e.g., unsupported patterns), if applicable.Based on learnings from issue
#1452objectives, this guide is expected to explain QC vs QCO in detail and illustrate the QC → QCO transformation with a running example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mlir/GettingStarted.md` around lines 83 - 89, The two subsections "The QC and QCO Dialects" and "Compilation Flow" in GettingStarted.md are empty; fill them with content that (1) defines QC and QCO semantics (key differences, example ops/constructs and when to use each) under the "The QC and QCO Dialects" header and (2) describes the QC→QCO transformation pipeline with a small running example showing input QC IR, the transformation steps (pass names or functions) and resulting QCO IR under "Compilation Flow"; reference the section titles "The QC and QCO Dialects" and "Compilation Flow" when adding content so the guide explains QC vs QCO and demonstrates the QC→QCO flow end-to-end as requested by the linked issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/mlir/GettingStarted.md`:
- Line 164: The "## Conclusion" heading in the GettingStarted.md document is
empty; either remove the heading or add a brief concluding paragraph summarizing
the guide. Locate the heading text "## Conclusion" and either delete that line
(and any trailing blank lines) or append 2–4 sentences that wrap up the
document’s key takeaways and next steps so the section is not left blank.
- Around line 30-33: Fix the wording in the MLIR intro: change "The core concept
in MLIR are _dialects_" to "The core concepts in MLIR are _dialects_"; change
"floating point" to "floating-point" (e.g., "integer and floating-point
operations"); and replace "which let's us define and call functions" with "which
lets us define and call functions" (remove the apostrophe). Keep references to
SCF, arith, and Func dialects intact.
---
Duplicate comments:
In `@docs/mlir/GettingStarted.md`:
- Around line 83-89: The two subsections "The QC and QCO Dialects" and
"Compilation Flow" in GettingStarted.md are empty; fill them with content that
(1) defines QC and QCO semantics (key differences, example ops/constructs and
when to use each) under the "The QC and QCO Dialects" header and (2) describes
the QC→QCO transformation pipeline with a small running example showing input QC
IR, the transformation steps (pass names or functions) and resulting QCO IR
under "Compilation Flow"; reference the section titles "The QC and QCO Dialects"
and "Compilation Flow" when adding content so the guide explains QC vs QCO and
demonstrates the QC→QCO flow end-to-end as requested by the linked issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5455081e-a4a9-4d40-a96e-60c3468ccfde
⛔ Files ignored due to path filters (1)
docs/_static/mlir-regions-blocks-ops.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
docs/mlir/GettingStarted.mddocs/mlir/index.md
denialhaag
left a comment
denialhaag
left a comment
There was a problem hiding this comment.
Thanks for addressing my previous round of comments, @MatthiasReumann!
Apart from #1555 (comment), this LGTM now. In an effort to get this along, I'll already approve this PR now. 😌
ystade
left a comment
ystade
left a comment
There was a problem hiding this comment.
@MatthiasReumann I am so sorry that I let you wait for so long. There had been quite some very important and timely tasks before. However, that does not diminish your great effort. I think we are close to the finish line. I left few very minor comments inline. Not sure whether all reviewers have to finish their review. For me it would be fine to merge this after all comments are resolved. However, I leave the final decision to @burgholzer .
Once again, a big thank you to @MatthiasReumann !
| ```cpp | ||
| // file: mlir/lib/Dialect/QCO/Transforms/Optimizations/CancelConsecutiveHadamards.cpp | ||
|
|
||
| #include "mlir/Dialect/QCO/IR/QCOOps.h" // Newly added. |
There was a problem hiding this comment.
I do not get the "Newly added". What does it refer to?
There was a problem hiding this comment.
A snippet above uses the same file. With "Newly added" I tried to convey the idea that this include must be added to the above snippet. Because you didn't get that, I don't think I managed to do this successfully. Any ideas on how to make this more clear?
There was a problem hiding this comment.
@coderabbitai Can you suggest a comprehensive solution for our problem here? I'll repeat the problem for clarity: In this tutorial document, we show different code snippets. In the first line we indicate the file where this snippet is located. We refer two times to the file mlir/lib/Dialect/QCO/Transforms/Optimizations/CancelConsecutiveHadamards.cpp, the first time in lines 1200 – 1229 and the second time in lines 1239 – 1273. In the second snippet, we want to somehow indicate that we continue the first snippet, in particular, that we include an additional header file.
There was a problem hiding this comment.
@ystade Good idea to get a second opinion here. My suggestion: drop the inline // Newly added. comment entirely and instead rely on (1) explicit prose stating that the snippet continues in the same file, and (2) the :emphasize-lines: option of the MyST {code-block} directive to visually highlight the new line(s). This is more robust than a code comment because it doesn't compete with the actual C++ syntax and it's immediately visible without reading closely.
Concrete suggestion for the second snippet:
Continuing in the same file
(`mlir/lib/Dialect/QCO/Transforms/Optimizations/CancelConsecutiveHadamards.cpp`),
we add the following include for the QCO operations header:
```{code-block} cpp
:emphasize-lines: 1
`#include` "mlir/Dialect/QCO/IR/QCOOps.h"
```If you prefer to show the include in the context of the full growing snippet (rather than as an isolated one-liner), you can instead re-print the whole updated snippet with :emphasize-lines: pointing at just the new line(s), e.g. :emphasize-lines: 3 if the include is the third line. That way the reader sees the complete, valid file state plus a clear visual cue for what changed, without relying on a comment that could be mistaken for part of the actual pass logic.
Either way, I'd recommend removing the // Newly added. comment since it reads like it's annotating a semantic aspect of the code rather than the tutorial's narrative flow.
✏️ Learnings added
Learnt from: ystade
Repo: munich-quantum-toolkit/core PR: 1555
File: docs/mlir/GettingStarted.md:1180-1180
Timestamp: 2026-07-07T06:15:06.729Z
Learning: In docs/mlir/GettingStarted.md (and similar MQT Compiler Collection tutorial docs), when a code snippet continues in the same file as a previous snippet (e.g., mlir/lib/Dialect/QCO/Transforms/Optimizations/CancelConsecutiveHadamards.cpp appears in multiple snippets with added lines like a new `#include`), avoid inline comments like "// Newly added." to indicate the addition, as this is confusing to reviewers. Prefer explicit prose stating the snippet continues in the same file, combined with the MyST/Sphinx `{code-block}` directive's `:emphasize-lines:` option to visually highlight the new/changed line(s).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1435
File: docs/mlir/QC.md:1-11
Timestamp: 2026-01-08T22:56:03.140Z
Learning: MLIR dialect docs (e.g., MLIRQCDialect.md, MLIRQCInterfaces.md) are generated during the documentation build (pre_build) and should not be committed. Do not track these in version control; rely on the build system to produce/update them. Ensure CI/docs workflow excludes these generated files from checks and that the documentation build step regenerates them consistently (e.g., on pre_build).
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1507
File: docs/qdmi/qdmi_backend.md:401-401
Timestamp: 2026-02-09T19:48:26.818Z
Learning: Documentation should consistently spell the PUB acronym as 'Primitive Unified Blocs'. Verify that PUB is defined correctly (not 'Block') in all Qiskit-related documentation references, including the file docs/qdmi/qdmi_backend.md. When uncertain, align with the official BaseSamplerV2/BaseEstimatorV2 documentation and use that expansion consistently across docs.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1570
File: docs/mlir/QC.md:5-10
Timestamp: 2026-03-18T15:34:14.323Z
Learning: Do not require top-level H1 headings in the MyST page-wrapper files under docs/mlir (e.g., docs/mlir/QC.md, docs/mlir/QCO.md). These files use YAML front-matter and {include} directives; markdownlint MD041 does not fire on them, and the CI covers this. Treat missing H1 in these wrapper pages as acceptable and focus lint checks on actual content headings elsewhere.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1570
File: docs/mlir/QCO.md:5-15
Timestamp: 2026-03-18T15:34:20.604Z
Learning: In docs/mlir/*.md files that start with YAML front matter (--- ... ---) the MD041 first-line-heading rule should not be flagged. Specifically, these files may omit a top-level H1 because the YAML front matter serves as the header, and CI will pass without MD041 violations for such documentation pages. Apply this exception to MD files in the docs/mlir directory (e.g., docs/mlir/QCO.md) unless the front matter pattern is not present.
There was a problem hiding this comment.
@MatthiasReumann What do you think about @coderabbitai's suggestions? One more idea would be to include the entire include section including the includes from the previous snippet, emphasizing the new one with the trick @coderabbitai suggested, then adding a placeholder for the struct from the previous snippet and then adding the new content. So overall, it would be something like:
```{code-block} cpp
:emphasize-lines: 2
#include "mlir/Dialect/QCO/Transforms/Passes.h"
#include "mlir/Dialect/QCO/IR/QCOOps.h"
namespace mlir::qco {
// [previous code snippet]
struct Pattern...
```There was a problem hiding this comment.
@ystade That refinement works well and addresses the ambiguity better than my original suggestion — showing the full, valid include block with the new line emphasized removes any doubt about what changed, and the elision placeholder keeps the snippet from becoming unwieldy while still signaling "this is the same file, continued."
A couple of small tweaks I'd suggest:
- Use a standard ellipsis-style comment rather than a descriptive one, so it reads as elided code rather than an instruction, e.g.:
:emphasize-lines: 2
`#include` "mlir/Dialect/QCO/Transforms/Passes.h"
`#include` "mlir/Dialect/QCO/IR/QCOOps.h"
namespace mlir::qco {
// ...
struct CancelConsecutiveHadamards final
: impl::CancelConsecutiveHadamardsBase<CancelConsecutiveHadamards> {
// ...
};
} // namespace mlir::qco
Something like // ... (or /* ... */) is a common convention readers will recognize immediately as "omitted for brevity," whereas // [previous code snippet] reads slightly more like a tutorial-authoring note left in by mistake.
- Add one sentence of prose right before the block making the continuation explicit regardless of the visual cues, e.g. "Continuing in the same file, we add the following include (highlighted) and now implement the
matchAndRewritemethod:" — this way readers skimming quickly, or using a screen reader where syntax highlighting/line emphasis isn't conveyed, still get the context.
Otherwise, this is a good, self-contained way to solve the "second snippet in the same file" pattern here and likely elsewhere in the doc where this pattern repeats.
🧠 Learnings used
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1435
File: docs/mlir/QC.md:1-11
Timestamp: 2026-01-08T22:56:03.140Z
Learning: MLIR dialect docs (e.g., MLIRQCDialect.md, MLIRQCInterfaces.md) are generated during the documentation build (pre_build) and should not be committed. Do not track these in version control; rely on the build system to produce/update them. Ensure CI/docs workflow excludes these generated files from checks and that the documentation build step regenerates them consistently (e.g., on pre_build).
Learnt from: marcelwa
Repo: munich-quantum-toolkit/core PR: 1507
File: docs/qdmi/qdmi_backend.md:401-401
Timestamp: 2026-02-09T19:48:26.818Z
Learning: Documentation should consistently spell the PUB acronym as 'Primitive Unified Blocs'. Verify that PUB is defined correctly (not 'Block') in all Qiskit-related documentation references, including the file docs/qdmi/qdmi_backend.md. When uncertain, align with the official BaseSamplerV2/BaseEstimatorV2 documentation and use that expansion consistently across docs.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1570
File: docs/mlir/QC.md:5-10
Timestamp: 2026-03-18T15:34:14.323Z
Learning: Do not require top-level H1 headings in the MyST page-wrapper files under docs/mlir (e.g., docs/mlir/QC.md, docs/mlir/QCO.md). These files use YAML front-matter and {include} directives; markdownlint MD041 does not fire on them, and the CI covers this. Treat missing H1 in these wrapper pages as acceptable and focus lint checks on actual content headings elsewhere.
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1570
File: docs/mlir/QCO.md:5-15
Timestamp: 2026-03-18T15:34:20.604Z
Learning: In docs/mlir/*.md files that start with YAML front matter (--- ... ---) the MD041 first-line-heading rule should not be flagged. Specifically, these files may omit a top-level H1 because the YAML front matter serves as the header, and CI will pass without MD041 violations for such documentation pages. Apply this exception to MD files in the docs/mlir directory (e.g., docs/mlir/QCO.md) unless the front matter pattern is not present.
Learnt from: ystade
Repo: munich-quantum-toolkit/core PR: 1555
File: docs/mlir/GettingStarted.md:1180-1180
Timestamp: 2026-07-07T06:15:06.729Z
Learning: In Munich Quantum Toolkit MLIR tutorial docs (e.g., docs/mlir/GettingStarted.md), when documenting a code snippet that continues in the same source file across multiple snippet blocks (such as showing additional lines like a new include), avoid inline “added” markers inside the snippet (e.g., comments like "// Newly added."), since they confuse reviewers. Instead, add explicit prose stating that the snippet continues in the same file, and use the MyST/Sphinx `{code-block}` directive’s `:emphasize-lines:` option to visually highlight the new/changed lines in the block.
There was a problem hiding this comment.
That's a pretty good idea!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| ```console | ||
| % mqt-cc ghz.qasm --emit-qir | mlir-translate --mlir-to-llvmir > ghz.ll | ||
| ``` |
|
I'd like to take one final look through the guide once all feedback points have been addressed. |
Co-authored-by: Yannick Stade <100073938+ystade@users.noreply.github.com> Signed-off-by: matthias <matthias@bereumann.com>
…-quantum-toolkit/core into docs/getting_started_mlir
|
I do not want to postpone reviewing this forever, but it kind of feels like there are a couple of PRs that would still be really good to get in before finalizing here. |
5 participants
Description
This pull request adds a "Getting Started with MLIR" tutorial to the ReadTheDocs documentation. Thus, resolves #1452.
Checklist: