-
Notifications
You must be signed in to change notification settings - Fork 35
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
Implement detensorize-scf pass (for / if / while) #1075
Conversation
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.
Just a few general comments:
- We can separate this into multiple functions
- If we can, I think we can separate this into two PatternOpRewriters, one for the
IfOp
, and one for theForOp
- The walkers will walk in post-order. I think you may be able to get access to the
YieldOp
for the current op by looking at the terminator of the basic block of the op you are looking at. This could clean the logic behindfirst_yield
.
Hi Erick, thanks for the feedback.
|
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 Vincent, this is nice work 💯
I left some info on things that would definitely not be obvious to someone starting to learn MLIR, hopefully you find these helpful :)
(There are still so many functions in the library I don't know about either 😅 )
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.
Really nice work, it's coming together very well 💯
We talked about a slightly different structure for the while pattern so I'm leaving the for loop / while loop review until after that, but the if pattern is looking really good with some small cleanups :)
Co-authored-by: David Ittah <[email protected]>
Co-authored-by: David Ittah <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1075 +/- ##
=======================================
Coverage 97.87% 97.87%
=======================================
Files 76 76
Lines 10847 10847
Branches 1282 1282
=======================================
Hits 10616 10616
Misses 179 179
Partials 52 52 ☔ View full report in Codecov by Sentry. |
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.
Awesome
**Context:** After applying `func.func(linalg-detensorize{aggressive-mode})`, operations in the SCF dialect still request scalar tensors for their operands/arguments and yield scalar tensor results as well. This leads to the introduction of several tensor.from_elements and tensor.extract operations around the operation interface. **Description of the Change:** Introduce pass to detensorize `scf::for / if / while`. The while detensorization is currently limited to [cases](https://github.com/PennyLaneAI/catalyst/pull/1075/files#diff-f6ee410945b4f019017fb45560a6ae38d9568439e3933ff7aa1673b1094ecffbR217-R225) where the before/after blocks have the same operands. **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** [sc-71505] --------- Co-authored-by: David Ittah <[email protected]>
Context:
After applying
func.func(linalg-detensorize{aggressive-mode})
, operations in the SCF dialect still request scalar tensors for their operands/arguments and yield scalar tensor results as well. This leads to the introduction of several tensor.from_elements and tensor.extract operations around the operation interface.Description of the Change:
Introduce pass to detensorize
scf::for / if / while
. The while detensorization is currently limited to cases where the before/after blocks have the same operands.Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-71505]