Uh oh!
There was an error while loading. Please reload this page.
[Unity][Pass] Fixpoint simplification - #15474
Conversation
tvm-bot
commented
Aug 3, 2023
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
1 similar comment
tvm-bot
commented
Aug 3, 2023
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
slyubomirsky
commented
Aug 9, 2023
Looks like a spurious/flaky test failure |
slyubomirsky
commented
Aug 9, 2023
@tvm-bot rerun |
Failed to re-run CI in https://github.com/apache/tvm/actions/runs/5812197184 Detailswith response |
kparzysz-quic
commented
Aug 10, 2023
All of this should be doable in a single execution of a sequence of optimizations. If we don't get it. we should examine our passes for deficiencies. Doing fixed point transformations is (IMO) a warning sign to look at what we're missing that we're trying to make up for with repeated iterations. Even in the pattern matching pass (where repetition is justifiable) we should have a max number of iterations to perform. |
slyubomirsky
commented
Aug 10, 2023
It's a tradeoff between increasing the complexity of the individual passes to handle more cases versus rerunning existing passes. I agree that running to fixpoint is a bit of a lazy "shotgun approach" but it could be useful in that it would allow for easily throwing in new passes to handle small transformations. |
kparzysz-quic
commented
Aug 10, 2023
I disagree with this approach. This is not the right way to compose transformations. |
slyubomirsky
commented
Aug 10, 2023
I'd be happy to discuss how the passes included should be modified to produce the behavior shown in the test cases, in that case. My concern is avoiding code duplication and mission creep for certain passes. |
kparzysz-quic
commented
Aug 10, 2023
Sure. I'll be out on vacation in the next 2 weeks, but I'd be happy to propose/work on improvements after I'm back. |
slyubomirsky
commented
Aug 10, 2023
I'll split the bugfix into its own PR then and we can leave this open for further discussion. |
3403be8 to
2b0b4caCompareslyubomirsky
commented
Nov 17, 2023
The specific motivation for this one has been superseded by later PRs like #15791 so there is no reason to keep this around. |
This PR implements a simple simplification pass that applies various other simplification passes until fixpoint (i.e., apply them until the module stops changing). This probably should not be part of the standard compilation, since it has the potential to repeat passes many times, but I doubt that we would be in danger of having it happen that much. I am certain that it terminates, since all of the passes included either make the module "more simplified" or leave it unchanged if they cannot find any opportunities to make changes (i.e., no pass has the potential to undo the work of other passes). Nevertheless, the pass includes an iteration limit in case passes are later added that in some situations do not converge (not recommended).
Incidentally, it also fixes a bug that I happened to discover in the binding canonicalization pass: It did not handle the case of a non-dataflow var being bound to a dataflow var and gave a segfault if it was encountered.A few points of discussion: