Skip to content
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

[CMSIS-NN] enable USMP with CMSIS-NN #10224

Merged
merged 3 commits into from
Mar 2, 2022
Merged

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Feb 11, 2022

This commit mainly enables the USMP with CMSIS-NN codegen.
In order to do that, CMSIS-NN functions needed to contain BufferMaps. This commit adds the necessary BufferMaps as well.
All the tests are modified to run with USMP while the networks tests run with and without USMP.

@manupak
Copy link
Contributor Author

manupak commented Feb 11, 2022

This is blocked on #10022 and changes belonging to this PR is only in the commit : 57851dc

cc : @ashutosh-arm @Mousius

@manupak manupak force-pushed the cmsisnn_usmp branch 3 times, most recently from a12bea9 to dda2bf2 Compare February 14, 2022 16:01
@manupak manupak mentioned this pull request Feb 17, 2022
@manupak manupak force-pushed the cmsisnn_usmp branch 2 times, most recently from 061dfc4 to 130f22d Compare February 23, 2022 15:31
@manupak manupak marked this pull request as ready for review February 23, 2022 15:31
@manupak
Copy link
Contributor Author

manupak commented Feb 23, 2022

This is ready for review. @ashutosh-arm @Mousius

@manupak
Copy link
Contributor Author

manupak commented Feb 25, 2022

Friendly ping for reviews! @ekalda @lhutton1 @ashutosh-arm @leandron

Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment. Will defer to others for approval

@@ -27,6 +27,7 @@
from tests.python.relay.aot.aot_test_utils import (
AOTTestModel,
AOT_CORSTONE300_RUNNER,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: unused import? (elsewhere as well)

Copy link
Contributor

@ashutosh-arm ashutosh-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @manupa-arm . Overall looks good to me :) Few minor comments.

src/relay/backend/contrib/cmsisnn/relay_to_tir.cc Outdated Show resolved Hide resolved
src/relay/backend/contrib/cmsisnn/relay_to_tir.cc Outdated Show resolved Hide resolved
src/relay/backend/contrib/cmsisnn/relay_to_tir.cc Outdated Show resolved Hide resolved
This commit mainly enables the USMP
with CMSIS-NN codegen.

In order to do that, CMSIS-NN functions needed
to contain BufferMaps. This commit adds the necessary
BufferMaps as well.

All the tests are modified to run with USMP
while the networks tests run with and without
USMP.

Change-Id: I18c7958addfff90b8243e9a6c70b7411158462fa
"NULL" should be supplied as an extern arg if
the context buffer is not allocated.

Change-Id: I58866df69a27ce976ac45a57e2f261094a48e48e
* renaming buffer_var to be context_buffer_var
* removing trailing underscore from local variable
  : buffer_creator

Change-Id: I874164aa4944087c0e13cef80ead04c0de2be149
@manupak
Copy link
Contributor Author

manupak commented Mar 1, 2022

@ashutosh-arm, I've addressed the comments, PTAL when you have some time!

@ashutosh-arm
Copy link
Contributor

Thanks @manupa-arm. I suppose it was already in a good state to be merged :)

@manupak
Copy link
Contributor Author

manupak commented Mar 2, 2022

cc : @Mousius

I think I have missed @lhutton1's comment (Sorry!) which I will address in a follow up.

@lhutton1
Copy link
Contributor

lhutton1 commented Mar 2, 2022

No worries, nothing major :)

Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reviews @ashutosh-arm / @lhutton1 - makes sense to me 😸

@Mousius Mousius merged commit 4913453 into apache:main Mar 2, 2022
@Mousius
Copy link
Member

Mousius commented Mar 2, 2022

Thanks @manupa-arm!

ziqiangxu8457 pushed a commit to ziqiangxu8457/tvm that referenced this pull request Mar 6, 2022
This commit mainly enables the USMP
with CMSIS-NN codegen.

In order to do that, CMSIS-NN functions needed
to contain BufferMaps. This commit adds the necessary
BufferMaps as well.

All the tests are modified to run with USMP
while the networks tests run with and without
USMP.
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
This commit mainly enables the USMP
with CMSIS-NN codegen.

In order to do that, CMSIS-NN functions needed
to contain BufferMaps. This commit adds the necessary
BufferMaps as well.

All the tests are modified to run with USMP
while the networks tests run with and without
USMP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants