Lunderberg commented on PR #15916: URL: https://github.com/apache/tvm/pull/15916#issuecomment-1785503708
I think the major issue is the inconsistency of normalization. From the code itself, there is no clear expectation of this being the normal form for Relax, and the majority of the handling in the codebase Behavior suggesting that the de facto normal form allows Tuple variables * Normalization does not suppress tuple variables when passed to `BlockBuilder::Emit`. * Normalization does not inline tuple variables when used in a `call_tir` expression. * Normalization does not unwrap `TupleGetItem` calls to a suppressed tuple variable. * A `relax.Call` node may return a `relax.Tuple`, which is bound to a variable, and is then immediately used as the argument tuple of `call_tir`. * Tuples are allowed to be on the RHS of an expression, bound to a variable. * User-written TVMScript can define `my_var = (x,y)`. * Pattern-matching, which acts on a is allowed to replace a `relax.Call` with a `relax.Tuple`. (e.g. replacing the output of `R.split` with a tuple of `R.strided_split` calls.) Pattern-matching operates on a `relax.Expr` level. Behavior suggesting that the de facto normal form requires in-line Tuple variables * Normalization keeps `relax.Tuple` arguments in-place, unlike `relax.Call` arguments which are extracted out into separate bindings. In order for in-line tuples be the normalization used in practice, and not just in principle, the `Normalizer` would need to have a number of changes made to it. It would need to check for `relax::Tuple` in `BlockBuilder::Emit`, and suppress the binding made for it. Whenever the `relax::Var` representing the suppressed tuple is used, the `Normalizer` would need to replace it with the tuple itself, including when used in nested tuple expressions. This seems much more fragile and easy to break than having the normalization of tuples follow the same rules as the normalization of other relax expressions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
