junrushao1994 edited a comment on pull request #8468: URL: https://github.com/apache/tvm/pull/8468#issuecomment-897040736
Hey @manupa-arm, thanks for the nice PR! As a main thing I am a little bit worried about, I would love to discuss specifically on the IR-related change, because in the worse case, adding/changing/removing an IR node would result in malfunctioning of many passes. Therefore, we tend to keep the IR stable if possible, and scrutinize any changes to the IR. For example, when @masahi was adding the while node (really awesome work https://github.com/apache/tvm/pull/7425), we discussed very carefully on the implication, and actually updated many passes that are affected by that particular change. As we are adopting the new RFC process, I would love to propose that if the RFC implies any IR modification, the modification should be written explicitly in a section (e.g. which fields are added), and be discussed really carefully on everything that could happen (e.g. which passes won't work). CC: @jroesch @vinx13 @areusch @comaniac On this particular case, we are seeing a set of IR changes, including this PR, the [`PrimFunc` PR](https://github.com/apache/tvm/pull/8452), the [`allocate_const` PR](https://github.com/apache/tvm/pull/8472). That will be super helpful if you guys could send a RFC discussing the rationale and potential implication on this set of changes to TIR, so that it could be more more maintainable in the foreseeable future. 🙏 Many thanks! -- 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]
