-
Notifications
You must be signed in to change notification settings - Fork 46
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
load all dialects preemptively #1584
load all dialects preemptively #1584
Conversation
Wait, was this not solved? 😅 I remember this either went away on its own or there was a hack? @sengthai |
At that time, that error disappeared, then it came back and forth. However, It was entirely solved by adding catalyst/mlir/include/Catalyst/Transforms/Passes.td Lines 184 to 192 in 8b385c1
|
Thanks! Yes, this is not a viable solution in the long term. I think we may need to update the source upstream. |
a854b46
to
d7d31ce
Compare
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.
Thanks for this PR, Erick. I tested ppm's passes, and they work great! I also went ahead and removed the empty dependentDialects
in the latest commit.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1584 +/- ##
==========================================
+ Coverage 96.47% 96.60% +0.13%
==========================================
Files 80 80
Lines 8556 8606 +50
Branches 819 832 +13
==========================================
+ Hits 8254 8314 +60
+ Misses 246 238 -8
+ Partials 56 54 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This one is hard to test for plugins because the standalone plugin in MLIR does not have a pass that depends on the standalone dialect. So I would need to create a pass to test it. I propose we merge this. We see that it already solves the issue with the QEC dialects. |
Patrick Hopf successfully tested this out. I'll be merging after standup. |
**Context:** The transform dialect appears not to load dependent dialects of transformations. **Description of the Change:** Load all dialects preemptively. **Benefits:** Third party developers (and even first party developers) can generate operations in unloaded dialects from within the apply-sequence-pass. **Possible Drawbacks:** This is a workaround. We should investigate some time to see where exactly the error comes from. [sc-88453] --------- Co-authored-by: Sengthai Heng <[email protected]>
Context: The transform dialect appears not to load dependent dialects of transformations.
Description of the Change: Load all dialects preemptively.
Benefits: Third party developers (and even first party developers) can generate operations in unloaded dialects from within the apply-sequence-pass.
Possible Drawbacks: This is a workaround. We should investigate some time to see where exactly the error comes from.
[sc-88453]