manupa-arm commented on pull request #8795: URL: https://github.com/apache/tvm/pull/8795#issuecomment-909087831
@jroesch @mbs-octoml , Sorry have been busy getting all the PRs up and green (which they are now :) . Yes, we were off over long-weekend here in the UK. Given its python dependency of Vela, we find keeping the integration and passes in python for the NPU lowering is beneficial, especially in debugging. However, we have moved that involves relay traversal implementations (bar the pattern rewrites -- because the python hook is just an interface and I believe the rewriting is actually happening in C++) to C++ while maintaining a python interface to cater the integration. Performance was the consideration to move the pass impl to C++. If we find some passes are slow, we'll put effort to move the impl to C++. @mbs-octoml For 1.) > Should the op defs go in python/tvm/relay/op/contrib/ethosu.py (where you currently have just the MergeComposite pattern defs), and the te defs should go in python/tvm/topi/ethosu/conv2d.py? We've gotten this far without a contrib under python/tvm/relay/backend so trying to follow existing convention as much as possible even with the custom scheduling would be nice. We do have a src/relay/backend/contrib/ -- this is just mirroring that folder in python, therefore I think it is less confusing to create it there. for op definitions, @mbaret WDYT ? -- Having everything closer to the Ethos-U codegen seems easier to navigate for me. For 2.) Yes, we agree to add the type annotations for relay types. For 3.) @mbaret , we need that by P3 right? -- 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]
