[GitHub] [tvm] rafzi opened a new pull request #8134: memory planning: add offset to planning output and respect it in graph executor

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

rafzi opened a new pull request #8134:
URL: https://github.com/apache/tvm/pull/8134


   This adds the possibility to specify an offset for the results of memory planning, such that buffers can be placed at other positions than the base address. This way we can also partially overlap buffers to enable the most optimal buffer placements.
   
   More details in this discussion: https://discuss.tvm.apache.org/t/discussion-alignment-memory-planning/9730


-- 
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:
users@infra.apache.org

[GitHub] [tvm] areusch commented on pull request #8134: memory planning: add offset to planning output and respect it in graph executor

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

areusch commented on pull request #8134:
URL: https://github.com/apache/tvm/pull/8134#issuecomment-860312504


   @rafzi apologies for the delay in reviewing this one. i'm not sure there is broad alignment yet on the way we intend to do full-graph memory planning in TVM. and, even when we do come to agreement on a model for memory (which I think may look similar to the one you're working towards here), we still need to implement support for it in both Graph and AOT executors. Also, the Graph executor is invoking TIR PrimFunc, so it's likely something similar to this PR will be useful. My thinking is that what you have here is fairly close and we'll just need to rename fields or add additional e.g. `pool_id` to give more context to the offset.
   
   So I'm not convinced we should drop this PR; however, before proceeding, I'd like to get everyone aligned around a single memory planning proposal. There are a couple of theoretically orthogonal pieces of such a proposal as well: a) the interface between the TVM graph and the memory planner; b) the algorithm(s) used in planning; c) the interface between TVM and the executors. At present there are two suggestions for (a) a TIR-level interface and a Relay-level planner. I think the TIR-based planner offers more flexibility but the Relay one is easier to implement to (e.g. it's nearly complete in the tree today).
   
   Would you be interested in reviewing the TIR-level interface proposed in the [USMP](https://discuss.tvm.apache.org/t/rfc-unified-static-memory-planning/10099) RFC? It would be great to get your thoughts whether it's possible to implement the algorithms you've proposed using that interface as well.
   
   Given there is some interest from the community in doing whole-program TIR optimization, plus the AOT top-level function is in TIR, it may be slightly more impactful to adopt that interface. However, I'd like to understand whether that precludes including the algorithms you've proposed [here](https://discuss.tvm.apache.org/t/discussion-alignment-memory-planning/9730). Finally, this PR could serve as a basis to implement the Graph executor changes required to support (c).
   
   Let me know your thoughts!


-- 
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:
users@infra.apache.org

[GitHub] [tvm] jroesch closed pull request #8134: memory planning: add offset to planning output and respect it in graph executor

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

jroesch closed pull request #8134:
URL: https://github.com/apache/tvm/pull/8134


   


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

[GitHub] [tvm] rafzi commented on pull request #8134: memory planning: add offset to planning output and respect it in graph executor

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

rafzi commented on pull request #8134:
URL: https://github.com/apache/tvm/pull/8134#issuecomment-860110757


   It seems like the long term plans of TVM are conflicting with this approach, in that the memory planning should happen in TIR.
   
   Is this something that is useful to TVM right now? Should I continue work on this or drop it in favor of the upcoming approach?


-- 
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:
users@infra.apache.org

[GitHub] [tvm] jroesch commented on pull request #8134: memory planning: add offset to planning output and respect it in graph executor

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

jroesch commented on pull request #8134:
URL: https://github.com/apache/tvm/pull/8134#issuecomment-1016833205


   This PR appears to be out of date, please feel free to reopen it if this is not the case.
   
   As part of the new year we are attempting to triage the project's open pull requests to ensure that code which
   is ready for review and/or merging receives adequate attention.
   
   Thanks again for your contribution, and feel free to reach out to discuss these changes.


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