lazycal opened a new pull request #10156: URL: https://github.com/apache/tvm/pull/10156
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread. I'm reviewing this function after PR #10118, and found several buggy places. For instance, - it does not respect the original input tensor's layout but always reset it to the other input's layout (see `test_broadcast_respect_input_layouts`); - it assumes one can always only transform one layout to adapt to the other. However, this is not always possible, see `test_broadcast_non_adaptable`. - `AdjustSubordinateFactors` should also be called in the else branch https://github.com/apache/tvm/blob/6a274af9cb4e7baf6d9dd06f0fb38bea5ac3154a/src/relay/transforms/infer_layout_utils.h#L222-L234. (see `test_alter_layout_nonscalar_broadcast`) This PR rewrites `BinaryBroadcastLayoutHelper` a bit, and I tried to make it as backward compatible as possible. ## Misc BTW I seem to spot another bug when I was working on this PR. So I sometimes I need to create a scalar layout, and I used `Layout("")`, but it returned an undefined layout. From the doc it should return a scalar layout right? I checked the constructor, and it seems in this if statement: https://github.com/apache/tvm/blob/6a274af9cb4e7baf6d9dd06f0fb38bea5ac3154a/src/tir/ir/data_layout.cc#L99, it directly returns without setting the `data_` field as in normal path: https://github.com/apache/tvm/blob/6a274af9cb4e7baf6d9dd06f0fb38bea5ac3154a/src/tir/ir/data_layout.cc#L144. I didn't fix this issue but work it around somehow, but it does seem like a bug to me :-). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
