Lunderberg commented on PR #13938: URL: https://github.com/apache/tvm/pull/13938#issuecomment-1431813726
I think this case should throw an error instead at this point, rather than a workaround at this location. Currently, the behavior of `Inverse` throws an error whenever padding would be introduced, whereas `MapRanges` and `MapShape` allow padding. If we make this change, the difference in padding semantics between `MapIndices` and `MapShape` would be even larger. For the transformation `n => [n//8, n%8]` applied to a buffer of shape `[7]`, `MapIndices` would attempt to reshape this into `[1,8]` and throw an error, while `MapShape` would attempt to reshape into `[1,7]` and return a success. I don't think the divergent semantics is appropriate here. Instead, I think we should throw the same error that occurs in `IndexMap::Inverse`, and throw an exception in this case. That is, this case would be an error in the calling scope rather than a use case that `IndexMap` should handle, with padded maps handled through a separate function call. For the use case where the SIMD size is large relative to the buffer size, don't we need the padding anyways so that the SIMD instruction doesn't overrun the bounds? I'm wondering if we could find a better place to apply a fix to explicitly allow the IndexMap to introduce padding including in the allocated buffer in all locations where it is used. -- 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]
