giuseros commented on pull request #8096:
URL: https://github.com/apache/tvm/pull/8096#issuecomment-859519657


   Hi @areusch, 
   Thanks for your reply. I think what you are saying is can be summarized in:
   a) The StorageRewrite pass does not solve our problem because it can still 
touch the output buffers (which are parameters of the `tir PrimFunc`)
   b) The solution that we have put in place in `StorageRewrite` does not 
address the reference counting in an general way. 
   
   About a), I disagree. `StorageRewrite` only rewrites `tir.Allocate` nodes, 
and the output is not allocated in our main runner function. This is why if you 
run mobilenet quantized with the `StorageRewrite` pass, it works (and it 
doesn't work with Graph Memory Planning). So `StorageRewrite` is a perfectly 
good candidate to solve the issue we have in #8062 . The problem with 
`StorageRewrite `is that it doesn't handle `tvm_struct_set` correctly, and this 
brings us to point b)
   
   About b), I agree. The solution we are proposing is not general, but any 
general solution can be built on top of it. As you said, we are trying to solve 
a specific case, but when you tackle the general case, the code I wrote is 
still valid. Especially in a TIR based world (which is the aim of TensorIR, 
afaiu) I think we will want (at some point) to extend this solution instead of 
accepting  that `StorageRewrite` does not work with some TIR built-ins. 
   
   In other words, our solution is not tailored to AOT at all. It is a partial 
solution to a generic problem that (only) AOT is hitting. It cannot break 
anything in the future, because either the user is doing what AOT is doing, and 
it will work. For more edge cases, `StorageRewrite` won't work as it didn't use 
to work before. 
   
   Bear also in mind that until we have a static memory planner, without this 
solution AOT is not usable. If you want, we can try to write a test that 
stresses the problem we are trying to solve with the change in 
`StorageRewrite`. 
   
   What do you think?


-- 
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:
[email protected]


Reply via email to