- User Since
- Jul 4 2021, 9:36 PM (126 w, 18 h)
Sep 30 2023
Aug 26 2023
Rebase to main.
I'm planning on accepting and landing this change soon because it is trivial and crucial.
Aug 25 2023
Please update the commit summary. It only contains the title currently. Also, change your revision and commit titles to [MLIR][transforms] Add a pass to sort commutative operands.
Aug 23 2023
Rename pass to RemoveDeadValues and propoagate this change to code comments and commit summary.
Call op arg operands handling.
Rebase to main
Aug 22 2023
Thanks for adding this. An initial comment:-
Thank you for working on this. There are a few issues here. Listing them below:-
Aug 21 2023
Update commit summary to reflect recent changes.
@Mogball and @jcai19, I have addressed all your comments. Thanks for the review!
@matthiaskramm, thank you for the clarification on the naming!
@mehdi_amini, awaiting your re-review and/or response, thanks! :)
Address all comments given so far.
Thanks for your comments, @Mogball. Working on addressing them.
Aug 18 2023
Aug 17 2023
Aug 16 2023
! In D157049#4577762, @srishti-pm wrote:
Again, it should be noted here that since this patch introduces this pass, it may not be perfect. The pass relies on liveness analysis which isn't perfect and liveness analysis relies on sparse backward dataflow which isn't perfect. But, theoretically speaking, if we assume that sparse backward dataflow analysis is perfect, liveness analysis is perfect, and this pass is perfect (as in, no forms of IR have been missed), the above holds true. I am also parallely trying to make these things perfect. Some patches that work towards that are here: https://reviews.llvm.org/D156376, https://reviews.llvm.org/D157261. It is potentially a long way to go. Hopefully we can achieve that 😅
Does this make sense?
Aug 15 2023
@mehdi_amini , theoretically, do you agree liveness analysis gets no benefit from canonicalization?
I can understand the theoretical view, but that seems a bit simplistic to me: liveness analysis has to be conservative, and it's not clear to me that canonicalization can't just improve the capability of the analysis.
I don't understand your question. Can you kindly re-state it?
Fix handling of region branch and region branch terminator ops based on the recent changes to their interface methods.
Aug 14 2023
Aug 11 2023
Rebase to main.
Aug 10 2023
Rebased to main.
Address Jian's comment.
Aug 9 2023
Can you make the summary precise? It's a bit long-winded and the bugs aren't clear from the summary. If you can explain a bug in one line or with a small example, it will be great. You don't need to explain the older functionality in detail.
All comments are addressed here and it is approved by two reviewers. Will wait one more day before landing this in case other reviewers would like to suggest changes too :)
Address Jian's comments.
All comments given so far in this patch have been addressed.
Rebased to main.
Aug 8 2023
Aug 7 2023
@mehdi_amini, I have updated the unit tests. Here's how you can navigate them:-
Update unit tests.
Aug 6 2023
Enhance test cases to reveal functionality difference from the canonicalize pass.
Aug 3 2023
More tests and notes to be added.
Add notes about comparisons with the canonicalize pass. More tests and notes to be added.
Enhance unit tests of cleanSimpleOp().
Update commit summary.