You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "ashutosh-arm (via GitHub)" <gi...@apache.org> on 2023/01/25 10:40:56 UTC

[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #13815: [CMSIS-NN] Reduction in code size of AOT test runner binary

ashutosh-arm commented on code in PR #13815:
URL: https://github.com/apache/tvm/pull/13815#discussion_r1086472372


##########
tests/python/relay/aot/corstone300.mk:
##########
@@ -64,7 +64,7 @@ PKG_CFLAGS = ${PKG_COMPILE_OPTS} \
 CMAKE_FLAGS = -DCMAKE_TOOLCHAIN_FILE=${TVM_ROOT}/tests/python/contrib/test_ethosu/reference_system/arm-none-eabi-gcc.cmake \
 	-DCMAKE_SYSTEM_PROCESSOR=${MCPU}
 
-PKG_LDFLAGS = -lm -specs=nosys.specs -static -T ${AOT_TEST_ROOT}/corstone300.ld
+PKG_LDFLAGS = -lm -specs=nosys.specs -static -Wl,--gc-sections -T ${AOT_TEST_ROOT}/corstone300.ld

Review Comment:
   maybe add a comment why `gc-section` and `fdata-sections` were used for the build?



##########
python/tvm/topi/arm_cpu/mprofile/dsp/micro_kernel/multi_channel_convolve.py:
##########
@@ -179,20 +179,26 @@ def _dual_int16_channel_convolve_impl(_tensor_h, tensor_w, channels, kernel_h, k
         extern "C"
         #endif
         int32_t {_get_func_name("int16", tensor_w, channels, kernel_h, kernel_w, suffix)}(
-            uint32_t *out,
-            uint32_t *tensor,
-            uint32_t *kernel) {{
+            int32_t *out,
+            int16_t *tensor,
+            int16_t *kernel) {{
 
-          uint32_t sum_c0 = 0;
-          uint32_t sum_c1 = 0;
+          int32_t sum_c0 = 0;
+          int32_t sum_c1 = 0;
+
+          int32_t kernel_i32[{kernel_h} * {kernel_w}];
+          memcpy(kernel_i32, kernel, {kernel_h} * {kernel_w} * 4);

Review Comment:
   nit: can we please add `sizeof()` instead of using 4 directly?



##########
python/tvm/topi/arm_cpu/mprofile/dsp/micro_kernel/multi_channel_convolve.py:
##########
@@ -179,20 +179,26 @@ def _dual_int16_channel_convolve_impl(_tensor_h, tensor_w, channels, kernel_h, k
         extern "C"
         #endif
         int32_t {_get_func_name("int16", tensor_w, channels, kernel_h, kernel_w, suffix)}(
-            uint32_t *out,
-            uint32_t *tensor,
-            uint32_t *kernel) {{
+            int32_t *out,
+            int16_t *tensor,
+            int16_t *kernel) {{
 
-          uint32_t sum_c0 = 0;
-          uint32_t sum_c1 = 0;
+          int32_t sum_c0 = 0;
+          int32_t sum_c1 = 0;
+
+          int32_t kernel_i32[{kernel_h} * {kernel_w}];
+          memcpy(kernel_i32, kernel, {kernel_h} * {kernel_w} * 4);

Review Comment:
   Same applies to the code below where 2 and 4 are used.



##########
python/tvm/topi/arm_cpu/mprofile/dsp/micro_kernel/multi_channel_convolve.py:
##########
@@ -179,20 +179,26 @@ def _dual_int16_channel_convolve_impl(_tensor_h, tensor_w, channels, kernel_h, k
         extern "C"
         #endif
         int32_t {_get_func_name("int16", tensor_w, channels, kernel_h, kernel_w, suffix)}(
-            uint32_t *out,
-            uint32_t *tensor,
-            uint32_t *kernel) {{
+            int32_t *out,
+            int16_t *tensor,
+            int16_t *kernel) {{
 
-          uint32_t sum_c0 = 0;
-          uint32_t sum_c1 = 0;
+          int32_t sum_c0 = 0;
+          int32_t sum_c1 = 0;
+
+          int32_t kernel_i32[{kernel_h} * {kernel_w}];
+          memcpy(kernel_i32, kernel, {kernel_h} * {kernel_w} * 4);
+
+          int32_t tensor_length = {((kernel_w - 1) * (channels // 2) + (kernel_h - 1) * tensor_w * (channels // 2)) + 1};

Review Comment:
   Curious why are braces used in this piece of logic?



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