✨ Add QC to QIR Adaptive Profile Conversion#1710
Merged
burgholzer merged 42 commits intoJun 12, 2026
Merged
Conversation
added 12 commits
May 13, 2026 15:38
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
added 2 commits
May 15, 2026 18:21
denialhaag
added
enhancement
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
📝 WalkthroughWalkthroughThis PR splits QC→QIR lowering into Base and Adaptive profiles with separate passes, adds shared lowering infrastructure, extends QIRProgramBuilder for profile-aware APIs and SCF builders, reorganizes build/tests, and updates pipeline/CLI to select profiles. ChangesQIR Profile-Based Lowering and Builder Extensions
Sequence DiagramssequenceDiagram
participant Pipeline as CompilerPipeline
participant PassMgr as PassManager
participant QCToQIRBase as QCToQIRBase
participant QCToQIRAdaptive as QCToQIRAdaptive
participant Common as CommonLowering
Pipeline->>PassMgr: determine profile (convertToQIRBase / convertToQIRAdaptive)
PassMgr->>QCToQIRBase: createQCToQIRBase() (if Base)
PassMgr->>QCToQIRAdaptive: createQCToQIRAdaptive() (if Adaptive)
QCToQIRBase->>Common: use LoweringState (static allocation)
QCToQIRAdaptive->>Common: use LoweringState (dynamic allocation)
Common->>PassMgr: register patterns via populateQCToQIRPatterns()
QCToQIRBase->>PassMgr: emit QIR init / measurement routing
QCToQIRAdaptive->>PassMgr: emit dynamic allocs, read_result, releases
sequenceDiagram
participant Client as QIRProgramBuilder::build
participant Builder as QIRProgramBuilder
participant Alloc as allocQubit / allocQubitRegister
participant SCF as scfFor / scfIf / scfWhile
participant Finalize as finalize
Client->>Builder: build(..., Profile::Adaptive)
Builder->>Builder: set profile = Adaptive
Builder->>Alloc: allocQubit() -> dynamic allocation (Adaptive)
Builder->>Alloc: allocQubitRegister() -> dynamic array (Adaptive)
Builder->>SCF: scfFor / scfIf / scfWhile -> construct blocks
Builder->>Finalize: finalize() -> emit release and output recording (profile-dependent)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h`:
- Around line 1076-1077: The DenseMap named loadedQubits (DenseMap<Value,
DenseSet<Value>>) is ambiguous because its key is a register Value while the
DenseSet stores index Values (integer indices); update the declaration of
loadedQubits to include a brief clarifying comment that the map keys are
register Values and the DenseSet contains index Value objects (i.e., integer
indices that have been loaded), referencing the symbol loadedQubits in
QIRProgramBuilder to make intent clear to future readers.
In `@mlir/include/mlir/Dialect/QIR/Utils/QIRMetadata.h`:
- Around line 34-39: Replace the magic-number int field backwardsBranching in
QIRMetadata with a scoped enum (e.g., enum class BackwardsBranching { None = 0,
For = 1, While = 2, Both = 3 }) and change the member to BackwardsBranching
backwardsBranching{BackwardsBranching::None}; then update all sites that read or
write this member (e.g., scfFor, scfWhile and any comparisons/assignments) to
use the enum values (BackwardsBranching::For, ::While, ::Both) instead of
integer literals; if you cannot change the type, at minimum add a clear comment
above the int declaration documenting the encoding (0=none,1=for,2=while,3=both)
and prefer named constants to replace literal uses.
In `@mlir/lib/Conversion/QCToQIR/QIRAdaptive/QCToQIRAdaptive.cpp`:
- Around line 523-540: The code only rewires the terminator of lastBlock so
other blocks that can return will bypass state.outputBlock; iterate over all
blocks in the main region (e.g., using main.getBlocks()) and for each block
(except the new entryBlock and finalBlock) examine its terminator (terminatorOp)
and replace any direct-return/exit terminators with a branch to finalBlock (or
move those terminators into finalBlock) so all exits route through
state.outputBlock; update uses of LLVM::BrOp::create, terminatorOp->moveBefore,
and the builder insertion points accordingly so every return-like terminator is
handled rather than only main.back().
In `@mlir/lib/Conversion/QCToQIR/QIRBase/QCToQIRBase.cpp`:
- Around line 331-334: The code currently aborts the compiler via
llvm::reportFatalInternalError when detecting unsupported input (e.g.,
main.getBlocks().size() > 1); change these checks (the one using
llvm::reportFatalInternalError around main and the similar one at 445-446) to
emit a proper MLIR error on the module op (use main.emitOpError("Modules with
multiple blocks are not supported in the Base Profile") or
emitError(main.getLoc()) with the same message) and then fail the pass
gracefully by signaling failure (call signalPassFailure() or return failure from
the pass entry point) so the pipeline reports a normal pass failure instead of
aborting.
- Around line 241-250: The allocation branch currently uses resultPtrs.size()
when op.getRegisterIndex() exists but the index key is missing, breaking stable
mapping; change the allocation to use the explicit register index
(static_cast<int64_t>(op.getRegisterIndex().value())) when calling
createPointerFromIndex and when emplacing into resultPtrs (replace
resultPtrs.size() key with the requested index), and update state.numResults to
account for sparse/high indices (e.g., set state.numResults =
std::max(state.numResults, requestedIndex + 1) or otherwise ensure the count
covers the allocated index). Ensure this logic is applied where result is
created (createPointerFromIndex) and where resultPtrs.try_emplace is invoked so
the requested register_index is preserved.
- Around line 109-113: The code currently defaults non-constant memref.load
indices to 0 by using getConstantIntValue(indexValue).value_or(0), which can
silently miscompile; replace that behavior by checking the Optional returned by
getConstantIntValue(indexValue) and explicitly failing the pattern when it has
no value (e.g., return failure() or emit a match/diagnostic) instead of falling
back to 0. Locate the indexValue handling (including the
UnrealizedConversionCastOp unwrap) and change the code to test
getConstantIntValue(indexValue).has_value() and handle the non-constant case by
rejecting the match with an explicit failure/diagnostic so unsupported dynamic
indices are not rewritten into qubit 0.
In `@mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp`:
- Around line 244-259: The duplicate-check using
loadedQubits[reg].contains(index) only compares SSA Value identity and misses
semantically-equal constant indices; update QIRProgramBuilder::load to perform a
stronger check: if index is a constant integer (detect via the LLVM constant/int
op/attribute for the Value), extract its integer value and check a per-register
set keyed by that integer, otherwise fall back to the existing SSA-Value check
(or add a comment documenting the limitation). Modify the storage used by
loadedQubits (or add an auxiliary map) to record both SSA Values and concrete
integer indices so load(), the contains check, and the insertion
(loadedQubits[reg].insert(...)) handle constant-index comparisons correctly
while preserving current behavior for dynamic indices.
- Around line 763-821: scfIf builds an i1 constant when cond is a bool but
always calls QIR_READ_RESULT with that value, causing a type mismatch because
QIR_READ_RESULT expects a pointer (measurement result), not an i1; change scfIf
so that when cond holds a bool you skip the
getOrCreateFunctionDeclaration/getOrCreateFunctionDeclaration + LLVM::CallOp
(the QIR_READ_RESULT/readOp calls) and use the constant i1 value directly for
the branch, while when cond is a Value (pointer) you preserve the existing path
that calls getOrCreateFunctionDeclaration(module, QIR_READ_RESULT, ...) and
LLVM::CallOp::create(...) to obtain the i1 to pass into LLVM::CondBrOp; update
references in this function (condition, readOp, QIR_READ_RESULT, LLVM::CallOp,
LLVM::CondBrOp) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c69551f5-cb87-4f0f-964c-1e2183044af9
📒 Files selected for processing (35)
mlir/include/mlir/Compiler/CompilerPipeline.hmlir/include/mlir/Conversion/QCToQIR/CMakeLists.txtmlir/include/mlir/Conversion/QCToQIR/Common/CommonQIR.hmlir/include/mlir/Conversion/QCToQIR/QIRAdaptive/CMakeLists.txtmlir/include/mlir/Conversion/QCToQIR/QIRAdaptive/QCToQIRAdaptive.hmlir/include/mlir/Conversion/QCToQIR/QIRAdaptive/QCToQIRAdaptive.tdmlir/include/mlir/Conversion/QCToQIR/QIRBase/CMakeLists.txtmlir/include/mlir/Conversion/QCToQIR/QIRBase/QCToQIRBase.hmlir/include/mlir/Conversion/QCToQIR/QIRBase/QCToQIRBase.tdmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/include/mlir/Dialect/QIR/Utils/QIRMetadata.hmlir/include/mlir/Dialect/QIR/Utils/QIRUtils.hmlir/lib/Compiler/CMakeLists.txtmlir/lib/Compiler/CompilerPipeline.cppmlir/lib/Conversion/QCToQIR/CMakeLists.txtmlir/lib/Conversion/QCToQIR/Common/CMakeLists.txtmlir/lib/Conversion/QCToQIR/Common/CommonQIR.cppmlir/lib/Conversion/QCToQIR/QCToQIR.cppmlir/lib/Conversion/QCToQIR/QIRAdaptive/CMakeLists.txtmlir/lib/Conversion/QCToQIR/QIRAdaptive/QCToQIRAdaptive.cppmlir/lib/Conversion/QCToQIR/QIRBase/CMakeLists.txtmlir/lib/Conversion/QCToQIR/QIRBase/QCToQIRBase.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QIR/Utils/QIRUtils.cppmlir/tools/mqt-cc/mqt-cc.cppmlir/unittests/Compiler/test_compiler_pipeline.cppmlir/unittests/Conversion/QCToQIR/CMakeLists.txtmlir/unittests/Conversion/QCToQIR/QCToQIRAdaptive/CMakeLists.txtmlir/unittests/Conversion/QCToQIR/QCToQIRAdaptive/test_qc_to_qir_adaptive.cppmlir/unittests/Conversion/QCToQIR/QCToQIRBase/CMakeLists.txtmlir/unittests/Conversion/QCToQIR/QCToQIRBase/test_qc_to_qir_base.cppmlir/unittests/Conversion/QCToQIR/test_qc_to_qir.cppmlir/unittests/Dialect/QIR/IR/test_qir_ir.cppmlir/unittests/programs/qir_programs.cppmlir/unittests/programs/qir_programs.h
💤 Files with no reviewable changes (2)
- mlir/lib/Conversion/QCToQIR/QCToQIR.cpp
- mlir/unittests/Conversion/QCToQIR/test_qc_to_qir.cpp
added 2 commits
May 19, 2026 11:48
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
Hey @denialhaag, Besides the issues that you mentioned I can mainly think of the conversion of the classical register informations of measure operations but that should be covered by #1726. Maybe something unrelated but a verifier for Base/Adaptive Profile that you mentioned yesterday in the meeting would be an interesting follow-up to properly verify QIR compliance. |
denialhaag
previously requested changes
May 22, 2026
denialhaag
left a comment
denialhaag
left a comment
Member
There was a problem hiding this comment.
Thanks for addressing all my comments! 🙂
I just noticed one more thing. Once this is resolved, we can mark this PR as closing #1619. I hope this is not too much of a headache now. Otherwise, I'd also be okay with postponing this to the follow-ups. We can discuss below.
That said, I'll already tag @burgholzer for his review.
Closed
Member
11 tasks
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
burgholzer
approved these changes
Jun 12, 2026
burgholzer
left a comment
burgholzer
left a comment
Member
There was a problem hiding this comment.
Hey @li-mingbao and @denialhaag 👋🏼
Finally found some time to dig through this.
Really nice work! While I did not check every single line for correctness, this seems to be doing what it should for now and feels like a great improvement for our QIR support.
Let's wait for CI to clear and then merge! 🚀
burgholzer
merged commit 61ae17f
into
munich-quantum-toolkit:main
29 of 32 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the conversion of QC to the Adaptive Profile of QIR including the conversion of scf operations.
The original conversion from QC to QIR is split up into two different conversion passes, separating the Base and the Adaptive Profile.
The Base QIR conversion now only supports operation that are available in the Base Profile. Any dynamic alllocation operations are converted into equivalent static qubit and result pointers during the conversion.
The QIRProgramBuilder is also updated to handle the Base and the Adaptive Profile.
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.