Skip to content

Commit 0eaccba

Browse files
authored
🐛 Rename CtrlOp region to region to avoid hiding trait method getBody() (#1430)
## Description Unfortunately, this became a little bit more cumbersome to figure out than expected. The automatically generated getter for the parameter "body" was hiding one of the trait interface methods. These methods do have a slightly different signature (`getBody()` returns a `Block*` and `getBodyRegion()` accepts an index parameter) which can lead to confusion and an inconsistent API. An alternative to this proposal could be simply the name "region" because `getRegion()` of the `OneRegion` trait has the same signature and thus the override would probably not be noticed. However, there would be two identical implementations for this method and this could make debugging harder. The downside to the current solution is that there are now three functions to access the region: `getRegion()` (by `OneRegion`), `getBodyRegion()` (by `SingleBlock`) and `getCtrlBodyRegion()` (by getter). ## Checklist: <!--- This checklist serves as a reminder of a couple of things that ensure your pull request will be merged swiftly. --> - [ ] The pull request only contains commits that are focused and relevant to this change. - [ ] I have added appropriate tests that cover the new/changed functionality. - [ ] I have updated the documentation to reflect these changes. - [ ] I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals. - [ ] I have added migration instructions to the upgrade guide (if needed). - [ ] The changes follow the project's style guidelines and introduce no new warnings. - [ ] The changes are fully tested and pass the CI checks. - [ ] I have reviewed my own code changes.
1 parent 187e6a2 commit 0eaccba

File tree

7 files changed

+15
-13
lines changed

7 files changed

+15
-13
lines changed

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ This project adheres to [Semantic Versioning], with the exception that minor rel
1111

1212
### Added
1313

14-
- ✨ Add initial infrastructure for new QC and QCO MLIR dialects ([#1264], [#1402]) ([**@burgholzer**], [**@denialhaag**])
14+
- ✨ Add initial infrastructure for new QC and QCO MLIR dialects ([#1264], [#1402], [#1428], [#1430]) ([**@burgholzer**], [**@denialhaag**], [**@taminob**])
1515
- ✨ Return device handle from `add_dynamic_device_library` for direct backend creation ([#1381]) ([**@marcelwa**])
1616
- ✨ Add IQM JSON support for job submission in Qiskit-QDMI Backend ([#1375], [#1382]) ([**@marcelwa**], [**@burgholzer**])
1717
- ✨ Add authentication support for QDMI sessions with token, username/password, auth file, auth URL, and project ID parameters ([#1355]) ([**@marcelwa**])
@@ -294,6 +294,8 @@ _📚 Refer to the [GitHub Release Notes](https://github.com/munich-quantum-tool
294294

295295
<!-- PR links -->
296296

297+
[#1430]: https://github.com/munich-quantum-toolkit/core/pull/1430
298+
[#1428]: https://github.com/munich-quantum-toolkit/core/pull/1428
297299
[#1415]: https://github.com/munich-quantum-toolkit/core/pull/1415
298300
[#1414]: https://github.com/munich-quantum-toolkit/core/pull/1414
299301
[#1413]: https://github.com/munich-quantum-toolkit/core/pull/1413

mlir/include/mlir/Dialect/QC/IR/QCOps.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -948,8 +948,8 @@ def CtrlOp : QCOp<"ctrl",
948948
}];
949949

950950
let arguments = (ins Arg<Variadic<QubitType>, "the control qubits", [MemRead, MemWrite]>:$controls);
951-
let regions = (region SizedRegion<1>:$body);
952-
let assemblyFormat = "`(` $controls `)` $body attr-dict `:` type($controls)";
951+
let regions = (region SizedRegion<1>:$region);
952+
let assemblyFormat = "`(` $controls `)` $region attr-dict `:` type($controls)";
953953

954954
let extraClassDeclaration = [{
955955
[[nodiscard]] UnitaryOpInterface getBodyUnitary();

mlir/include/mlir/Dialect/QCO/IR/QCOOps.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,10 +1067,10 @@ def CtrlOp : QCOOp<"ctrl", traits =
10671067
let arguments = (ins Arg<Variadic<QubitType>, "the control qubits", [MemRead]>:$controls_in,
10681068
Arg<Variadic<QubitType>, "the target qubits", [MemRead]>:$targets_in);
10691069
let results = (outs Variadic<QubitType>:$controls_out, Variadic<QubitType>:$targets_out);
1070-
let regions = (region SizedRegion<1>:$body);
1070+
let regions = (region SizedRegion<1>:$region);
10711071
let assemblyFormat = [{
10721072
`(` $controls_in `)` $targets_in
1073-
$body attr-dict `:`
1073+
$region attr-dict `:`
10741074
`(` `{` type($controls_in) `}` ( `,` `{` type($targets_in)^ `}` )? `)`
10751075
`->`
10761076
`(` `{` type($controls_out) `}` ( `,` `{` type($targets_out)^ `}` )? `)`

mlir/lib/Conversion/QCOToQC/QCOToQC.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,8 @@ struct ConvertQCOCtrlOp final : OpConversionPattern<qco::CtrlOp> {
777777
auto qcoOp = rewriter.create<qc::CtrlOp>(op.getLoc(), qcControls);
778778

779779
// Clone body region from QCO to QC
780-
auto& dstRegion = qcoOp.getBody();
781-
rewriter.cloneRegionBefore(op.getBody(), dstRegion, dstRegion.end());
780+
auto& dstRegion = qcoOp.getRegion();
781+
rewriter.cloneRegionBefore(op.getRegion(), dstRegion, dstRegion.end());
782782

783783
// Replace the output qubits with the same QC references
784784
rewriter.replaceOp(op, adaptor.getOperands());

mlir/lib/Conversion/QCToQCO/QCToQCO.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,8 +1127,8 @@ struct ConvertQCCtrlOp final : StatefulOpConversionPattern<qc::CtrlOp> {
11271127
state.targetsIn.try_emplace(state.inCtrlOp, qcoTargets);
11281128

11291129
// Clone body region from QC to QCO
1130-
auto& dstRegion = qcoOp.getBody();
1131-
rewriter.cloneRegionBefore(op.getBody(), dstRegion, dstRegion.end());
1130+
auto& dstRegion = qcoOp.getRegion();
1131+
rewriter.cloneRegionBefore(op.getRegion(), dstRegion, dstRegion.end());
11321132

11331133
rewriter.eraseOp(op);
11341134
return success();

mlir/lib/Dialect/QC/IR/Modifiers/CtrlOp.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ struct CtrlInlineGPhase final : OpRewritePattern<CtrlOp> {
109109
} // namespace
110110

111111
UnitaryOpInterface CtrlOp::getBodyUnitary() {
112-
return llvm::dyn_cast<UnitaryOpInterface>(&getBody().front().front());
112+
return llvm::dyn_cast<UnitaryOpInterface>(&getBody()->front());
113113
}
114114

115115
size_t CtrlOp::getNumQubits() { return getNumTargets() + getNumControls(); }
@@ -173,7 +173,7 @@ void CtrlOp::build(OpBuilder& odsBuilder, OperationState& odsState,
173173
}
174174

175175
LogicalResult CtrlOp::verify() {
176-
auto& block = getBody().front();
176+
auto& block = *getBody();
177177
if (block.getOperations().size() != 2) {
178178
return emitOpError("body region must have exactly two operations");
179179
}

mlir/lib/Dialect/QCO/IR/Modifiers/CtrlOp.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ struct CtrlInlineId final : OpRewritePattern<CtrlOp> {
152152
} // namespace
153153

154154
UnitaryOpInterface CtrlOp::getBodyUnitary() {
155-
return llvm::dyn_cast<UnitaryOpInterface>(&getBody().front().front());
155+
return llvm::dyn_cast<UnitaryOpInterface>(&getBody()->front());
156156
}
157157

158158
size_t CtrlOp::getNumQubits() { return getNumTargets() + getNumControls(); }
@@ -272,7 +272,7 @@ void CtrlOp::build(OpBuilder& odsBuilder, OperationState& odsState,
272272
}
273273

274274
LogicalResult CtrlOp::verify() {
275-
auto& block = getBody().front();
275+
auto& block = *getBody();
276276
if (block.getOperations().size() != 2) {
277277
return emitOpError("body region must have exactly two operations");
278278
}

0 commit comments

Comments
 (0)