vinx13 edited a comment on pull request #42: URL: https://github.com/apache/tvm-rfcs/pull/42#issuecomment-957860936
Thank you eric for bringing up the discussion points. The primary reason for this discussion is because of the underlying tradeoffs: - 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, many passes would also need the read/write information, having an explicit BufferAccess can results in implementations 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. From my personal pov, considering all the factors, the clarity of read/write relation(in T1) and simplicity of AST outweights the benefits in T0. I agree that IR change is certainly important even if it is just a developer win, as long as it is strictly better than the old design. However, because of the impact, we might need some more deliberations on the tradeoffs. This review process already helps us to uncover some of the hidden problems(such as the initial leaking issue into values by using a BufferPointer, which can be addressed by having BufferAccess as an object). -- 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]
