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 2022/06/14 12:15:35 UTC
[GitHub] [tvm] PhilippvK opened a new pull request, #11702: [CMSIS-NN] Fix typo in EmitPool2D
PhilippvK opened a new pull request, #11702:
URL: https://github.com/apache/tvm/pull/11702
This bug, caused that no `context_buffer`s are generated for AvgPool2D function calls breaking the following configuration:
- `flags.dsp=1`
- `flags.mvei=0`
--
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 #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1171078712
Can we not check with cortex-m55+nomve ? If so it just need to modify the test runner (maybe a new one) to cover this case.
Here :
https://github.com/apache/tvm/blob/e7851ed763cd9e7e64c1e298908297d3f4ba93c7/python/tvm/micro/testing/aot_test_utils.py#L36-L47
(Happy to take as a follow up)
--
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 #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
manupa-arm merged PR #11702:
URL: https://github.com/apache/tvm/pull/11702
--
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] PhilippvK commented on pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
PhilippvK commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1155129833
@ashutosh-arm AFAIK there currently exist no test for the `RelayToTIRVisitor` itself. The routine `AvgPoolBufferSize` is correctly implemented but never invoked due to a typo in the if-statement in `relay_to_tir.cc`.
If we want to catch that sort of bug in the future, we need to write Tests for the individual `Emit*` functions.
A test would probably have failed if `cortex-m33`(DSP only) instead of `cortex-m55` (MVEI+DSP) would have been used in integrations tests with CMSIS-NN (Assuming that an `avg_pool2d` is part of a used model). However I am not sure in which degree cmsisnn is currently involved in integration tests.
--
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 pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1155114537
Thanks @PhilippvK for the fix.
Could you please check why the existing test does not catch this? https://github.com/apache/tvm/blob/27b0aad5a55254815a076dbcacb53e9725019f9d/src/relay/backend/contrib/cmsisnn/buffer_size.cc#L74
If you case is different, then please add a new unit test.
--
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] PhilippvK commented on pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
PhilippvK commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1155106341
@Mousius @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 pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1155235375
@PhilippvK You are correct there are no direct tests for ```RelayToTIRVisitor```.
I get it why the buffer checks pass in this case. Thanks!
Since we don't test execution for anything other than Cortex-M55 at the moment, we should probably use [this](https://github.com/apache/tvm/blob/27b0aad5a55254815a076dbcacb53e9725019f9d/python/tvm/micro/testing/aot_test_utils.py#L49) API to pass on the correct cpu arch by preparing PassConfig first :thinking:
--
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] areusch commented on pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
areusch commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1169102501
@manupa-arm should we merge this and file a follow-on GH issue to investigate?
--
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 #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1188997606
@ashutosh-arm, can you approve it explicitly ?
(as a token of confirmation that this change will be tested in the follow up you mentioned)
--
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] PhilippvK commented on pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
PhilippvK commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1171016290
@ashutosh-arm
> Since we don't test execution for anything other than Cortex-M55 at the moment, we should probably use [this](https://github.com/apache/tvm/blob/27b0aad5a55254815a076dbcacb53e9725019f9d/python/tvm/micro/testing/aot_test_utils.py#L49) API to pass on the correct cpu arch by preparing PassConfig first
While we could run the test for `cortex-m55` with MVEI+DSP support, `cortex-m33` with DSP and let's say `cortex-m0` without special instructios using the `mcpu` pass config we also need adapt the Makefiles which are currently hardcoded for the `cortex-m55`.
@areusch As the fixed issue breaks CMSISNN BYOC support for models with pooling layers on DSP-only devices I would appreciate getting this merged to `main`. If anything is missing please let me know.
--
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 pull request #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1188920006
I think this PR can be merged now that we have upcoming support for passing +nomve flag around from test runner: https://github.com/apache/tvm/pull/12132
--
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 #11702: [CMSIS-NN] Fix typo in EmitPool2D
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on PR #11702:
URL: https://github.com/apache/tvm/pull/11702#issuecomment-1189001319
Thanks @PhilippvK @ashutosh-arm @areusch !
--
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