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/12/19 11:37:51 UTC
[GitHub] [tvm] NicolaLancellotti opened a new pull request, #13643: [CMSIS-NN] Add a runtime error message
NicolaLancellotti opened a new pull request, #13643:
URL: https://github.com/apache/tvm/pull/13643
This pr generates the code to catch and save, through TVMAPISetLastError, a CMSIS-NN error.
In case of an error is raised, it is retrieved with TVMGetLastError in the AOT test runner and then it is printed.
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
Mousius commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1061728070
##########
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:
There are two separate issues here:
1. The C Runtime doesn't make it easy to use only the bits you want so you suddenly need array handling if you want error handling - this is a future problem if we want to tackle it
2. If we emit calls to these functions we're breaking anyone who currently compiles their model with CMSIS-NN - we need to have a way for users to specify that they don't want the debugging behaviour and then we don't emit the calls to the `TVMAPISetLastError`/`TVMAPIGetLastError` apis.
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1060750863
##########
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 am not sure I understood, can you elaborate?
In this pr, we save the error through `TVMAPISetLastError` and then we retrieve it through `TVMGetLastError`, so we need the definitions of these two functions.
In the current implementation the functions are defined in `crt_runtime_api.c`, the problem is that this file has other functions, which we do not use, that depend on functions defined in `func_registry.c` and `ndarray.c`.
Ashutosh proposed not to use the definitions in `crt_runtime_api.c`, but to just emit the definitions of these two functions in `aot.py`.
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
Mousius commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1060611546
##########
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:
If we don't emit the lines that set them, we shouldn't need the definitions at all?
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1072048375
##########
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've added an option `debug_last_error` to `CMSISNNCompilerConfigNode`.
The calls to `TVMAPISetLastError` and `TVMAPIGetLastError` are generated only if this option is true.
When the option is false we no longer need the dependencies: `crt_runtime_api.c`, `func_registry.c` and `ndarray.c`. We need them only when the option is true.
I've also added a test, that checks if a model compiles with both values of this option.
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1060438817
##########
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:
Yes, they are dependencies for `crt_runtime_api.c` which defines `TVMAPISetLastError` and `TVMGetLastError`.
Instead of depending on `crt_runtime_api.c` we can generate the definitions of these functions as @ashutosh-arm thought: https://github.com/apache/tvm/pull/13643#discussion_r1052140042, what do you think?
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by 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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "ashutosh-arm (via GitHub)" <gi...@apache.org>.
ashutosh-arm commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1118738360
##########
tests/python/contrib/test_cmsisnn/test_last_error.py:
##########
@@ -0,0 +1,164 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""CMSIS-NN integration tests: debug_last_error"""
+
+import re
+import numpy as np
+import pytest
+import tvm
+from tvm import relay
+from tvm.relay.op.contrib import cmsisnn
+
+from tvm.testing.aot import (
+ get_dtype_range,
+ generate_ref_data,
+ AOTTestModel,
+ compile_and_run,
+)
+from .utils import (
+ make_module,
+ get_same_padding,
+ make_qnn_relu,
+ assert_partitioned_function,
+ create_test_runner,
+)
+
+
+def make_model(
+ pool_op,
+ shape,
+ pool_size,
+ strides,
+ padding,
+ dtype,
+ scale,
+ zero_point,
+ relu_type,
+ layout,
+ input_op,
+):
+ """Create a Relay Function / network model"""
+ if input_op:
+ op = input_op
+ else:
+ op = relay.var("input", shape=shape, dtype=dtype)
+ pad_ = (0, 0, 0, 0)
+ if padding == "SAME":
+ dilation = (1, 1)
+ pad_ = get_same_padding((shape[1], shape[2]), pool_size, dilation, strides)
+ op = relay.nn.pad(
+ op,
+ pad_width=[(0, 0), (pad_[0], pad_[2]), (pad_[1], pad_[3]), (0, 0)],
+ pad_value=zero_point,
+ pad_mode="constant",
+ )
+ if pool_op.__name__ == relay.nn.avg_pool2d.__name__:
+ op = relay.cast(op, "int32")
+ op = pool_op(
+ op, pool_size=pool_size, strides=strides, padding=pad_, ceil_mode=True, layout=layout
+ )
+ if pool_op.__name__ == relay.nn.avg_pool2d.__name__:
+ op = relay.cast(op, dtype)
+ op = make_qnn_relu(op, relu_type, scale, zero_point, dtype)
+ return op
+
+
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("debug_last_error", [True, False])
+def test_last_error(debug_last_error):
+ """Tests debug_last_error"""
+ dtype = "int16"
+ in_shape = (1, 28, 28, 12)
+ pool_size = (3, 3)
+ strides = (2, 2)
+ padding = "SAME"
+ relu_type = "NONE"
+ pool_type = relay.nn.avg_pool2d
+ zero_point = -34
+ scale = 0.0256
+ compiler_cpu = "cortex-m55"
+ cpu_flags = "+nomve"
+ layout = "NHWC"
+ input_op = None
+
+ interface_api = "c"
+ use_unpacked_api = True
+
+ model = make_model(
+ pool_op=pool_type,
+ shape=in_shape,
+ pool_size=pool_size,
+ strides=strides,
+ padding=padding,
+ dtype=dtype,
+ scale=scale,
+ zero_point=zero_point,
+ relu_type=relu_type,
+ layout=layout,
+ input_op=input_op,
+ )
+ orig_mod = make_module(model)
+
+ cmsisnn_mod = cmsisnn.partition_for_cmsisnn(orig_mod)
+
+ # validate pattern matching
+ assert_partitioned_function(orig_mod, cmsisnn_mod)
+
+ # validate the output
+ in_min, in_max = get_dtype_range(dtype)
+ inputs = {
+ "input": np.random.randint(in_min, high=in_max, size=in_shape, dtype=dtype),
+ }
+ output_list = generate_ref_data(orig_mod["main"], inputs)
+
+ def checker(base_path: str) -> bool:
+ def read_file(path):
+ with open(path) as f:
+ return f.read()
+
+ test = read_file(base_path + "/build/test.c")
+ test_check = "TVMGetLastError" in test
+
+ default_lib2 = read_file(base_path + "/codegen/host/src/default_lib2.c")
+ regex = (
+ r"(?s)arm_avgpool_s16(.*?)"
+ r'ARM_CMSIS_NN_ARG_ERROR: TVMAPISetLastError\("ARM_CMSIS_NN_ARG_ERROR(.*?)'
+ r'ARM_CMSIS_NN_NO_IMPL_ERROR: TVMAPISetLastError\("ARM_CMSIS_NN_NO_IMPL_ERROR'
+ )
+ default_lib2_check = re.search(regex, default_lib2) is not None
+
+ if debug_last_error:
+ return test_check and default_lib2_check
+ else:
+ return not (test_check or default_lib2_check)
+
+ result = compile_and_run(
+ AOTTestModel(
+ module=cmsisnn_mod,
+ inputs=inputs,
+ outputs=output_list,
+ params=None,
+ output_tolerance=1,
+ ),
+ create_test_runner(compiler_cpu, cpu_flags, debug_last_error=debug_last_error),
+ interface_api,
+ use_unpacked_api,
+ debug_last_error=debug_last_error,
+ checker=checker,
Review Comment:
I am little confused about the introduction of `checker` as an argument to `run_and_check`. is it not possible to assert on info available in the sources available after codegen? For instance
```
assert re.search(regex, default_lib2) is not None, "XXX pattern not found in the codegen sources"
```
--
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 #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #13643:
URL: https://github.com/apache/tvm/pull/13643#issuecomment-1362763496
Thanks @NicolaLancellotti for addressing the comments. Let's also hear from @Mousius. Alternative, here is to emit Get/Set APIs from the AOT Test runner infra.
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "NicolaLancellotti (via GitHub)" <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1118586813
##########
python/tvm/testing/aot.py:
##########
@@ -765,6 +793,8 @@ def run_and_check_body(base_path):
)
use_usmp = runner.pass_config.get("tir.usmp.enable", False)
+ options = runner.pass_config.get("relay.ext.cmsisnn.options")
+ debug_last_error = options.get("debug_last_error", False) if options else False
Review Comment:
Done. Thanks for the review!
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "Mousius (via GitHub)" <gi...@apache.org>.
Mousius commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1089394773
##########
tests/python/contrib/test_cmsisnn/test_last_error.py:
##########
@@ -0,0 +1,119 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""CMSIS-NN integration tests: test if the model builds in case debug_last_error is enabled"""
+
+import numpy as np
+import pytest
+
+import tvm
+from tvm import relay
+from tvm.relay.op.contrib import cmsisnn
+from tvm.testing.aot import get_dtype_range, generate_ref_data, AOTTestModel, compile_and_run
+
+
+from .utils import (
+ skip_if_no_reference_system,
+ make_module,
+ make_qnn_relu,
+ assert_partitioned_function,
+ create_test_runner,
+)
+
+
+def generate_variable(name, dtype="int8"):
+ return relay.var(name, shape=(1, 16, 16, 3), dtype=dtype)
+
+
+def make_model(
+ op,
+ input_0,
+ input_1,
+ input_0_scale,
+ input_0_zero_point,
+ input_1_scale,
+ input_1_zero_point,
+ relu_type="NONE",
+ out_scale=1.0 / 256,
+ out_zero_point=-128,
+):
+ """Create a Relay Function / network model"""
+ binary_op = op(
+ input_0,
+ input_1,
+ relay.const(input_0_scale, "float32"),
+ relay.const(input_0_zero_point, "int32"),
+ relay.const(input_1_scale, "float32"),
+ relay.const(input_1_zero_point, "int32"),
+ relay.const(out_scale, "float32"),
+ relay.const(out_zero_point, "int32"),
+ )
+ return make_qnn_relu(binary_op, relu_type, out_scale, out_zero_point, "int8")
+
+
+@skip_if_no_reference_system
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("debug_last_error", [True, False])
+def test_last_error(debug_last_error):
Review Comment:
This checks it compiles with last error enabled but doesn't actually check the last error which is the most important functionality in this patch - can you check the error you're expecting is printed?
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "NicolaLancellotti (via GitHub)" <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1115761965
##########
tests/python/contrib/test_cmsisnn/test_last_error.py:
##########
@@ -0,0 +1,119 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""CMSIS-NN integration tests: test if the model builds in case debug_last_error is enabled"""
+
+import numpy as np
+import pytest
+
+import tvm
+from tvm import relay
+from tvm.relay.op.contrib import cmsisnn
+from tvm.testing.aot import get_dtype_range, generate_ref_data, AOTTestModel, compile_and_run
+
+
+from .utils import (
+ skip_if_no_reference_system,
+ make_module,
+ make_qnn_relu,
+ assert_partitioned_function,
+ create_test_runner,
+)
+
+
+def generate_variable(name, dtype="int8"):
+ return relay.var(name, shape=(1, 16, 16, 3), dtype=dtype)
+
+
+def make_model(
+ op,
+ input_0,
+ input_1,
+ input_0_scale,
+ input_0_zero_point,
+ input_1_scale,
+ input_1_zero_point,
+ relu_type="NONE",
+ out_scale=1.0 / 256,
+ out_zero_point=-128,
+):
+ """Create a Relay Function / network model"""
+ binary_op = op(
+ input_0,
+ input_1,
+ relay.const(input_0_scale, "float32"),
+ relay.const(input_0_zero_point, "int32"),
+ relay.const(input_1_scale, "float32"),
+ relay.const(input_1_zero_point, "int32"),
+ relay.const(out_scale, "float32"),
+ relay.const(out_zero_point, "int32"),
+ )
+ return make_qnn_relu(binary_op, relu_type, out_scale, out_zero_point, "int8")
+
+
+@skip_if_no_reference_system
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("debug_last_error", [True, False])
+def test_last_error(debug_last_error):
Review Comment:
I've added a 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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1082846680
##########
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:
Ping for review @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] tvm-bot commented on pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13643:
URL: https://github.com/apache/tvm/pull/13643#issuecomment-1357511246
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @areusch, @ashutosh-arm <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "ashutosh-arm (via GitHub)" <gi...@apache.org>.
ashutosh-arm commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1119029716
##########
tests/python/contrib/test_cmsisnn/test_last_error.py:
##########
@@ -0,0 +1,164 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""CMSIS-NN integration tests: debug_last_error"""
+
+import re
+import numpy as np
+import pytest
+import tvm
+from tvm import relay
+from tvm.relay.op.contrib import cmsisnn
+
+from tvm.testing.aot import (
+ get_dtype_range,
+ generate_ref_data,
+ AOTTestModel,
+ compile_and_run,
+)
+from .utils import (
+ make_module,
+ get_same_padding,
+ make_qnn_relu,
+ assert_partitioned_function,
+ create_test_runner,
+)
+
+
+def make_model(
+ pool_op,
+ shape,
+ pool_size,
+ strides,
+ padding,
+ dtype,
+ scale,
+ zero_point,
+ relu_type,
+ layout,
+ input_op,
+):
+ """Create a Relay Function / network model"""
+ if input_op:
+ op = input_op
+ else:
+ op = relay.var("input", shape=shape, dtype=dtype)
+ pad_ = (0, 0, 0, 0)
+ if padding == "SAME":
+ dilation = (1, 1)
+ pad_ = get_same_padding((shape[1], shape[2]), pool_size, dilation, strides)
+ op = relay.nn.pad(
+ op,
+ pad_width=[(0, 0), (pad_[0], pad_[2]), (pad_[1], pad_[3]), (0, 0)],
+ pad_value=zero_point,
+ pad_mode="constant",
+ )
+ if pool_op.__name__ == relay.nn.avg_pool2d.__name__:
+ op = relay.cast(op, "int32")
+ op = pool_op(
+ op, pool_size=pool_size, strides=strides, padding=pad_, ceil_mode=True, layout=layout
+ )
+ if pool_op.__name__ == relay.nn.avg_pool2d.__name__:
+ op = relay.cast(op, dtype)
+ op = make_qnn_relu(op, relu_type, scale, zero_point, dtype)
+ return op
+
+
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("debug_last_error", [True, False])
+def test_last_error(debug_last_error):
+ """Tests debug_last_error"""
+ dtype = "int16"
+ in_shape = (1, 28, 28, 12)
+ pool_size = (3, 3)
+ strides = (2, 2)
+ padding = "SAME"
+ relu_type = "NONE"
+ pool_type = relay.nn.avg_pool2d
+ zero_point = -34
+ scale = 0.0256
+ compiler_cpu = "cortex-m55"
+ cpu_flags = "+nomve"
+ layout = "NHWC"
+ input_op = None
+
+ interface_api = "c"
+ use_unpacked_api = True
+
+ model = make_model(
+ pool_op=pool_type,
+ shape=in_shape,
+ pool_size=pool_size,
+ strides=strides,
+ padding=padding,
+ dtype=dtype,
+ scale=scale,
+ zero_point=zero_point,
+ relu_type=relu_type,
+ layout=layout,
+ input_op=input_op,
+ )
+ orig_mod = make_module(model)
+
+ cmsisnn_mod = cmsisnn.partition_for_cmsisnn(orig_mod)
+
+ # validate pattern matching
+ assert_partitioned_function(orig_mod, cmsisnn_mod)
+
+ # validate the output
+ in_min, in_max = get_dtype_range(dtype)
+ inputs = {
+ "input": np.random.randint(in_min, high=in_max, size=in_shape, dtype=dtype),
+ }
+ output_list = generate_ref_data(orig_mod["main"], inputs)
+
+ def checker(base_path: str) -> bool:
+ def read_file(path):
+ with open(path) as f:
+ return f.read()
+
+ test = read_file(base_path + "/build/test.c")
+ test_check = "TVMGetLastError" in test
+
+ default_lib2 = read_file(base_path + "/codegen/host/src/default_lib2.c")
+ regex = (
+ r"(?s)arm_avgpool_s16(.*?)"
+ r'ARM_CMSIS_NN_ARG_ERROR: TVMAPISetLastError\("ARM_CMSIS_NN_ARG_ERROR(.*?)'
+ r'ARM_CMSIS_NN_NO_IMPL_ERROR: TVMAPISetLastError\("ARM_CMSIS_NN_NO_IMPL_ERROR'
+ )
+ default_lib2_check = re.search(regex, default_lib2) is not None
+
+ if debug_last_error:
+ return test_check and default_lib2_check
+ else:
+ return not (test_check or default_lib2_check)
+
+ result = compile_and_run(
+ AOTTestModel(
+ module=cmsisnn_mod,
+ inputs=inputs,
+ outputs=output_list,
+ params=None,
+ output_tolerance=1,
+ ),
+ create_test_runner(compiler_cpu, cpu_flags, debug_last_error=debug_last_error),
+ interface_api,
+ use_unpacked_api,
+ debug_last_error=debug_last_error,
+ checker=checker,
Review Comment:
From offline discussion, I understand that the `checker` provides consistency to the CMSIS-NN testing and also allows for conditional execution if required in future.
--
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 merged pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "ashutosh-arm (via GitHub)" <gi...@apache.org>.
ashutosh-arm merged PR #13643:
URL: https://github.com/apache/tvm/pull/13643
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1052140042
##########
python/tvm/testing/aot.py:
##########
@@ -244,8 +244,15 @@ def _emit_main_prologue(
va_start(args, msg);
vfprintf(stdout, msg, args);
va_end(args);
-}\n
-TVM_DLL int TVMFuncRegisterGlobal(const char* name, TVMFunctionHandle f, int override) {}
+}
+tvm_crt_error_t TVMPlatformTimerStart() {
+ return kTvmErrorFunctionCallNotImplemented;
+}
+tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds) {
+ return kTvmErrorFunctionCallNotImplemented;
+}
+const TVMModule* TVMSystemLibEntryPoint(void) { return NULL; }
+
Review Comment:
Should we be adding error set/get apis instead?
##########
src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc:
##########
@@ -355,13 +355,7 @@ class CodeGenCMSISNN : public codegen::CodeGenCHost {
stream << "&" << filter_dim << ", " << filter_data << ", ";
stream << "&" << bias_dim << ", " << bias_data << ", ";
stream << "&" << output_dim << ", " << output_data << ");\n";
- PrintIndent();
- stream << "if (status != ARM_CMSIS_NN_SUCCESS) {\n";
- PrintIndent();
- PrintIndent();
- stream << "return -1;\n";
- PrintIndent();
- stream << "}\n";
+ EmitCheckError();
Review Comment:
nit: does `EmitErrorCheck` sound better? your call.
##########
python/tvm/testing/aot.py:
##########
@@ -455,6 +465,7 @@ def _emit_main_common_includes(main_file, custom_includes):
main_file.write("#include <math.h>\n")
main_file.write('#include "tvm/runtime/c_runtime_api.h"\n')
main_file.write('#include "tvm/runtime/crt/stack_allocator.h"\n')
+ main_file.write('#include "tvm/runtime/crt/module.h"\n')
Review Comment:
interesting, I thought `c_runtime_api.h` would be enough :smile_cat:
--
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] NicolaLancellotti commented on a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1052365665
##########
python/tvm/testing/aot.py:
##########
@@ -244,8 +244,15 @@ def _emit_main_prologue(
va_start(args, msg);
vfprintf(stdout, msg, args);
va_end(args);
-}\n
-TVM_DLL int TVMFuncRegisterGlobal(const char* name, TVMFunctionHandle f, int override) {}
+}
+tvm_crt_error_t TVMPlatformTimerStart() {
+ return kTvmErrorFunctionCallNotImplemented;
+}
+tvm_crt_error_t TVMPlatformTimerStop(double* elapsed_time_seconds) {
+ return kTvmErrorFunctionCallNotImplemented;
+}
+const TVMModule* TVMSystemLibEntryPoint(void) { return NULL; }
+
Review Comment:
`TVMGetLastError` and `TVMAPISetLastError` are defined in `crt_runtime_api.c`.
##########
python/tvm/testing/aot.py:
##########
@@ -455,6 +465,7 @@ def _emit_main_common_includes(main_file, custom_includes):
main_file.write("#include <math.h>\n")
main_file.write('#include "tvm/runtime/c_runtime_api.h"\n')
main_file.write('#include "tvm/runtime/crt/stack_allocator.h"\n')
+ main_file.write('#include "tvm/runtime/crt/module.h"\n')
Review Comment:
`tvm/runtime/crt/module.h` defines `TVMModule` needed by
`TVMModule* TVMSystemLibEntryPoint(void)`.
##########
src/relay/backend/contrib/cmsisnn/tir_to_runtime.cc:
##########
@@ -355,13 +355,7 @@ class CodeGenCMSISNN : public codegen::CodeGenCHost {
stream << "&" << filter_dim << ", " << filter_data << ", ";
stream << "&" << bias_dim << ", " << bias_data << ", ";
stream << "&" << output_dim << ", " << output_data << ");\n";
- PrintIndent();
- stream << "if (status != ARM_CMSIS_NN_SUCCESS) {\n";
- PrintIndent();
- PrintIndent();
- stream << "return -1;\n";
- PrintIndent();
- stream << "}\n";
+ EmitCheckError();
Review Comment:
I agree, I'll update the pr.
--
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 a diff in pull request #13643: [CMSIS-NN] Add a runtime error message
Posted by "Mousius (via GitHub)" <gi...@apache.org>.
Mousius commented on code in PR #13643:
URL: https://github.com/apache/tvm/pull/13643#discussion_r1118549414
##########
python/tvm/testing/aot.py:
##########
@@ -765,6 +793,8 @@ def run_and_check_body(base_path):
)
use_usmp = runner.pass_config.get("tir.usmp.enable", False)
+ options = runner.pass_config.get("relay.ext.cmsisnn.options")
+ debug_last_error = options.get("debug_last_error", False) if options else False
Review Comment:
The AOT test runner shouldn't have knowledge of `CMSIS-NN` specific arguments, can we pass `debug_last_error` as a parameter?
--
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