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/05/05 16:50:23 UTC

[GitHub] [tvm] grant-arm opened a new pull request, #11221: [CMSIS-NN] Fix memory alignment bug in CMSIS-NN demo

grant-arm opened a new pull request, #11221:
URL: https://github.com/apache/tvm/pull/11221

   There is a latent bug in the CMSIS-NN demo app where the input and output tensors generated by the `create_image.py` script are not 16-byte aligned in memory. Although this does not cause an issue in the demo using the current `person_detect` model, if a different model is substituted with a larger output tensor, it causes the FVP to hang in certain cases.
   
   This PR updates `create_image.py` to correct the issue.
   
   @Mousius @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


[GitHub] [tvm] ashutosh-arm commented on pull request #11221: [CMSIS-NN] Fix memory alignment bug in CMSIS-NN demo

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11221:
URL: https://github.com/apache/tvm/pull/11221#issuecomment-1119412360

   > > @grant-arm Thanks for the PR. Just for my understanding, why do we need to align .rodata to 16 bytes? I have seen this for constants in the code generated C files as well. Also, does it hold good for all variations of Arm® Cortex®-M processors?
   > 
   > Hi @ashutosh-arm, the reason is because we default the workspace allocation alignment to 16 bytes in a few places, for example:
   > 
   > https://github.com/apache/tvm/blob/eae836cdf66f54f1e81e78e48bfa051431e8556f/src/relay/backend/aot_executor_codegen.cc#L831-L832
   > 
   > Which means we have to ensure we start on a 16 byte boundary to avoid overflows when we round up the allocations in memory planning. I believe we chose this because it's the max alignment required across all devices (including microNPUs), we could actually detect this and align as per the architecture at some point but the savings are minimal.
   
   Thanks for the detailed explanation @Mousius. 


-- 
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 merged pull request #11221: [CMSIS-NN] Fix memory alignment bug in CMSIS-NN demo

Posted by GitBox <gi...@apache.org>.
Mousius merged PR #11221:
URL: https://github.com/apache/tvm/pull/11221


-- 
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 #11221: [CMSIS-NN] Fix memory alignment bug in CMSIS-NN demo

Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11221:
URL: https://github.com/apache/tvm/pull/11221#issuecomment-1119399310

   @grant-arm Thanks for the PR. Just for my understanding, why do we need to align .rodata to 16 bytes? I have seen this for constants in the code generated C files as well. Also, does it hold good for all variations of Arm® Cortex®-M processors?


-- 
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 #11221: [CMSIS-NN] Fix memory alignment bug in CMSIS-NN demo

Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11221:
URL: https://github.com/apache/tvm/pull/11221#issuecomment-1119408205

   > @grant-arm Thanks for the PR. Just for my understanding, why do we need to align .rodata to 16 bytes? I have seen this for constants in the code generated C files as well. Also, does it hold good for all variations of Arm® Cortex®-M processors?
   
   Hi @ashutosh-arm, the reason is because we default the workspace allocation alignment to 16 bytes in a few places, for example:
   https://github.com/apache/tvm/blob/eae836cdf66f54f1e81e78e48bfa051431e8556f/src/relay/backend/aot_executor_codegen.cc#L831-L832
   
   Which means we have to ensure we start on a 16 byte boundary to avoid overflows when we round up the allocations in memory planning. I believe we chose this because it's the max alignment required across all devices (including microNPUs), we could actually detect this and align as per the architecture at some point but the savings are minimal.


-- 
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