-
Notifications
You must be signed in to change notification settings - Fork 737
Redefine ApplyRegisteredPass in the Transform dialect #7956
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
Redefine ApplyRegisteredPass in the Transform dialect #7956
Conversation
|
Hello. You may have forgotten to update the changelog!
|
|
Thanks! I will take a look in a bit 👍 |
pennylane/compiler/python_compiler/transforms/api/transform_interpreter.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Mudit Pandey <[email protected]>
|
Is this change necessary because we are ahead of xdsl's LLVM version? In other words, can this be reverted once xdsl updates their transform dialect to reflect the new LLVM change? |
Yes |
paul0403
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pre-approving given the resolution of my comments since I'm away next week!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7956 +/- ##
==========================================
- Coverage 99.68% 99.68% -0.01%
==========================================
Files 545 543 -2
Lines 56658 55894 -764
==========================================
- Hits 56482 55718 -764
Misses 176 176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mudit2812
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, just one non-blocking comment. Looks great!
**Context:** The local transform dialect was created in PennyLaneAI/pennylane#7956 because xdsl was not up to date with Catalyst's LLVM version which updated the transform dialect to accept dictionaries as options. Since xDSL has now updated their LLVM version, we can remove the local dialect. **Description of the Change:** Remove the local shim for the transform dialect and use the xdsl upstream one. **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:**
Context: Catalyst is updating LLVM, which includes a redefinition of this operation in the transform dialect.
Note:
Only merge after PennyLaneAI/catalyst#1916
Description of the Change: Redefine the ApplyRegisteredPass in the transform dialect to support Catalyst's generic format.
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-96972]