[GitHub] [tvm] gromero edited a comment on pull request #9068: [microTVM][Zephyr] Add MIMXRT1050 board support

[GitBox <gi...@apache.org>]

gromero edited a comment on pull request #9068:
URL: https://github.com/apache/tvm/pull/9068#issuecomment-927379582


   @mehrdadh Hi. I took a second look at this change and tested it on a RT 1050 EVK board. It looks good. I just have a few additional comments besides Andrew's comment.
   
   I'd like to get rid of the following annoying warning that happens afaics only currently on that NXP board:
   
   ```
   [  7%] Building C object CMakeFiles/common.dir/crt/src/runtime/crt/common/crt_runtime_api.c.obj
   /tmp/x16/crt/src/runtime/crt/common/crt_runtime_api.c:108:13: warning: 'IsContiguous' defined but not used [-Wunused-function]
     108 | static bool IsContiguous(const DLTensor* arr) {
         |    
   ```
   
   This is caused because NXP code in Zephyr seems to honor more the `-DNDEBUG` and since `isContiguous()` used inside an `assert()` it ends up being unused because the assert will be removed if `-DNDEBUG` is set  (as it happens on microTVM cmake/make files).
   
   I think it's possible to avoid it:
   
   1. by wrapping  `isContiguous`  inside a `#ifndef NDEBUG ...`
   2.  by using the C attribute  `unused` attribute for `isContiguous`, like:
   `__attribute__ ((unused)) static bool IsContiguous(const DLTensor* arr) { ...`
   3. by using C++ attribute `[[maybe_unused]]`, like:
   ```[[maybe_unused]]
   static bool IsContiguous(const DLTensor* arr) { ...
   ```
   
   The one I prefer is 3. and I think it's ok because it's present in GCC 7.5 packaged by Ubuntu 18.04 (our reference environment, used for the RVM, right?).  @areusch wdyt?
   
   Speaking of RVM, should you add the new board to RVM Zephyr's `test-config.json` file in this patch?
   
   You'll need to tweak it a bit to add the board property to `boards.json` now that https://github.com/apache/tvm/issues/9049 is merged :)
   
   Finally, that board also fails `test_autotune_conv2d` so I added it to issue https://github.com/apache/tvm/issues/9049
   
   Cheers.


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