✨ Add QCO If operation#1506
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/lib/Dialect/QCO/IR/QCOOps.cpp`:
- Around line 118-152: The parser currently deduplicates operands after parsing
then/else aliasing, which can mask ordering or set mismatches; update
parseIfOpAliasing to validate that the aliasing lists produced by
parseTargetAliasing for thenRegion and elseRegion are identical (same operands
in the same order) before performing the llvm::erase_if dedupe step, and return
failure() if they differ so IfOp::verify() and operand-to-block-argument
alignment remain correct; locate this check inside parseIfOpAliasing immediately
after both parseTargetAliasing calls and before the duplicate-removal logic.
In `@mlir/unittests/programs/qco_programs.cpp`:
- Around line 2017-2050: In nestedTrueIf and nestedFalseIf fix incorrect
brace-initializer conversions from ValueRange/ValueRange-returning qcoIf to
SmallVector: where the lambdas return
llvm::SmallVector<mlir::Value>{innerQubits} or
llvm::SmallVector<mlir::Value>{innerResult}, replace those with
llvm::to_vector(innerQubits) and llvm::to_vector(innerResult) respectively so
the ValueRange is properly converted to a SmallVector; update the inner lambdas
passed to qcoIf (referenced symbols: nestedTrueIf, nestedFalseIf, innerQubits,
innerResult, qcoIf) accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/unittests/programs/qco_programs.cpp`:
- Around line 2026-2027: Remove the stray unreachable semicolon after the return
expression in the function that returns llvm::to_vector(innerResult) (the line
showing "return llvm::to_vector(innerResult); ;"); locate the return statement
(e.g., where llvm::to_vector(innerResult) is returned) and delete the extra
trailing ";" so the function has a single valid return statement.
|
@burgholzer |
Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
burgholzer
left a comment
There was a problem hiding this comment.
Thanks @li-mingbao 🙏🏼
I only have one final round of comments for the tests, which I have not looked at in detail before. These should be very easy to address and then this can go in. Looking forward to it!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mlir/unittests/Dialect/QCO/IR/test_qco_ir.cpp`:
- Around line 120-138: The test suite name has a typo: change the instantiation
identifier QCOIflOpTest to QCOIfOpTest in the INSTANTIATE_TEST_SUITE_P call so
the suite name matches the IfOp naming; update the symbol QCOIflOpTest to
QCOIfOpTest where used in that instantiation (INSTANTIATE_TEST_SUITE_P) to
correct the spelling.
burgholzer
left a comment
There was a problem hiding this comment.
Nice! Let's get this in. I'll quickly apply the one suggestion to the test.
Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
|
Ah damn. I obviously messed up one of the tests with the last commit 🙃 |
d9209b7
into
munich-quantum-toolkit:main
Description
This PR adds the
qco.ifoperation to the QCO dialect. This operation is similar to thescf.ifoperation but it takes an additional list of qubits as input that are passed down to the branches as block arguments. This is used in order to support linar typing for qubit values.Example:
Checklist: