-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[microTVM] Update support for ARMv7m intrinsic #8990
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
[microTVM] Update support for ARMv7m intrinsic #8990
Conversation
sergio-grovety
commented
Sep 12, 2021
- Improved implementaion of gemm function for conv2d
- Removed %4 restriction for channels
- Added test case to verify SMLAD intrinsic speed acceleration
- Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com>
mehrdadh
left a comment
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.
Thanks for this great contribution. I did an initial review. I will try to run this on hardware and get back with more details.
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.
thanks @sergey-grovety for pushing this up! a couple questions
cc @manupa-arm @u99127 is anyone available to look at this from the ARM team?
- Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com>
…-grovety/tvm into update-arm-simd-intrinsic
|
for the test image maybe you could reuse this image: https://github.com/apache/tvm/blob/main/tests/micro/testdata/mnist/digit-2.jpg |
Yes, sure. |
Fixed micro model library test. Checking size reduced to 16 bytes from 2466816.
Removed changes.
mehrdadh
left a comment
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.
Thanks for this change. LGTM!
Just added a clarification question.
areusch
left a comment
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.
@sergey-grovety not sure if we should land #9092 first and refactor test logic there? otherwise aside from the two comments i think we can merge and move forwards.
| pytest tests/micro/zephyr/test_zephyr_aot.py --zephyr-board=${board} | ||
| fi | ||
|
|
||
| pytest tests/micro/zephyr/test_zephyr_armv7m.py --zephyr-board=${board} |
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.
could you move this to tests/scripts/task_python_microtvm.sh? This is just the base-box pre-release test, where we just want to do a smoke test to make sure that Zephyr is working with attached hardware.
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.
It seems moving to tests/scripts/task_python_microtvm.sh make no sense. The only supported platform there is qemu_x86 for now. We don't run our test with it. And mps2_an521 disabled.
BTW, all tests in tests/micro/zephyr must be called, isn't it?
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.
we discussed offline and have decided to proceed with local testing in this PR. in a follow-on, we will add support for Corstone 300 tests.
Yes. Need to clean up from this. |
|
|
||
| if board not in [ | ||
| "mps2_an521", | ||
| "stm32f746xx_disco", |
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.
this board name has changed recently to stm32f746g_disco.
https://github.com/apache/tvm/pull/8998/files
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.
@sergey-grovety given the CI is green and upcoming work on the test strategy feel free to address this in a follow-on.
|
thanks @sergey-grovety and team! |
* [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * Implemented discussed changes. * Removed unnecessary test files. * Formatting fixed. * Formatting fixed2. * Formatting fixed3. * Formatting fixed4. * Formatting fixed5. * Fixed test time result checking. * Check rebuild. * Formatting fixed. * Formatting fixed. * [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * Implemented discussed changes. * Removed unnecessary test files. * Formatting fixed. * Formatting fixed2. * Formatting fixed3. * Formatting fixed4. * Formatting fixed5. * Fixed test time result checking. * Check rebuild. * Formatting fixed. * Issue 8717 Add schedule for depthwise_conv2d_nhwc * Resolve merge conflict. * Resolve merge conflicts. * Fixed formatting. * From Issue 8717// Fixed micro model library test. Checking size reduced to 16 bytes from 2466816. * From Issue 8717. Removed changes. * From Issue 8717. Fixed typo. * Fixed import. * Fixed import and method call. * Added QEMU testing comment. * Fixed ZEPHYR_BOARD usage. * Fixed tests. Removed issue 8717 changes. * Formatting fixed. * Removed test call from base_box_test.sh
* [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * Implemented discussed changes. * Removed unnecessary test files. * Formatting fixed. * Formatting fixed2. * Formatting fixed3. * Formatting fixed4. * Formatting fixed5. * Fixed test time result checking. * Check rebuild. * Formatting fixed. * Formatting fixed. * [microTVM] Update support for ARMv7m intrinsic - Improved implementaion of gemm function for conv2d - Removed %4 restriction for channels - Added test case to verify SMLAD intrinsic speed acceleration Signed-off-by: Sergey Smirnov <Sergey@grovety.com> * Implemented discussed changes. * Removed unnecessary test files. * Formatting fixed. * Formatting fixed2. * Formatting fixed3. * Formatting fixed4. * Formatting fixed5. * Fixed test time result checking. * Check rebuild. * Formatting fixed. * Issue 8717 Add schedule for depthwise_conv2d_nhwc * Resolve merge conflict. * Resolve merge conflicts. * Fixed formatting. * From Issue 8717// Fixed micro model library test. Checking size reduced to 16 bytes from 2466816. * From Issue 8717. Removed changes. * From Issue 8717. Fixed typo. * Fixed import. * Fixed import and method call. * Added QEMU testing comment. * Fixed ZEPHYR_BOARD usage. * Fixed tests. Removed issue 8717 changes. * Formatting fixed. * Removed test call from base_box_test.sh