[GitHub] [tvm] masahi commented on pull request #7303: [TOPI] Make cumsum IR reusable, add thrust scan

[GitBox <gi...@apache.org>]

masahi commented on pull request #7303:
URL: https://github.com/apache/tvm/pull/7303#issuecomment-763106769


   1. Right now, inclusive scan can be supported by `exclusive_scan(data) + data`. I think that is fine for now, given that our scan IR is far from stable and we don't want to maintain two IRs for the sake of removing the additional sum.
   
   2. Yes, we can definitely do that. But this PR is already not small and I want to keep the original IR as close as possible for this PR. There are other TODO items for scan (e.g. support other binary ops), so I hope we can address this problem in the future as well.
   
   A related discussion point: Do you expect scan performance on non-innermost axis to be slower than the innermost case? If that's the case (which I believe yes), I think supporting non innermost scan and other ranks by 
   ```
   reshape + transpose + innermost scan + reshape and transpose back 
   ```
   is a good solution. It is definitely preferred in terms of implementation simplicity, allowing scan implementation to focus on 1 or 2D + innermost axis.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org