[GitHub] [tvm-rfcs] vinx13 commented on pull request #42: [TIR] Adding BufferPointer node, used by BufferLoad and BufferStore

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

vinx13 commented on pull request #42:
URL: https://github.com/apache/tvm-rfcs/pull/42#issuecomment-957860936


   To be clear, we should discuss the tradeoff here.
   
   T0: On one hand, for analysis/rewriting that do not need to differentiate read/write relation, having a common BufferAccess class is helpful. 
   T1: On the other hand, for analysis/rewriting that do need read/write and other relation, having an explicit BufferAccess can results in methods that forget to consider r/w relations(because of the common BufferAccess class), or other relations that follow.
   
   So from a developer experience pov, it is hard to argue which way is clearly better. T0 would call for the introduction of BufferAccess, while T1 would favor the original AST where read/write are explicit.  Given most goals of T0 can be implemented via a common util function. Considering all the factors, the clarity of read/write relation(in T1) and simplicity of AST outweights the benefits in T0.
   
   IR change is certainly important even if it is just a developer win, as long as it is strictly better than the old design,  but any IR change would require some deliberation since there usually will be tradeoffs like T0 and T1(also the more serious problem bought by BufferPointer, which can be addressed if BufferAcccess is just an Object) as a result can take longer time.


-- 
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-unsubscribe@tvm.apache.org

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