-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Use CMSIS-NN with TVM #15
Conversation
Minor nit - the title of the RFC should really read - [RFC] Use CMSIS-NN with TVM. @manupa-arm , @Mousius .. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few nits here and there. Generally looks very good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of other acronyms.
Change-Id: I3b0954f3fdb4d54b3e38a84de0ab649c1e79bca8
Change-Id: I6142c001175cdf41c58b5bb555a39e07c834254f
Change-Id: Id63d1866cd783f5e59b568f36c9177ee8715bc4d
Change-Id: Id54bae4bd2ca4bd9c3ab734e8cae966ebbe332b2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. One thing I'm a little bit worry about is this RFC depends on another two RFCs (target hook and Ethos-U integration). I would be better if this RFC can also discuss the impact and alternative plan without other 2 RFCs so that this RFC won't be blocked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashutosh-arm apologies for the delay! just a few points to clarify here.
Change-Id: I56a5f9bf319576d342a5bdc3771402262584e8c4
… with some terminologies Change-Id: I002be9cc67b72444ea27fe0a31769549fb6fd452
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashutosh-arm i did another round here to clarify your intentions. i think this should be the bulk of my questions on this RFC
Merging as Cody said LGTM in his last review |
This RFC details integration of CMSIS-NN API into TVM to support code generation for Arm(R) Cortex-M using the following library: https://github.com/ARM-software/CMSIS_5/tree/develop/CMSIS/NN
cc : @areusch @mbaret @tqchen @Mousius @manupa-arm