-
-
Notifications
You must be signed in to change notification settings - Fork 72
✨ Add Dead Gate Elimination Pattern #1755
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
91b3ef9
ba57aa2
b83afab
4243ac0
a50032b
d8c9ad5
9717547
2d65418
278a54e
44eaf7e
8b3af43
6998110
d56ccaf
fb9fffb
44f295a
9e8e711
b6cbf8e
dee0752
244f8bb
6eaac40
07a3f4d
cab4155
6f1c64e
7e63992
2e9f4e6
29b0cbe
225c88f
4c5a22d
fd40b82
e2ac131
2d70d31
88bac4e
8a519b8
8726654
2851aee
304f530
29d9627
8ca4c96
7ae3898
0567332
bba85e8
62f1ab5
fd72319
fc9a663
f18025b
20426f8
afd395d
aeb9838
bb8152f
973cf43
12aa9d1
ff48f93
dc89944
67d21dd
6dbd019
b5fcbb0
92eb0c7
20468cf
5917c1b
21f01f0
2b2d9b4
472361b
065d23a
98fafe6
224bf4a
5635099
88d1886
a8e7acc
6b53a71
af1009f
1507296
e327250
7035f0d
915052d
1dfe637
360d849
56bf24b
3cc7522
a33d5d3
a179e89
4eba897
0139926
ec6777d
6ab9653
0cd289a
900c370
93b32c4
75767b2
5a775ac
2db079d
5ba7983
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,12 +9,29 @@ | |
| */ | ||
|
|
||
| #include "mlir/Dialect/QCO/IR/QCOOps.h" | ||
| #include "mlir/Dialect/QCO/QCOUtils.h" | ||
|
|
||
| #include <mlir/Support/LogicalResult.h> | ||
|
|
||
| using namespace mlir; | ||
| using namespace mlir::qco; | ||
|
|
||
| namespace { | ||
|
|
||
| /** | ||
| * @brief Remove dead measurements. | ||
| */ | ||
| struct DeadMeasurementRemoval final : OpRewritePattern<MeasureOp> { | ||
| using OpRewritePattern::OpRewritePattern; | ||
|
Check warning on line 25 in mlir/lib/Dialect/QCO/IR/Operations/MeasureOp.cpp
|
||
|
|
||
| LogicalResult matchAndRewrite(MeasureOp op, | ||
| PatternRewriter& rewriter) const override { | ||
| return checkAndRemoveDeadGate(op, rewriter); | ||
| } | ||
| }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point, I am not quite sure there is much value in the shared helper function, when the implementation here would essentially be 5 lines of code. Same comment applies to the ResetOp
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, true. When I originally implemented it I was able to use it everywhere but due to all the edge cases it ended up getting s specialized that I might as well have individual implementations for all uses. I did that now. Anyways, I have done something else for now: The helper function is now called In an ideal world where we really want to minimize code reuse, we might even want to implement some "remove this operation" helper methods for every
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to the comment above: Wouldn't it be simpler if this were simply a canonicalization of the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the answer above. I honestly don't really feel like the way it's implemented right now is wrong. Having the special operation handling in the gate files tells you exectly: "Okay I'm looking at the handling for an
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not saying it is wrong by any means; it is obviously working.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's how I implemented it now. I agree. |
||
|
|
||
| } // namespace | ||
|
|
||
| LogicalResult MeasureOp::verify() { | ||
| const auto registerName = getRegisterName(); | ||
| const auto registerSize = getRegisterSize(); | ||
|
|
@@ -37,3 +54,8 @@ | |
| } | ||
| return success(); | ||
| } | ||
|
|
||
| void MeasureOp::getCanonicalizationPatterns(RewritePatternSet& results, | ||
| MLIRContext* context) { | ||
| results.add<DeadMeasurementRemoval>(context); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,39 @@ TEST_F(QCOTest, BuilderRejectsMixedStaticAndDynamicQubitAllocationModes) { | |
| "Cannot mix dynamic and static qubit allocation modes"); | ||
| } | ||
|
|
||
| TEST_F(QCOTest, CheckDeadGateElimination) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that it does not necessarily make sense to define such specialized programs in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that this is not the cleanest place to put and handle the test. I feel like, ideally, there should be a specialised test file for general canonicalisations. I'll have a quick look and then either try that or implement it the way you suggested.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ended up moving the program definitions to |
||
| QCOProgramBuilder builder(context.get()); | ||
| builder.initialize(); | ||
| auto q0_0 = builder.allocQubit(); | ||
| auto q1_0 = builder.allocQubit(); | ||
| auto q0_1 = builder.h(q0_0); | ||
| auto [q0_2, q1_1] = builder.cx(q0_1, q1_0); | ||
| auto q1_2 = builder.h(q1_1); | ||
| builder.sink(q0_2); | ||
| builder.sink(q1_2); | ||
| auto module = builder.finalize(); | ||
|
|
||
| QCOProgramBuilder reference(context.get()); | ||
| reference.initialize(); | ||
| auto r0 = reference.allocQubit(); | ||
| auto r1 = reference.allocQubit(); | ||
| reference.sink(r0); | ||
| reference.sink(r1); | ||
|
burgholzer marked this conversation as resolved.
Outdated
|
||
| auto ref = reference.finalize(); | ||
|
|
||
| ASSERT_TRUE(module); | ||
| EXPECT_TRUE(verify(*module).succeeded()); | ||
| EXPECT_TRUE(runQCOCleanupPipeline(module.get()).succeeded()); | ||
| EXPECT_TRUE(verify(*module).succeeded()); | ||
|
|
||
| ASSERT_TRUE(ref); | ||
| EXPECT_TRUE(verify(*ref).succeeded()); | ||
| EXPECT_TRUE(runQCOCleanupPipeline(ref.get()).succeeded()); | ||
| EXPECT_TRUE(verify(*ref).succeeded()); | ||
|
|
||
| EXPECT_TRUE(areModulesEquivalentWithPermutations(module.get(), ref.get())); | ||
| } | ||
|
DRovara marked this conversation as resolved.
Outdated
|
||
|
|
||
| TEST_F(QCOTest, DirectIfBuilder) { | ||
| // Test If construction directly | ||
| QCOProgramBuilder builder(context.get()); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.