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 tell 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, the
gain of T0 may not be as large. 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). I also agree that it is better
to separate the TIR change from RFC0039, we just need more discussions about
the design here.
I am looking forward to build better designs together :)
--
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]
