👨💻 Add implicit location handling to program builders and new utils tests #1446
Conversation
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
…Builder Signed-off-by: burgholzer <[email protected]>
…ilder Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
burgholzer
added
usability
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Signed-off-by: burgholzer <[email protected]>
Contributor
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRefactors QC, QCO, and QIR dialect builders to inherit from Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-12-02T07:37:46.860ZApplied to files:
📚 Learning: 2026-01-10T17:31:22.110ZApplied to files:
📚 Learning: 2026-01-10T15:54:56.832ZApplied to files:
📚 Learning: 2025-12-08T23:44:39.930ZApplied to files:
📚 Learning: 2026-01-10T18:49:44.352ZApplied to files:
📚 Learning: 2025-12-17T11:32:45.843ZApplied to files:
🪛 Cppcheck (2.19.0)mlir/unittests/Dialect/Utils/test_utils.cpp[information] 18-18: Include file (missingIncludeSystem) [information] 19-19: Include file (missingIncludeSystem) [information] 20-20: Include file (missingIncludeSystem) [information] 21-21: Include file (missingIncludeSystem) [information] 22-22: Include file (missingIncludeSystem) [information] 23-23: Include file (missingIncludeSystem) [information] 24-24: Include file (missingIncludeSystem) [information] 25-25: Include file (missingIncludeSystem) [information] 26-26: Include file (missingIncludeSystem) [information] 27-27: Include file (missingIncludeSystem) 🔇 Additional comments (4)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
mlir/unittests/Dialect/Utils/test_utils.cpp (1)
114-125: Fold test: assert the folded op type/shape beforegetResult(0).
If fold behavior changes,newConstants[0]->getResult(0)can become brittle; a quickdyn_cast(orgetNumResults()assertion) will make failures clearer.Proposed change
ASSERT_TRUE(builder->tryFold(op, tmp, &newConstants).succeeded()); ASSERT_EQ(newConstants.size(), 1); - const auto stdValue = utils::valueToDouble(newConstants[0]->getResult(0)); + auto cst = dyn_cast<arith::ConstantOp>(newConstants[0]); + ASSERT_TRUE(cst); + const auto stdValue = utils::valueToDouble(cst.getResult()); ASSERT_TRUE(stdValue.has_value()); EXPECT_DOUBLE_EQ(stdValue.value(), 3.5);mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (2)
167-216: Critical:registerResultMapkey can dangle (StringRef from temporarystd::string).
registerResultMapis keyed bystd::pair<llvm::StringRef, int64_t>. Inmeasure(Value, int64_t),defaultRegNameis a localstd::string; insertingStringRef(defaultRegName)into the DenseMap leaves a dangling pointer after return (UB in hashing/lookup/iteration).Proposed fix
- // Choose a safe default register name - std::string defaultRegName = "c"; - if (llvm::any_of(registerResultMap, [](const auto& entry) { - return entry.first.first == "c"; - })) { - defaultRegName = "__unnamed__"; - } + // Choose a safe default register name (must have stable backing storage for + // DenseMap<StringRef,...> keys). + llvm::StringRef defaultRegName = "c"; + if (llvm::any_of(registerResultMap, [](const auto& entry) { + return entry.first.first == "c"; + })) { + defaultRegName = "__unnamed__"; + } // Save current insertion point const InsertionGuard guard(*this); // Insert in measurements block (before branch) setInsertionPoint(measurementsBlock->getTerminator()); const auto key = std::make_pair(defaultRegName, resultIndex); if (const auto it = registerResultMap.find(key); it != registerResultMap.end()) { return it->second; } Value resultValue{}; if (const auto it = ptrCache.find(resultIndex); it != ptrCache.end()) { resultValue = it->second; } else { resultValue = createPointerFromIndex(*this, getLoc(), resultIndex); ptrCache[resultIndex] = resultValue; - registerResultMap.insert({key, resultValue}); } + registerResultMap.insert({key, resultValue});
268-331:createCallOp()has a redundantInsertionGuard(hard to reason about).
The second guard (entryBlockGuard) doesn’t appear to protect a distinct scope beyond what the outer guard already guarantees.Possible simplification
// Save current insertion point const InsertionGuard guard(*this); // Insert constants in entry block setInsertionPoint(entryBlock->getTerminator()); SmallVector<Value> parameterOperands; parameterOperands.reserve(parameters.size()); ... parameterOperands.push_back(parameterOperand); } - // Save current insertion point - const InsertionGuard entryBlockGuard(*this); - // Insert in body block (before branch) setInsertionPoint(bodyBlock->getTerminator());
🤖 Fix all issues with AI agents
In @mlir/unittests/Dialect/Utils/test_utils.cpp:
- Around line 40-46: The helper method createAddition is marked const but
mutates IR (creates operations and can change the builder insertion point), so
remove the const qualifier from its declaration and definition (i.e., change
arith::AddFOp createAddition(...) const to non-const) and optionally drop the
redundant const on the double parameters; update any call sites if needed to
compile against the non-const method.
- Around line 28-38: The test fixture UtilsTest creates an ImplicitLocOpBuilder
without an insertion point so ops it creates are detached and leak; update SetUp
to create a ModuleOp (e.g., via ModuleOp::create or equivalent on the
MLIRContext), set the builder’s insertion point into that module (use
builder->setInsertionPointToStart(module.getBody()) or the appropriate
setInsertionPointToStart call), and replace the FileLineColLoc row/column args
from 0,0 to 1,1; ensure the ModuleOp is a member of the fixture so created ops
are owned and cleaned up.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.mdmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cpp
🧰 Additional context used
🧠 Learnings (30)
📓 Common learnings
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
📚 Learning: 2026-01-12T12:05:47.463Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T18:49:44.352Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hCHANGELOG.mdmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T15:54:56.832Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:891-923
Timestamp: 2026-01-10T15:54:56.832Z
Learning: In MLIR conversion code (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf::WhileOp construction only creates empty regions, not blocks. After creating a WhileOp with `rewriter.create<scf::WhileOp>(...)`, the first calls to `rewriter.createBlock(&whileOp.getBefore(), ...)` and `rewriter.createBlock(&whileOp.getAfter(), ...)` create the initial blocks in those empty regions, so no block erasure is needed. This differs from scf::IfOp, which auto-creates an empty block that must be erased when using `inlineRegionBefore()`.
<!-- <review_comment_addressed>
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-07T12:29:16.380Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1070-1085
Timestamp: 2026-01-07T12:29:16.380Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the ConvertQCOFuncFuncOp pattern assumes that when a func.func operation is matched for conversion, all of its arguments are qco.qubit types (never mixed qubit/classical). The pattern unconditionally converts all arguments to qc::QubitType based on this assumption.
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T16:07:55.896Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:988-1024
Timestamp: 2026-01-10T16:07:55.896Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the SCF operation conversion patterns (ConvertQCOScfYieldOp, ConvertQCOScfConditionOp, ConvertQCOScfIfOp, ConvertQCOScfWhileOp, ConvertQCOScfForOp) assume that all operands are qubit types (qco.qubit or qc.qubit), never mixed qubit/classical types. The conversion is scoped to handle all-qubit SCF operations only.
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T16:28:41.975Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:1729-1763
Timestamp: 2026-01-10T16:28:41.975Z
Learning: In the QCToQCO conversion pass (mlir/lib/Conversion/QCToQCO/QCToQCO.cpp), the dynamic legality checks for func operations (func::CallOp, func::FuncOp, func::ReturnOp) and scf operations assume that operations use either all classical types or all qubit types, never mixed. Therefore, checking for the presence of qc::QubitType in operands or arguments is sufficient to determine if conversion is needed—there is no need to check both operands and results separately.
<!-- </add_learning]
Applied to files:
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-01-08T22:56:09.502Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1435
File: docs/mlir/QC.md:1-11
Timestamp: 2026-01-08T22:56:09.502Z
Learning: In the munich-quantum-toolkit/core repository, MLIR dialect documentation files (e.g., MLIRQCDialect.md, MLIRQCInterfaces.md) are automatically generated during the documentation build via the ReadTheDocs `pre_build` step and do not need to be committed to the repository.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-01T11:00:40.342Z
Learnt from: flowerthrower
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: CHANGELOG.md:18-18
Timestamp: 2025-12-01T11:00:40.342Z
Learning: In the munich-quantum-toolkit/core-plugins-catalyst repository, the CHANGELOG.md intentionally references the parent MQT Core repository's release notes (https://github.com/munich-quantum-toolkit/core/releases) because the plugin repository is based on work performed in the parent repository.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-24T10:19:41.147Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-01-10T17:31:22.110Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:816-862
Timestamp: 2026-01-10T17:31:22.110Z
Learning: In mlir/lib/Conversion/QCOToQC/QCOToQC.cpp, when creating scf::IfOp with `rewriter.create<scf::IfOp>(loc, types, condition, false)`, passing `false` for the withElseRegion parameter creates an else region that exists but contains no blocks. Therefore, inlineRegionBefore can be called on the else region without checking if it exists, and there's no need to erase an auto-created empty block from the else region (unlike the then region, which does get an auto-created empty block that must be erased).
Applied to files:
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-10-09T13:28:29.237Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/DeadGateEliminationPattern.cpp:42-45
Timestamp: 2025-10-09T13:28:29.237Z
Learning: In the MQTOpt MLIR dialect, linear types enforce single-use semantics where each qubit value can only be consumed once, preventing duplicate deallocations and making RAUW operations safe when replacing output qubits with corresponding input qubits in transformation patterns.
Applied to files:
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h
📚 Learning: 2025-12-09T00:55:11.926Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Operations/StandardGates/BarrierOp.cpp:45-54
Timestamp: 2025-12-09T00:55:11.926Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux), qubits follow single-user/linear-type semantics where each qubit value can only be consumed once, similar to MQTOpt. This invariant makes it safe to dereference getUsers().begin() in canonicalization patterns like MergeSubsequentBarrier in BarrierOp.cpp, as there will be at most one user per qubit output.
Applied to files:
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.
Applied to files:
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2026-01-07T12:29:02.062Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1040-1050
Timestamp: 2026-01-07T12:29:02.062Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), ConvertQCOFuncCallOp assumes that if a func::CallOp has qubit results, then all arguments and results are qubits (no mixed classical/quantum types). The conversion is scoped to handle all-qubit function calls only.
Applied to files:
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:58:09.648Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:80-100
Timestamp: 2025-12-08T23:58:09.648Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), quartz.ctrl uses reference semantics and does not return values, unlike flux.ctrl which uses value semantics and returns transformed qubits. When inlining a GPhaseOp in the CtrlInlineGPhase pattern, it's correct to create POp operations for positive controls and erase the CtrlOp without collecting or replacing result values.
Applied to files:
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-17T17:44:31.349Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:259-259
Timestamp: 2025-12-17T17:44:31.349Z
Learning: In the QCO dialect (mlir/include/mlir/Dialect/QCO/IR/QCOOps.td), GPhaseOp intentionally uses `MemoryEffects<[MemWrite]>` instead of `Pure` to prevent the remove-dead-values pass from eliminating it. Since GPhaseOp is a zero-target operation with no result values, it would otherwise be removed by DCE, even though it has a meaningful effect on the global quantum state.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T23:44:39.930Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Operations/StandardGates/PhaseOp.cpp:0-0
Timestamp: 2025-12-08T23:44:39.930Z
Learning: In MLIR code under any mlir/ directory, avoid using const qualifiers on core MLIR types in function parameters/signatures (e.g., Value, Type, Attribute, Operation*, Block*, Region*, etc.). This aligns with MLIR's design rationale and should be applied to C++ source files (e.g., .cpp) within mlir/; see https://mlir.llvm.org/docs/Rationale/UsageOfConst/ for details.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-10T02:20:01.189Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T14:55:43.899Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-100
Timestamp: 2025-12-08T14:55:43.899Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), GPhaseOp is a zero-target operation (global phase). When a CtrlOp wraps a GPhaseOp, it only has control qubits and no targets. The CtrlInlineGPhase canonicalization pattern correctly produces outputs only for the positive controls, not targets.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T12:44:05.883Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:60-70
Timestamp: 2025-12-08T12:44:05.883Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The RemoveTrivialCtrl pattern correctly only checks getNumPosControls() when determining if a CtrlOp should be removed.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:16:20.680Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-101
Timestamp: 2025-12-08T23:16:20.680Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The CtrlInlineGPhase canonicalization pattern correctly only checks getNumPosControls() and processes only positive controls when inlining a GPhaseOp.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-14T17:02:02.997Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp:187-210
Timestamp: 2025-12-14T17:02:02.997Z
Learning: In the Flux dialect builder (mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp), the coding style relies on implicit conversion from Value to ValueRange in ctrl() calls (e.g., `ctrl(control, {}, ...)` instead of `ctrl(ValueRange{control}, ValueRange{}, ...)`). This pattern is used consistently throughout all macro-generated methods and should be maintained for consistency.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-17T11:32:45.843Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:83-86
Timestamp: 2025-12-17T11:32:45.843Z
Learning: In the mlir portion of munich-quantum-toolkit/core, prefer marking free functions as static (static linkage) over placing them in anonymous namespaces, per the clang-tidy rule llvm-prefer-static-over-anonymous-namespace. Do not apply this to type/class definitions; they may continue to use anonymous namespaces. This guideline should be checked across C++ source files under mlir/ (e.g., any free function in LayeredUnit.cpp) to ensure free functions have static linkage, while types/classes can remain in anonymous namespaces.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-10T02:19:50.634Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:19:50.634Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), ensure a cache check prevents duplicate measurements: if registerResultMap.find(key) succeeds, return early to avoid generating multiple mz calls for the same classical bit index and guarantee only one entry per index in the output. This enforces an override-like semantics where repeated measurements reuse the cached pointer without extra side effects. Apply this caching/early-return pattern to similar measurement paths in the QIR builder code to improve correctness and efficiency.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
🧬 Code graph analysis (5)
mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (1)
QCOProgramBuilder(37-42)
mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (1)
QCProgramBuilder(35-40)
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h (1)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (1)
QIRProgramBuilder(38-45)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (1)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (4)
reset(187-197)reset(187-187)checkFinalized(620-625)checkFinalized(620-620)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (1)
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h (5)
entryBlock(823-823)bodyBlock(825-825)measurementsBlock(827-827)outputBlock(829-829)index(151-170)
🪛 Cppcheck (2.19.0)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
[information] 26-26: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
[information] 24-24: Include file
(missingIncludeSystem)
mlir/unittests/Dialect/Utils/test_utils.cpp
[information] 21-21: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
[information] 26-26: Include file
(missingIncludeSystem)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🇨 Lint / 🚨 Lint
- GitHub Check: 🇨 Coverage / 📈 Coverage
🔇 Additional comments (13)
CHANGELOG.md (2)
14-14: Changelog entry looks consistent; please verify ordering vs “latest PRs first” rule.Adding
[#1446]to the existing aggregated “Added” bullet matches the current style in thisCHANGELOG.md. Please just double-check that appending[#1446]at the end still respects the file header note (Line 1) about listing the latest PRs first (i.e., that#1446is the most recent merge among#1264/#1402/#1428/#1430/#1436/#1443/#1446).
310-310: PR link definition for[#1446]is correct.The new PR reference is added in the expected section and follows the existing link-definition format.
mlir/unittests/Dialect/Utils/test_utils.cpp (2)
21-21: Good: includemlir/IR/Location.hforFileLineColLoc.
This makes the new location construction explicit and avoids relying on transitive includes.
49-58: LGTM: tests consistently useImplicitLocOpBuilder+op.getResult()forvalueToDouble.
This is cleaner than keeping separateValuevariables and aligns with the implicit-location builder migration.Also applies to: 60-69, 71-80, 82-96, 98-104, 106-112
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h (2)
73-73: Inheritance switch toImplicitLocOpBuilderis consistent with the refactor direction.
843-848: Good: cachingptrType/voidTypeas members avoids repeated type construction and keeps the builder self-contained.mlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.h (1)
63-63: Builder inheritance migration looks correct; keeps public API stable while removing explicit loc plumbing.mlir/include/mlir/Dialect/QC/Builder/QCProgramBuilder.h (1)
54-54: Builder inheritance migration looks correct; aligns QC/QCO/QIR builder patterns.mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (2)
37-41: Ctor/initialize migration to implicit-loc builder is consistent and removes boilerplate cleanly.Also applies to: 44-59
565-585: Validate ctrl-body arity before emittingqco.yield(avoid building invalid IR right before abort). [suggest_minor_issue]Proposed adjustment
const InsertionGuard guard(*this); setInsertionPointToStart(&block); const auto innerTargetsOut = body(block.getArguments()); - YieldOp::create(*this, innerTargetsOut); - if (innerTargetsOut.size() != targets.size()) { llvm::reportFatalUsageError( "Ctrl body must return exactly one output qubit per target"); } + YieldOp::create(*this, innerTargetsOut);mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (2)
35-57: QC builder conversion to implicit-loc + loc-less op creation is coherent and matches QCO/QIR patterns.
155-172: Double-checkqc::CtrlOp::create(*this, ...)guarantees correct insertion-point/RAII behavior for the provided lambdas.
IfCtrlOp::createdoesn’t internally guard/restore insertion points, these macro helpers can leak insertion state across user calls.Also applies to: 180-196, 214-235, 245-268, 277-305, 313-330, 341-362, 372-397, 416-421
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (1)
38-92: QIR initialize() refactor to implicit-loc builder preserves the 4-block structure and keeps terminators well-defined.
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:
- Around line 175-182: Replace the plain const char* style static constexpr auto
string literals with llvm::StringLiteral to improve type safety: change
defaultRegName and fallbackRegName to constexpr llvm::StringLiteral, and keep
regName as a StringRef initialized from those StringLiterals; update the
comparison against registerResultMap (used in the lambda) to compare
entry.first.first to regName or to defaultRegName.asStringRef() as appropriate
so types match (symbols: defaultRegName, fallbackRegName, regName,
registerResultMap).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cpp
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T17:31:22.110Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:816-862
Timestamp: 2026-01-10T17:31:22.110Z
Learning: In mlir/lib/Conversion/QCOToQC/QCOToQC.cpp, when creating scf::IfOp with `rewriter.create<scf::IfOp>(loc, types, condition, false)`, passing `false` for the withElseRegion parameter creates an else region that exists but contains no blocks. Therefore, inlineRegionBefore can be called on the else region without checking if it exists, and there's no need to erase an auto-created empty block from the else region (unlike the then region, which does get an auto-created empty block that must be erased).
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T15:54:56.832Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:891-923
Timestamp: 2026-01-10T15:54:56.832Z
Learning: In MLIR conversion code (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf::WhileOp construction only creates empty regions, not blocks. After creating a WhileOp with `rewriter.create<scf::WhileOp>(...)`, the first calls to `rewriter.createBlock(&whileOp.getBefore(), ...)` and `rewriter.createBlock(&whileOp.getAfter(), ...)` create the initial blocks in those empty regions, so no block erasure is needed. This differs from scf::IfOp, which auto-creates an empty block that must be erased when using `inlineRegionBefore()`.
<!-- <review_comment_addressed>
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:44:39.930Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Operations/StandardGates/PhaseOp.cpp:0-0
Timestamp: 2025-12-08T23:44:39.930Z
Learning: In MLIR code under any mlir/ directory, avoid using const qualifiers on core MLIR types in function parameters/signatures (e.g., Value, Type, Attribute, Operation*, Block*, Region*, etc.). This aligns with MLIR's design rationale and should be applied to C++ source files (e.g., .cpp) within mlir/; see https://mlir.llvm.org/docs/Rationale/UsageOfConst/ for details.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-17T11:32:45.843Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:83-86
Timestamp: 2025-12-17T11:32:45.843Z
Learning: In the mlir portion of munich-quantum-toolkit/core, prefer marking free functions as static (static linkage) over placing them in anonymous namespaces, per the clang-tidy rule llvm-prefer-static-over-anonymous-namespace. Do not apply this to type/class definitions; they may continue to use anonymous namespaces. This guideline should be checked across C++ source files under mlir/ (e.g., any free function in LayeredUnit.cpp) to ensure free functions have static linkage, while types/classes can remain in anonymous namespaces.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-12T12:05:47.463Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T18:49:44.352Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T16:28:41.975Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:1729-1763
Timestamp: 2026-01-10T16:28:41.975Z
Learning: In the QCToQCO conversion pass (mlir/lib/Conversion/QCToQCO/QCToQCO.cpp), the dynamic legality checks for func operations (func::CallOp, func::FuncOp, func::ReturnOp) and scf operations assume that operations use either all classical types or all qubit types, never mixed. Therefore, checking for the presence of qc::QubitType in operands or arguments is sufficient to determine if conversion is needed—there is no need to check both operands and results separately.
<!-- </add_learning]
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-07T12:29:02.062Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1040-1050
Timestamp: 2026-01-07T12:29:02.062Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), ConvertQCOFuncCallOp assumes that if a func::CallOp has qubit results, then all arguments and results are qubits (no mixed classical/quantum types). The conversion is scoped to handle all-qubit function calls only.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2026-01-10T16:07:55.896Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:988-1024
Timestamp: 2026-01-10T16:07:55.896Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the SCF operation conversion patterns (ConvertQCOScfYieldOp, ConvertQCOScfConditionOp, ConvertQCOScfIfOp, ConvertQCOScfWhileOp, ConvertQCOScfForOp) assume that all operands are qubit types (qco.qubit or qc.qubit), never mixed qubit/classical types. The conversion is scoped to handle all-qubit SCF operations only.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-17T17:44:31.349Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:259-259
Timestamp: 2025-12-17T17:44:31.349Z
Learning: In the QCO dialect (mlir/include/mlir/Dialect/QCO/IR/QCOOps.td), GPhaseOp intentionally uses `MemoryEffects<[MemWrite]>` instead of `Pure` to prevent the remove-dead-values pass from eliminating it. Since GPhaseOp is a zero-target operation with no result values, it would otherwise be removed by DCE, even though it has a meaningful effect on the global quantum state.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-07T12:29:16.380Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1070-1085
Timestamp: 2026-01-07T12:29:16.380Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the ConvertQCOFuncFuncOp pattern assumes that when a func.func operation is matched for conversion, all of its arguments are qco.qubit types (never mixed qubit/classical). The pattern unconditionally converts all arguments to qc::QubitType based on this assumption.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-09T00:55:11.926Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Operations/StandardGates/BarrierOp.cpp:45-54
Timestamp: 2025-12-09T00:55:11.926Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux), qubits follow single-user/linear-type semantics where each qubit value can only be consumed once, similar to MQTOpt. This invariant makes it safe to dereference getUsers().begin() in canonicalization patterns like MergeSubsequentBarrier in BarrierOp.cpp, as there will be at most one user per qubit output.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-10T02:20:01.189Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:20:01.189Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), the intentional design prevents duplicate measurements: the early return when `registerResultMap.find(key)` succeeds avoids generating multiple `mz` calls to the same classical bit index and ensures output recording contains only one entry per index. This implements an "override" semantic where repeated measurements to the same resultIndex reuse the cached pointer without additional side effects.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:58:09.648Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:80-100
Timestamp: 2025-12-08T23:58:09.648Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), quartz.ctrl uses reference semantics and does not return values, unlike flux.ctrl which uses value semantics and returns transformed qubits. When inlining a GPhaseOp in the CtrlInlineGPhase pattern, it's correct to create POp operations for positive controls and erase the CtrlOp without collecting or replacing result values.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.hmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T14:55:43.899Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-100
Timestamp: 2025-12-08T14:55:43.899Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), GPhaseOp is a zero-target operation (global phase). When a CtrlOp wraps a GPhaseOp, it only has control qubits and no targets. The CtrlInlineGPhase canonicalization pattern correctly produces outputs only for the positive controls, not targets.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T12:44:05.883Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp:60-70
Timestamp: 2025-12-08T12:44:05.883Z
Learning: In the Quartz dialect (mlir/lib/Dialect/Quartz/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The RemoveTrivialCtrl pattern correctly only checks getNumPosControls() when determining if a CtrlOp should be removed.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-12-08T23:16:20.680Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-101
Timestamp: 2025-12-08T23:16:20.680Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), negative controls are not supported at the current stage. The CtrlInlineGPhase canonicalization pattern correctly only checks getNumPosControls() and processes only positive controls when inlining a GPhaseOp.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-14T17:02:02.997Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp:187-210
Timestamp: 2025-12-14T17:02:02.997Z
Learning: In the Flux dialect builder (mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp), the coding style relies on implicit conversion from Value to ValueRange in ctrl() calls (e.g., `ctrl(control, {}, ...)` instead of `ctrl(ValueRange{control}, ValueRange{}, ...)`). This pattern is used consistently throughout all macro-generated methods and should be maintained for consistency.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
📚 Learning: 2025-10-09T13:13:51.224Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReplaceBasisStateControlsWithIfPattern.cpp:171-180
Timestamp: 2025-10-09T13:13:51.224Z
Learning: In MQT Core MLIR, UnitaryInterface operations guarantee 1-1 correspondence between input and output qubits in the same order. When cloning or modifying unitary operations (e.g., removing controls), this correspondence is maintained by construction, so yielding getAllInQubits() in else-branches matches the result types from the operation's outputs.
Applied to files:
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
📚 Learning: 2025-12-10T02:19:50.634Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:19:50.634Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), ensure a cache check prevents duplicate measurements: if registerResultMap.find(key) succeeds, return early to avoid generating multiple mz calls for the same classical bit index and guarantee only one entry per index in the output. This enforces an override-like semantics where repeated measurements reuse the cached pointer without extra side effects. Apply this caching/early-return pattern to similar measurement paths in the QIR builder code to improve correctness and efficiency.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
🧬 Code graph analysis (2)
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h (1)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (1)
QIRProgramBuilder(38-45)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (3)
mlir/unittests/Dialect/Utils/test_utils.cpp (1)
context(35-42)mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (4)
reset(187-197)reset(187-187)checkFinalized(620-625)checkFinalized(620-620)mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (4)
reset(248-264)reset(248-248)checkFinalized(561-566)checkFinalized(561-561)
🪛 Cppcheck (2.19.0)
mlir/unittests/Dialect/Utils/test_utils.cpp
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
[information] 26-26: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
[information] 26-26: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp
[information] 24-24: Include file
(missingIncludeSystem)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (17)
mlir/include/mlir/Dialect/QIR/Builder/QIRProgramBuilder.h (3)
73-73: LGTM! Clean migration to ImplicitLocOpBuilder.The inheritance change from
OpBuildertoImplicitLocOpBuilderis correct and aligns with the PR objective of eliminating explicitUnknownLocusage in operation creation calls.
814-849: LGTM! Well-organized private member additions.The new private members are appropriately organized:
moduleandmainFuncprovide direct access to key IR structuresptrTypeandvoidTypecache commonly-used LLVM types, avoiding repeated::get()calls- The
stringSaver(line 822) properly manages StringRef lifetimes for theregisterResultMapThis refactoring improves both clarity and efficiency.
139-146: StringRef lifetime management forBit::registerNameis safe with current usage patterns.The change from
std::stringtollvm::StringRefdoes create a potential lifetime hazard:Bitinstances returned fromClassicalRegister::operator[](line 169-170) hold a non-owning reference toClassicalRegister::name. However, verification of actual usage patterns confirms this is safe. All measure calls follow the same pattern:const auto& c = b.allocClassicalBitRegister(3); b.measure(q[0], c[0]); b.measure(q[0], c[1]);The
Bitstruct is created byoperator[]and immediately passed tomeasure(), which converts theStringRefto an MLIR string attribute within the same scope. The ClassicalRegister outlives all uses of the returnedBit. No code storesBitinstances for later use.The design is safe by API contract but fragile: future code that stores a
Bitbeyond the lifetime of its parentClassicalRegisterwould create a dangling reference. Consider documenting this requirement or adjusting the API design for safety.mlir/unittests/Dialect/Utils/test_utils.cpp (3)
32-42: LGTM! Test fixture properly migrated to ImplicitLocOpBuilder.The setup correctly:
- Creates a
FileLineColLocfor consistent location tracking- Uses
ModuleOp::create(loc)for module creation- Initializes
ImplicitLocOpBuilderwith the same location- Sets the insertion point to the module body
This mirrors the pattern used in the actual builder implementations.
44-50: LGTM! Helper function correctly updated.The
createAdditionhelper now:
- Uses the static
arith::ConstantOp::create(*builder, ...)API- Uses
arith::AddFOp::create(*builder, ...)for the addition- Returns the operation directly (enabled by
[[nodiscard]]attribute)This is consistent with the implicit location pattern used throughout the PR.
118-130: LGTM! Value folding test correctly adapted.The test properly handles the folded constant:
- Uses
dyn_cast<arith::ConstantOp>to safely cast the folded operation- Calls
getResult()on the constant to extract the value- Maintains the test's original verification logic
mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp (3)
37-42: LGTM! Constructor properly initializes ImplicitLocOpBuilder.The constructor correctly:
- Initializes the
ImplicitLocOpBuilderbase with a descriptiveFileLineColLoc- Creates the module using the implicit location via
ModuleOp::create(*this)- Loads the required dialect
This establishes the implicit location context for all subsequent operations.
565-599: LGTM! ctrl() method correctly handles implicit location context.The
ctrl()method properly:
- Creates
CtrlOpusing implicit location- Uses
getLoc()forblock.addArgument()which requires an explicitLocationparameter- Uses
InsertionGuardto manage insertion point- Creates
YieldOpusing implicit locationThe explicit
getLoc()call on line 574 is necessary becauseBlock::addArgumentrequires aLocationparameter—this is the correct approach when the underlying MLIR API requires an explicit location.
627-675: LGTM! Finalization correctly uses implicit location pattern.The
finalize()method properly:
- Creates
DeallocOpoperations using implicit location- Creates the exit code constant using implicit location
- Creates the return operation using implicit location
- Invalidates the context to prevent use-after-finalize
The implementation is clean and consistent with the rest of the file.
mlir/lib/Dialect/QC/Builder/QCProgramBuilder.cpp (3)
35-40: LGTM! Constructor follows the established pattern.The constructor correctly initializes
ImplicitLocOpBuilderwith a descriptiveFileLineColLoc("<qc-program-builder>"), consistent with the QCO and QIR builders.
155-172: LGTM! Macro-generated operations correctly use implicit location.The
DEFINE_ZERO_TARGET_ONE_PARAMETERmacro demonstrates the correct pattern:
- Line 159:
OP_CLASS::create(*this, PARAM)uses implicit location- Line 170:
CtrlOp::create(*this, controls, [&] { ... })uses implicit location with lambda bodyThis pattern is consistently applied across all operation type macros.
454-505: LGTM! Finalization is clean and well-structured.The
finalize()method:
- Validates the builder state and insertion point
- Sorts and deallocates remaining qubits deterministically
- Creates exit code and return operation using implicit location
- Invalidates context to prevent use-after-finalize
The implementation properly mirrors the QCO builder's finalization logic.
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (5)
38-45: LGTM! Constructor efficiently caches commonly-used types.The constructor:
- Initializes
ImplicitLocOpBuilderwith a descriptive location- Creates the module using implicit location
- Caches
ptrTypeandvoidTypeto avoid repeated::get()calls- Loads the LLVM dialect
Caching
ptrTypeandvoidTypeis a good optimization since these types are used frequently throughout the builder.
47-92: LGTM! QIR 4-block structure correctly initialized.The
initialize()method properly sets up the QIR Base Profile structure:
- Creates main function with LLVM calling convention
- Establishes 4-block layout (entry, body, measurements, output)
- Adds
__quantum__rt__initializecall in entry block- Creates unconditional branches between blocks
- Sets up return in output block
All operations correctly use implicit location via
*thisor the static::create()pattern.
270-330: LGTM! createCallOp helper properly manages insertion points.The
createCallOpmethod correctly:
- Uses
InsertionGuardto restore insertion point- Inserts constants in entry block (before terminator)
- Inserts the actual call in body block (before terminator)
- Uses cached
voidTypefor function signature- Properly constructs operand ordering (controls, targets, parameters)
The separation of constant creation and call creation across different blocks is intentional for QIR compliance.
568-632: LGTM! Output recording generation is well-structured.The
generateOutputRecording()method:
- Groups measurements by register name
- Sorts for deterministic output
- Generates array_record_output and result_record_output calls
- Uses
createResultLabelhelper for label generation- Properly uses the cached type members
The implementation correctly follows the QIR output recording specification.
634-645: LGTM! Simplified finalization for QIR builder.Unlike QC/QCO builders, QIR doesn't need automatic qubit deallocation (QIR uses static qubit references). The finalization correctly:
- Generates output recording
- Sets QIR metadata attributes
- Marks the builder as finalized
This is simpler and appropriate for the QIR model.
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (1)
196-203: Duplicate measurement bug when pointer is cached fromstaticQubit.If
ptrCachealready containsresultIndex(e.g., from a priorstaticQubitcall) butregisterResultMapdoes not contain the key, thetry_emplaceat line 202 is skipped because it's inside theelseblock. Subsequent calls tomeasurewith the sameresultIndexwill:
- Fail the
registerResultMapcheck (key still absent)- Retrieve the cached pointer from
ptrCache- Create another
mzcall — a duplicate measurementThe
try_emplaceshould be called unconditionally after obtainingresultValueto ensure the key is tracked.🐛 Proposed fix
Value resultValue{}; if (const auto it = ptrCache.find(resultIndex); it != ptrCache.end()) { resultValue = it->second; } else { resultValue = createPointerFromIndex(*this, getLoc(), resultIndex); ptrCache[resultIndex] = resultValue; - registerResultMap.try_emplace(key, resultValue); } + registerResultMap.try_emplace(key, resultValue); // Update result count
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cppmlir/unittests/Dialect/Utils/test_utils.cpp
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:988-1024
Timestamp: 2026-01-10T16:07:55.896Z
Learning: In the QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), the SCF operation conversion patterns (ConvertQCOScfYieldOp, ConvertQCOScfConditionOp, ConvertQCOScfIfOp, ConvertQCOScfWhileOp, ConvertQCOScfForOp) assume that all operands are qubit types (qco.qubit or qc.qubit), never mixed qubit/classical types. The conversion is scoped to handle all-qubit SCF operations only.
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:816-862
Timestamp: 2026-01-10T17:31:22.110Z
Learning: In mlir/lib/Conversion/QCOToQC/QCOToQC.cpp, when creating scf::IfOp with `rewriter.create<scf::IfOp>(loc, types, condition, false)`, passing `false` for the withElseRegion parameter creates an else region that exists but contains no blocks. Therefore, inlineRegionBefore can be called on the else region without checking if it exists, and there's no need to erase an auto-created empty block from the else region (unlike the then region, which does get an auto-created empty block that must be erased).
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:891-923
Timestamp: 2026-01-10T15:54:56.832Z
Learning: In MLIR conversion code (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf::WhileOp construction only creates empty regions, not blocks. After creating a WhileOp with `rewriter.create<scf::WhileOp>(...)`, the first calls to `rewriter.createBlock(&whileOp.getBefore(), ...)` and `rewriter.createBlock(&whileOp.getAfter(), ...)` create the initial blocks in those empty regions, so no block erasure is needed. This differs from scf::IfOp, which auto-creates an empty block that must be erased when using `inlineRegionBefore()`.
<!-- <review_comment_addressed>
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCToQCO/QCToQCO.cpp:1729-1763
Timestamp: 2026-01-10T16:28:41.975Z
Learning: In the QCToQCO conversion pass (mlir/lib/Conversion/QCToQCO/QCToQCO.cpp), the dynamic legality checks for func operations (func::CallOp, func::FuncOp, func::ReturnOp) and scf operations assume that operations use either all classical types or all qubit types, never mixed. Therefore, checking for the presence of qc::QubitType in operands or arguments is sufficient to determine if conversion is needed—there is no need to check both operands and results separately.
<!-- </add_learning]
📚 Learning: 2025-12-02T07:37:46.860Z
Learnt from: MatthiasReumann
Repo: munich-quantum-toolkit/core PR: 1301
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp:144-151
Timestamp: 2025-12-02T07:37:46.860Z
Learning: In MLIR transformation code (mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/sc/NaiveRoutingPass.cpp and similar), when inserting operations before a target operation, prefer `rewriter.setInsertionPoint(op)` over `rewriter.setInsertionPointAfter(op->getPrevNode())`. The former is cleaner, avoids null pointer edge cases (when op is first in block), and is semantically clearer.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T17:31:22.110Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:816-862
Timestamp: 2026-01-10T17:31:22.110Z
Learning: In mlir/lib/Conversion/QCOToQC/QCOToQC.cpp, when creating scf::IfOp with `rewriter.create<scf::IfOp>(loc, types, condition, false)`, passing `false` for the withElseRegion parameter creates an else region that exists but contains no blocks. Therefore, inlineRegionBefore can be called on the else region without checking if it exists, and there's no need to erase an auto-created empty block from the else region (unlike the then region, which does get an auto-created empty block that must be erased).
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T15:54:56.832Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:891-923
Timestamp: 2026-01-10T15:54:56.832Z
Learning: In MLIR conversion code (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp), scf::WhileOp construction only creates empty regions, not blocks. After creating a WhileOp with `rewriter.create<scf::WhileOp>(...)`, the first calls to `rewriter.createBlock(&whileOp.getBefore(), ...)` and `rewriter.createBlock(&whileOp.getAfter(), ...)` create the initial blocks in those empty regions, so no block erasure is needed. This differs from scf::IfOp, which auto-creates an empty block that must be erased when using `inlineRegionBefore()`.
<!-- <review_comment_addressed>
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T23:44:39.930Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Quartz/IR/Operations/StandardGates/PhaseOp.cpp:0-0
Timestamp: 2025-12-08T23:44:39.930Z
Learning: In MLIR code under any mlir/ directory, avoid using const qualifiers on core MLIR types in function parameters/signatures (e.g., Value, Type, Attribute, Operation*, Block*, Region*, etc.). This aligns with MLIR's design rationale and should be applied to C++ source files (e.g., .cpp) within mlir/; see https://mlir.llvm.org/docs/Rationale/UsageOfConst/ for details.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-10T18:49:44.352Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Conversion/QCOToQC/QCOToQC.cpp:1045-1119
Timestamp: 2026-01-10T18:49:44.352Z
Learning: The QCOToQC conversion pass (mlir/lib/Conversion/QCOToQC/QCOToQC.cpp) does not need defensive mixed-type checks in its func conversion patterns (ConvertQCOFuncFuncOp, ConvertQCOFuncCallOp, ConvertQCOFuncReturnOp) because the conversion workflow always starts from QC to QCO, and the QCToQCO pass already enforces through its dynamic legality checks that func operations contain only qubit types (no mixed classical/quantum). This upstream guarantee justifies the all-qubit assumptions in QCOToQC patterns.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-17T11:32:45.843Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/MQTOpt/Transforms/Transpilation/LayeredUnit.cpp:83-86
Timestamp: 2025-12-17T11:32:45.843Z
Learning: In the mlir portion of munich-quantum-toolkit/core, prefer marking free functions as static (static linkage) over placing them in anonymous namespaces, per the clang-tidy rule llvm-prefer-static-over-anonymous-namespace. Do not apply this to type/class definitions; they may continue to use anonymous namespaces. This guideline should be checked across C++ source files under mlir/ (e.g., any free function in LayeredUnit.cpp) to ensure free functions have static linkage, while types/classes can remain in anonymous namespaces.
Applied to files:
mlir/unittests/Dialect/Utils/test_utils.cppmlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T23:41:55.972Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:96-117
Timestamp: 2025-12-08T23:41:55.972Z
Learning: In the QIR (Quantum Intermediate Representation) Builder (mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp), the `ptrCache` is intentionally shared between qubit and result pointer creation (in `staticQubit()` and `measure()` methods) because QIR uses opaque pointers and `inttoptr` conversions for both qubits and results. For any given index N, the LLVM IR pointer representation is identical whether it represents a qubit or a result, so the pointer only needs to be created once and can be safely reused across both contexts.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-12T12:05:47.463Z
Learnt from: li-mingbao
Repo: munich-quantum-toolkit/core PR: 1396
File: mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp:599-757
Timestamp: 2026-01-12T12:05:47.463Z
Learning: In the QCO builder (mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp), the SCF and func builder methods (scfFor, scfWhile, scfIf, funcCall) assume that all operands passed to them are qubit types (qco.qubit). The current validation through updateQubitTracking is sufficient for this use case. The same assumption applies to the QC builder methods.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-14T17:02:02.997Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp:187-210
Timestamp: 2025-12-14T17:02:02.997Z
Learning: In the Flux dialect builder (mlir/lib/Dialect/Flux/Builder/FluxProgramBuilder.cpp), the coding style relies on implicit conversion from Value to ValueRange in ctrl() calls (e.g., `ctrl(control, {}, ...)` instead of `ctrl(ValueRange{control}, ValueRange{}, ...)`). This pattern is used consistently throughout all macro-generated methods and should be maintained for consistency.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2026-01-04T22:25:13.956Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1356
File: src/qir/runner/Runner.cpp:70-76
Timestamp: 2026-01-04T22:25:13.956Z
Learning: In src/qir/runner/Runner.cpp, the REGISTER_SYMBOL macro intentionally performs dual registration (both llvm::sys::DynamicLibrary::AddSymbol and adding to manualSymbols vector) to ensure symbol resolution works correctly when compiling with shared libraries enabled, even though it may appear redundant.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-10T02:19:50.634Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp:0-0
Timestamp: 2025-12-10T02:19:50.634Z
Learning: In QIRProgramBuilder::measure(Value, int64_t), ensure a cache check prevents duplicate measurements: if registerResultMap.find(key) succeeds, return early to avoid generating multiple mz calls for the same classical bit index and guarantee only one entry per index in the output. This enforces an override-like semantics where repeated measurements reuse the cached pointer without extra side effects. Apply this caching/early-return pattern to similar measurement paths in the QIR builder code to improve correctness and efficiency.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-10-09T13:25:36.887Z
Learnt from: DRovara
Repo: munich-quantum-toolkit/core PR: 1108
File: mlir/lib/Dialect/MQTOpt/Transforms/ReuseQubitsPattern.cpp:98-100
Timestamp: 2025-10-09T13:25:36.887Z
Learning: In MLIR code, when traversing parent operations to find a common block between two operations where one uses the result of another, explicit bounds checking is not necessary. MLIR's SSA properties and scope locality guarantees ensure that operations using results must be in compatible scopes and will always share a common ancestor in the operation hierarchy.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-09T00:55:11.926Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Operations/StandardGates/BarrierOp.cpp:45-54
Timestamp: 2025-12-09T00:55:11.926Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux), qubits follow single-user/linear-type semantics where each qubit value can only be consumed once, similar to MQTOpt. This invariant makes it safe to dereference getUsers().begin() in canonicalization patterns like MergeSubsequentBarrier in BarrierOp.cpp, as there will be at most one user per qubit output.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-17T17:44:31.349Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/include/mlir/Dialect/QCO/IR/QCOOps.td:259-259
Timestamp: 2025-12-17T17:44:31.349Z
Learning: In the QCO dialect (mlir/include/mlir/Dialect/QCO/IR/QCOOps.td), GPhaseOp intentionally uses `MemoryEffects<[MemWrite]>` instead of `Pure` to prevent the remove-dead-values pass from eliminating it. Since GPhaseOp is a zero-target operation with no result values, it would otherwise be removed by DCE, even though it has a meaningful effect on the global quantum state.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
📚 Learning: 2025-12-08T14:55:43.899Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1264
File: mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp:78-100
Timestamp: 2025-12-08T14:55:43.899Z
Learning: In the Flux dialect (mlir/lib/Dialect/Flux/IR/Modifiers/CtrlOp.cpp), GPhaseOp is a zero-target operation (global phase). When a CtrlOp wraps a GPhaseOp, it only has control qubits and no targets. The CtrlInlineGPhase canonicalization pattern correctly produces outputs only for the positive controls, not targets.
Applied to files:
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
🧬 Code graph analysis (1)
mlir/unittests/Dialect/Utils/test_utils.cpp (1)
mlir/include/mlir/Dialect/Utils/Utils.h (3)
mlir(19-72)nodiscard(31-42)nodiscard(52-70)
🪛 Cppcheck (2.19.0)
mlir/unittests/Dialect/Utils/test_utils.cpp
[information] 18-18: Include file
(missingIncludeSystem)
[information] 19-19: Include file
(missingIncludeSystem)
[information] 20-20: Include file
(missingIncludeSystem)
[information] 21-21: Include file
(missingIncludeSystem)
[information] 22-22: Include file
(missingIncludeSystem)
[information] 23-23: Include file
(missingIncludeSystem)
[information] 24-24: Include file
(missingIncludeSystem)
[information] 25-25: Include file
(missingIncludeSystem)
[information] 26-26: Include file
(missingIncludeSystem)
[information] 27-27: Include file
(missingIncludeSystem)
mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp
[information] 26-26: Include file
(missingIncludeSystem)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Debug) / 🏁 windows-2025 msvc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Release) / 🍎 macos-15 clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04-arm, gcc, Release) / 🐧 ubuntu-24.04-arm gcc Release
- GitHub Check: 🇨 Test 🏁 (windows-11-arm, msvc, Release) / 🏁 windows-11-arm msvc Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Debug) / 🐧 ubuntu-24.04 gcc Debug
- GitHub Check: 🇨 Test 🍎 (macos-15, clang, Debug) / 🍎 macos-15 clang Debug
- GitHub Check: 🇨 Test 🏁 (windows-2025, msvc, Release) / 🏁 windows-2025 msvc Release
- GitHub Check: 🇨 Test 🍎 (macos-15-intel, clang, Release) / 🍎 macos-15-intel clang Release
- GitHub Check: 🇨 Test 🐧 (ubuntu-24.04, gcc, Release) / 🐧 ubuntu-24.04 gcc Release
- GitHub Check: 🇨 Coverage / 📈 Coverage
- GitHub Check: 🇨 Lint / 🚨 Lint
🔇 Additional comments (9)
mlir/unittests/Dialect/Utils/test_utils.cpp (2)
31-53: LGTM! Clean migration to ImplicitLocOpBuilder pattern.The refactoring properly adopts the
ImplicitLocOpBuilderand static create APIs. UsingFileLineColLocwith a descriptive name"<utils-test-builder>"is a good improvement overUnknownLocfor debugging. TheOwningOpRef<ModuleOp>correctly manages the module lifecycle, and the[[nodiscard]]attribute oncreateAdditionappropriately warns if the return value is ignored.
55-133: Test cases properly updated to new APIs.All tests correctly use the static
createAPIs and explicit.getResult()calls for value extraction. ThevalueToDoubleFoldedConstanttest appropriately usesdyn_cast(line 128) to safely extract theConstantOpfrom the folded result before accessing its value.mlir/lib/Dialect/QIR/Builder/QIRProgramBuilder.cpp (7)
38-92: LGTM!The constructor and
initialize()method correctly implement the implicit-location pattern. The 4-block QIR Base Profile structure is properly established, and all operation creations consistently use theOp::create(*this, ...)API.
94-134: LGTM!The
staticQubitandallocQubitRegistermethods correctly use the pointer cache and delegate appropriately. The implicit-loc pattern is consistently applied.
220-246: LGTM!This overload correctly requires pre-allocation of the classical register and properly retrieves the result pointer from
registerResultMap.
248-264: LGTM!The
resetmethod correctly usesInsertionGuardand places the reset call in the measurements block.
270-330: LGTM!The
createCallOpmethod correctly separates constant creation (entry block) from call insertion (body block) usingInsertionGuard. The argument ordering follows QIR conventions.
342-555: LGTM!The macro-generated gate operations consistently delegate to
createCallOpwith appropriate arguments. The pattern is well-structured across all gate variants.
568-645: LGTM!The
generateOutputRecordingandfinalizemethods correctly implement output recording with deterministic ordering. The implicit-location pattern is consistently applied throughout.
Signed-off-by: burgholzer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 makes use of the newly-discovered
ImplicitLocOpBuilderin MLIR, which makes it way more convenient to use the OpBuilder interface, because it does not require specifying a location forcreatecalls (a place where we, more often than not, simply usedUnknownLoc).Furthermore, this PR also introduces explicit locations for the builders, which seems to be good practice in general and is favored over using the unknown location.
This also refactors the QIR Program Builder to inherit from MLIR's builder class, which it did not previously.
Overall, this streamlines quite a bit of code, including the tests recently added in #1443.
Checklist: