[GitHub] [tvm] Mousius commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message

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

Mousius commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1057230406


##########
apps/microtvm/cmsisnn/include/crt_config.h:
##########
@@ -23,4 +23,37 @@
 /*! Log level of the CRT runtime */
 #define TVM_CRT_LOG_LEVEL TVM_CRT_LOG_LEVEL_DEBUG
 
+/*! Support low-level debugging in MISRA-C runtime */
+#define TVM_CRT_DEBUG 0
+
+/*! Maximum supported dimension in NDArray */
+#define TVM_CRT_MAX_NDIM 6
+
+/*! Maximum supported arguments in generated functions */
+#define TVM_CRT_MAX_ARGS 10
+
+/*! Size of the global function registry, in bytes. */
+#define TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES 512
+
+/*! Maximum number of registered modules. */
+#define TVM_CRT_MAX_REGISTERED_MODULES 2
+
+/*! Maximum packet size, in bytes, including the length header. */
+#define TVM_CRT_MAX_PACKET_SIZE_BYTES 2048
+
+/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
+#define TVM_CRT_MAX_STRLEN_DLTYPE 10
+
+/*! Maximum supported string length in function names */
+#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120
+
+/*! Maximum supported string length in parameter names */
+#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80
+
+/*! \brief Maximum length of a PackedFunc function name. */
+#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
+
+/*! \brief Enable checks to enforce the stack allocator with a FIFO ordering. Off by default */
+// #define TVM_CRT_STACK_ALLOCATOR_ENABLE_FIFO_CHECK

Review Comment:
   Can we check whether these values are actually used rather than copying around all of them?



##########
apps/microtvm/cmsisnn/Makefile:
##########
@@ -69,6 +69,18 @@ $(BUILD_DIR)/crt_backend_api.o: $(STANDALONE_CRT_PATH)/src/runtime/crt/common/cr
 	$(QUIET)mkdir -p $(@D)
 	$(QUIET)$(CC) -c $(PKG_CFLAGS) -o $@  $^
 
+$(BUILD_DIR)/crt_runtime_api.o: $(STANDALONE_CRT_PATH)/src/runtime/crt/common/crt_runtime_api.c
+	$(QUIET)mkdir -p $(@D)
+	$(QUIET)$(CC) -c $(PKG_CFLAGS) -o $@  $^
+
+$(BUILD_DIR)/func_registry.o: $(STANDALONE_CRT_PATH)/src/runtime/crt/common/func_registry.c
+	$(QUIET)mkdir -p $(@D)
+	$(QUIET)$(CC) -c $(PKG_CFLAGS) -o $@  $^
+
+$(BUILD_DIR)/ndarray.o: $(STANDALONE_CRT_PATH)/src/runtime/crt/common/ndarray.c
+	$(QUIET)mkdir -p $(@D)
+	$(QUIET)$(CC) -c $(PKG_CFLAGS) -o $@  $^
+

Review Comment:
   I'm assuming these are dependencies for `crt_runtime_api.c`? We should raise an issue to remove that dependency if we can?
   
   @areusch had some thoughts awhile back on this https://discuss.tvm.apache.org/t/pre-rfc-api-change-formalizing-c-backend-api/10380



##########
src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc:
##########
@@ -478,10 +466,28 @@ class CodeGenCMSISNN : public codegen::CodeGenCHost {
     stream << "&" << input_dim << ", " << input_data << ", ";
     stream << "&" << filter_dim << ", ";
     stream << "&" << output_dim << ", " << output_data << ");\n";
+    EmitErrorCheck();
+  }
+
+  void EmitErrorCheck() {
+    auto emit_error = [&](std::string error) {
+      stream << "TVMAPISetLastError(\"" << error << "\");";
+    };
+
     PrintIndent();
-    stream << "if (status != ARM_CMSIS_NN_SUCCESS) {\n";
+    stream << "switch (status) {\n";
+    PrintIndent();
+    PrintIndent();
+    stream << "case ARM_CMSIS_NN_SUCCESS: break;\n";
+    PrintIndent();
+    PrintIndent();

Review Comment:
   Potential fun exercise for the future, add a parameter to `PrintIndent` for the number of indents 😸 



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