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: [email protected]
