Mousius commented on PR #13606: URL: https://github.com/apache/tvm/pull/13606#issuecomment-1351632169
:wave: Hello @masahi / @csullivan / @echuraev / @junrushao, Whilst I appreciate this is useful with the use-case demonstrated, I believe it might've needed a bit more time for others to formulate a review as it was up for less than 24 hours before merge? One of my concerns is that this seems to be creating new public interfaces to the internals of the TECompiler which in the past we've attempted to remove: * https://discuss.tvm.apache.org/t/rfc-relay-tecompiler-rewrite-existing-compile-engine-to-match-updated-compiler-flow/9233 * https://github.com/apache/tvm/pull/8775 * https://github.com/apache/tvm/pull/9282 This change means we have to maintain an interface which generates a single `tir::PrimFunc` from a `relay::Function` where-as it could potentially lead to multiple `tir::PrimFunc` with a single `IRModule` as a result. The next line down: ``` IRModule tir_mod = PrimFuncToIRModule(f.value()); ``` Implies we wanted it in `IRModule` form anyway, so creating an `IRModule` with a `Function` and running `LowerTE(mod)` seems the best result here? -- 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]
