-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[TIR] Flatten SeqStmt on construction #14492
Conversation
Previously, SeqStmt could be nested, making a distinction between the nested `SeqStmt({SeqStmt({a,b}), c})` and the flat `SeqStmt({a,b,c})`, even though the two are semantically equivalent. This also caused an issue with round-trips through TVMScript, which does not preserve this distinction. This commit updates the `SeqStmt` constructor and the `SeqStmt` visitor in `StmtMutator` to flatten nested sequential statements provided.
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 |
The flattening still can produce ambiguous structure for `SeqStmt` of size 0 and 1. Normalizing so that `SeqStmt::Flatten` will automatically unwrap when flattening produces a single statement, and will convert to `Evaluate(0)` when flattening removes all statements.
If neither flattening nor unwrapping is required, the returned SeqStmt should be the same object as the argument.
Otherwise, the SeqStmt removes the "Evaluate(0)" before we can get to the part that we want to test.
8be5885
to
a5eff13
Compare
Sorry for the late response. Could you please rebase it again and let's merge it? |
No worries (this has been on my back burner as well). Main has been merged into the PR branch to resolve conflicts, and now running through CI. |
@tvm-bot rerun |
Thanks @Lunderberg for the PR :) |
Previously, SeqStmt could be nested, making a distinction between the nested
SeqStmt({SeqStmt({a,b}), c})
and the flatSeqStmt({a,b,c})
, even though the two are semantically equivalent. This also caused an issue with round-trips through TVMScript, which does not preserve this distinction.This commit updates the
SeqStmt
constructor and theSeqStmt
visitor inStmtMutator
to flatten nested sequential statements provided.This is part of changes described in #14486, to improve round-trip failures that occur in lowering.