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


   > BufferPointer is somehow like the combination of Buffer and indices. It's 
good if we can have it, but my question is if it's necessary? As we are 
changing the core data structure, which is a breaking change, the cost is 
greater than the benefits in my opinion.
   
   Thank you, and that was the main concern that I had.  This change would make 
the areas that I've been working on lately be much cleaner, but outside of that 
.
   
   The breakage was minimized by keeping the existing 
`BufferLoad`/`BufferStore` constructors, while adding an overload that accepts 
`BufferPointer`, and the same could be added on the python side.  That would 
improve the source backwards compatibility, though it would still break binary 
backwards compatibility.
   
   > BTW, Could you please show some more design details? such as the data 
structure and the AST with BufferPointer? So that more people can join the 
discussion.
   
   Certainly.  I won't be able to add them here until Monday, but I have a 
sample implementation at [PR#9391](https://github.com/apache/tvm/pull/9391).
   
   > If we don't have corresponding language construct to independently define 
a pointer and use it, ... , then BufferPointer is merely an IR data structure 
that attaches to BufferLoad/BufferStore, which benefits the developer but 
doesn't bring additional expressiveness for the user.
   
   The current implementation does expose it, though primarily for testing 
purposes.  At the moment, none of the code-generators handle it explicitly, so 
the only current use is with BufferLoad/BufferStore.  It might be able to 
replace the builtin `tvm_access_ptr`, though the ease for developers was my 
main thought on it.
   
   > If we introduce such definition statements of pointer variables, then a 
latent problem is alias, though I can't immediately mock up an example to 
clearly show which effects this will bring to the IR transformation analysis.
   
   Agreed.  I kept to having the same buffer/indices as 
`BufferLoad`/`BufferStore` had, so that at least it wouldn't increase the risk. 
 I wouldn't want to have pointer arithmetic allowed, since that would make 
analysis to avoid aliasing or out-of-bounds access be harder.


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


Reply via email to