You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/10/29 09:26:14 UTC
[GitHub] [tvm] Mousius opened a new pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Mousius opened a new pull request #9397:
URL: https://github.com/apache/tvm/pull/9397
This migrates CMSIS-NN to use Target Hooks instead of fully BYOC, which
means it will now go through any central passes the Driver API.
Found a few things whilst doing this:
* Forgot to mutate PrimFunc arguments in LowerTE which meant functions weren't getting lowered passed the first function in test_networks
* Target `cmsis-nn` needs to match external code generator `cmsis-nn` to connect the Target with the external code generator
* Partition Graph needed to sanitise compiler names to generate them properly in C
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#issuecomment-963371984
@ashutosh-arm could you take a look and explicitly approve if it's ok to move the name change to a follow up :smile_cat:
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#issuecomment-954591385
CC @ashutosh-arm @manupa-arm
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#discussion_r745508192
##########
File path: src/relay/backend/contrib/cmsisnn/relay_to_tir.cc
##########
@@ -33,29 +34,46 @@ namespace relay {
namespace contrib {
namespace cmsisnn {
-class RelayToTIRVisitor : public MixedModeVisitor {
+class RelayToTIRVisitor : public MixedModeMutator {
Review comment:
Sorry, I didn't mean it as a blocking comment. A follow up is fine, should've marked as nit.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a change in pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#discussion_r744917026
##########
File path: src/relay/backend/contrib/cmsisnn/relay_to_tir.cc
##########
@@ -33,29 +34,46 @@ namespace relay {
namespace contrib {
namespace cmsisnn {
-class RelayToTIRVisitor : public MixedModeVisitor {
+class RelayToTIRVisitor : public MixedModeMutator {
Review comment:
I'll move this to a follow up :smile_cat:
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on a change in pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#discussion_r740266306
##########
File path: src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc
##########
@@ -25,114 +25,50 @@
#include "../../../../runtime/file_utils.h"
#include "../../../../target/source/codegen_c.h"
+#include "../../../../target/source/codegen_c_host.h"
namespace tvm {
-namespace codegen {
-
using namespace tir;
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
-class CodeGenCMSISNN : public CodeGenC {
+class CodeGenCMSISNN : public codegen::CodeGenCHost {
Review comment:
In other `Target`s they get PrimFunc's which are essentially:
```
{ [Host Module] -> [Device Lib Module] } -> [Device]
```
So conceptually it makes sense to do:
```
{ [Host Module] } -> [Lib]
```
For CMSIS-NN and other library targets, as TVM is hosting the library - I think, would be interested in @manupa-arm's view 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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm commented on pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#issuecomment-964104980
Thanks @Mousius @ashutosh-arm !
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ashutosh-arm commented on a change in pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on a change in pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#discussion_r740108140
##########
File path: src/relay/backend/contrib/cmsisnn/relay_to_tir.cc
##########
@@ -33,29 +34,46 @@ namespace relay {
namespace contrib {
namespace cmsisnn {
-class RelayToTIRVisitor : public MixedModeVisitor {
+class RelayToTIRVisitor : public MixedModeMutator {
Review comment:
Should it be renamed RelayToTIRVisitor --> RelayToTIRMutator?
##########
File path: src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc
##########
@@ -25,114 +25,50 @@
#include "../../../../runtime/file_utils.h"
#include "../../../../target/source/codegen_c.h"
+#include "../../../../target/source/codegen_c_host.h"
namespace tvm {
-namespace codegen {
-
using namespace tir;
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
-class CodeGenCMSISNN : public CodeGenC {
+class CodeGenCMSISNN : public codegen::CodeGenCHost {
Review comment:
Its great that its working with CodeGenCHost now. Just looks tiny bit odd that we are generating target code by deriving from Host codegen. But I also want to say that I don't have any real problem if it works.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm commented on a change in pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on a change in pull request #9397:
URL: https://github.com/apache/tvm/pull/9397#discussion_r745528465
##########
File path: src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc
##########
@@ -25,114 +25,50 @@
#include "../../../../runtime/file_utils.h"
#include "../../../../target/source/codegen_c.h"
+#include "../../../../target/source/codegen_c_host.h"
namespace tvm {
-namespace codegen {
-
using namespace tir;
+namespace relay {
+namespace contrib {
+namespace cmsisnn {
-class CodeGenCMSISNN : public CodeGenC {
+class CodeGenCMSISNN : public codegen::CodeGenCHost {
Review comment:
This seems fine to me.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] manupa-arm merged pull request #9397: [CMSIS-NN] Convert CMSIS-NN to use Target Hooks
Posted by GitBox <gi...@apache.org>.
manupa-arm merged pull request #9397:
URL: https://github.com/apache/tvm/pull/9397
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org