From 927e3dd36ed90c9d5c210cec6c795152912995dd Mon Sep 17 00:00:00 2001 From: Andrew Reusch Date: Mon, 5 Oct 2020 12:43:42 -0700 Subject: [PATCH 1/5] fix up compiler options, add separate generated compiler options. - allows use of -Werror with microTVM builds. --- python/tvm/micro/build.py | 41 +++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/python/tvm/micro/build.py b/python/tvm/micro/build.py index da0cc45cfeb6..3a7392a78bb8 100644 --- a/python/tvm/micro/build.py +++ b/python/tvm/micro/build.py @@ -67,9 +67,13 @@ def path(self): RUNTIME_SRC_REGEX = re.compile(r"^.*\.cc?$", re.IGNORECASE) +_COMMON_CFLAGS = ["-Wall", "-Werror"] + + _CRT_DEFAULT_OPTIONS = { - "ccflags": ["-std=c++11"], - "ldflags": ["-std=gnu++14"], + "cflags": ["-std=c11"] + _COMMON_CFLAGS, + "ccflags": ["-std=c++11"] + _COMMON_CFLAGS, + "ldflags": ["-std=c++11"], "include_dirs": [ f"{TVM_ROOT_DIR}/include", f"{TVM_ROOT_DIR}/3rdparty/dlpack/include", @@ -77,21 +81,30 @@ def path(self): f"{TVM_ROOT_DIR}/3rdparty/dmlc-core/include", f"{CRT_ROOT_DIR}/include", ], - "profile": {"common": ["-Wno-unused-variable"]}, } +_CRT_GENERATED_LIB_OPTIONS = copy.copy(_CRT_DEFAULT_OPTIONS) + + + +# Disable due to limitation in the TVM C codegen, which generates lots of local variable +# declarations at the top of generated code without caring whether they're used. +# Example: +# void* arg0 = (((TVMValue*)args)[0].v_handle); +# int32_t arg0_code = ((int32_t*)arg_type_ids)[(0)]; +_CRT_GENERATED_LIB_OPTIONS['cflags'].append("-Wno-unused-variable") def default_options(target_include_dir): """Return default opts passed to Compile commands.""" bin_opts = copy.deepcopy(_CRT_DEFAULT_OPTIONS) bin_opts["include_dirs"].append(target_include_dir) lib_opts = copy.deepcopy(bin_opts) - lib_opts["profile"]["common"].append("-Werror") lib_opts["cflags"] = ["-Wno-error=incompatible-pointer-types"] return {"bin_opts": bin_opts, "lib_opts": lib_opts} -def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=None): +def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=None, + generated_lib_opts=None): """Build the on-device runtime, statically linking the given modules. Parameters @@ -102,11 +115,15 @@ def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=No module : IRModule Module to statically link. - lib_opts : dict - Extra kwargs passed to library(), + lib_opts : Optional[dict] + The `options` parameter passed to compiler.library(). + + bin_opts : Optional[dict] + The `options` parameter passed to compiler.binary(). - bin_opts : dict - Extra kwargs passed to binary(), + generated_lib_opts : Optional[dict] + The `options` parameter passed to compiler.library() when compiling the generated TVM C + source module. Returns ------- @@ -115,6 +132,10 @@ def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=No """ lib_opts = _CRT_DEFAULT_OPTIONS if lib_opts is None else lib_opts bin_opts = _CRT_DEFAULT_OPTIONS if bin_opts is None else bin_opts + generated_lib_opts = ( + _CRT_GENERATED_LIB_OPTIONS + if generated_lib_opts is None + else generated_lib_opts) mod_build_dir = workspace.relpath(os.path.join("build", "module")) os.makedirs(mod_build_dir) @@ -136,7 +157,7 @@ def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=No libs.append(compiler.library(lib_build_dir, lib_srcs, lib_opts)) - libs.append(compiler.library(mod_build_dir, [mod_src_path], lib_opts)) + libs.append(compiler.library(mod_build_dir, [mod_src_path], generated_lib_opts)) runtime_build_dir = workspace.relpath(f"build/runtime") os.makedirs(runtime_build_dir) From 3f3b9413fbc1beba9c910cb021566deb7a69f6df Mon Sep 17 00:00:00 2001 From: Andrew Reusch Date: Mon, 5 Oct 2020 12:44:29 -0700 Subject: [PATCH 2/5] fix host debug flow for microTVM - some typos. still need a unit test here but that is complex. --- python/tvm/micro/compiler.py | 2 +- python/tvm/micro/debugger.py | 10 +++++----- python/tvm/micro/transport.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/python/tvm/micro/compiler.py b/python/tvm/micro/compiler.py index fa5887c8ccf2..307c9809fc21 100644 --- a/python/tvm/micro/compiler.py +++ b/python/tvm/micro/compiler.py @@ -320,7 +320,7 @@ def flash(self, micro_binary): [micro_binary.abspath(micro_binary.binary_file)] ) return transport.DebugWrapperTransport( - debugger=gdb_wrapper, transport=gdb_wrapper.Transport() + debugger=gdb_wrapper, transport=gdb_wrapper.transport() ) return transport.SubprocessTransport([micro_binary.abspath(micro_binary.binary_file)]) diff --git a/python/tvm/micro/debugger.py b/python/tvm/micro/debugger.py index 33db55457764..8c7a300c2aae 100644 --- a/python/tvm/micro/debugger.py +++ b/python/tvm/micro/debugger.py @@ -67,7 +67,7 @@ def start(self): self.did_terminate = threading.Event() self.old_signal = signal.signal(signal.SIGINT, signal.SIG_IGN) self.popen = subprocess.Popen(**kwargs) - threading.Thread(target=self._WaitRestoreSignal).start() + threading.Thread(target=self._wait_restore_signal).start() def stop(self): self.did_terminate.set() @@ -130,14 +130,14 @@ def _wait_for_process_death(self): self.stdout.close() def start(self): - to_return = super(GdbTransportDebugger, self).Start() + to_return = super(GdbTransportDebugger, self).start() threading.Thread(target=self._wait_for_process_death, daemon=True).start() return to_return def stop(self): self.stdin.close() self.stdout.close() - super(GdbTransportDebugger, self).Stop() + super(GdbTransportDebugger, self).stop() class _Transport(_transport.Transport): def __init__(self, gdb_transport_debugger): @@ -189,11 +189,11 @@ def popen_kwargs(self): def start(self): if self.wrapping_context_manager is not None: self.wrapping_context_manager.__enter__() - super(GdbRemoteDebugger, self).Start() + super(GdbRemoteDebugger, self).start() def stop(self): try: - super(GdbRemoteDebugger, self).Stop() + super(GdbRemoteDebugger, self).stop() finally: if self.wrapping_context_manager is not None: self.wrapping_context_manager.__exit__(None, None, None) diff --git a/python/tvm/micro/transport.py b/python/tvm/micro/transport.py index f9b41a47fb52..c789bc6c856a 100644 --- a/python/tvm/micro/transport.py +++ b/python/tvm/micro/transport.py @@ -216,12 +216,12 @@ def __init__(self, debugger, transport): self.debugger.on_terminate_callbacks.append(self.transport.close) def open(self): - self.debugger.Start() + self.debugger.start() try: self.transport.open() except Exception: - self.debugger.Stop() + self.debugger.stop() raise def write(self, data): @@ -232,7 +232,7 @@ def read(self, n): def close(self): self.transport.close() - self.debugger.Stop() + self.debugger.stop() TransportContextManager = typing.ContextManager[Transport] From e85134ca8b2081fd951f45feeda158d93aab62c5 Mon Sep 17 00:00:00 2001 From: Andrew Reusch Date: Mon, 5 Oct 2020 12:45:15 -0700 Subject: [PATCH 3/5] Stop using builtin math.h functions. - always codegen with math.h - add unit test --- python/tvm/micro/build.py | 5 +++++ src/target/source/codegen_c_host.cc | 1 + tests/python/unittest/test_crt.py | 17 +++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/python/tvm/micro/build.py b/python/tvm/micro/build.py index 3a7392a78bb8..dbd6b2e1de52 100644 --- a/python/tvm/micro/build.py +++ b/python/tvm/micro/build.py @@ -94,6 +94,11 @@ def path(self): # void* arg0 = (((TVMValue*)args)[0].v_handle); # int32_t arg0_code = ((int32_t*)arg_type_ids)[(0)]; _CRT_GENERATED_LIB_OPTIONS['cflags'].append("-Wno-unused-variable") + + +# Many TVM-intrinsic operators (i.e. expf, in particular) +_CRT_GENERATED_LIB_OPTIONS["cflags"].append("-fno-builtin") + def default_options(target_include_dir): """Return default opts passed to Compile commands.""" bin_opts = copy.deepcopy(_CRT_DEFAULT_OPTIONS) diff --git a/src/target/source/codegen_c_host.cc b/src/target/source/codegen_c_host.cc index 5bd7b2e91c31..dc93c31e7024 100644 --- a/src/target/source/codegen_c_host.cc +++ b/src/target/source/codegen_c_host.cc @@ -42,6 +42,7 @@ void CodeGenCHost::Init(bool output_ssa, bool emit_asserts) { declared_globals_.clear(); decl_stream << "#include \"tvm/runtime/c_runtime_api.h\"\n"; decl_stream << "#include \"tvm/runtime/c_backend_api.h\"\n"; + decl_stream << "#include \n"; decl_stream << "void* " << module_name_ << " = NULL;\n"; CodeGenC::Init(output_ssa); } diff --git a/tests/python/unittest/test_crt.py b/tests/python/unittest/test_crt.py index 3fc2e045868a..21c423b0073c 100644 --- a/tests/python/unittest/test_crt.py +++ b/tests/python/unittest/test_crt.py @@ -144,7 +144,24 @@ def @main(%a : Tensor[(1, 2), uint8], %b : Tensor[(1, 2), uint8]) { assert (out.asnumpy() == np.array([6, 10])).all() +def test_std_math_functions(): + """Verify that standard math functions can be used.""" + workspace = tvm.micro.Workspace() + A = tvm.te.placeholder((2,), dtype="float32", name='A') + B = tvm.te.compute(A.shape, lambda i: tvm.te.exp(A[i]), name="B") + s = tvm.te.create_schedule(B.op) + + with _make_sess_from_op(workspace, "myexpf", s, [A, B]) as sess: + A_data = tvm.nd.array(np.array([2.0, 3.0], dtype="float32"), ctx=sess.context) + B_data = tvm.nd.array(np.array([2.0, 3.0], dtype="float32"), ctx=sess.context) + lib = sess.get_system_lib() + func = lib["myexpf"] + func(A_data, B_data) + np.testing.assert_allclose(B_data.asnumpy(), np.array([ 7.389056, 20.085537])) + + if __name__ == "__main__": test_compile_runtime() test_reset() test_graph_runtime() + test_std_math_functions() From e4b0b81299ae0b9152c03ccdac34535d4858295a Mon Sep 17 00:00:00 2001 From: Andrew Reusch Date: Mon, 5 Oct 2020 13:04:05 -0700 Subject: [PATCH 4/5] fix additional warnings not present on osx --- src/runtime/crt/common/func_registry.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/crt/common/func_registry.c b/src/runtime/crt/common/func_registry.c index 1ffffa5eaf62..cc1ba56c8cdc 100644 --- a/src/runtime/crt/common/func_registry.c +++ b/src/runtime/crt/common/func_registry.c @@ -136,8 +136,12 @@ tvm_crt_error_t TVMMutableFuncRegistry_Set(TVMMutableFuncRegistry* reg, const ch idx++; } + if (reg_name_ptr > ((const char*)reg->registry.funcs)) { + return kTvmErrorFunctionRegistryFull; + } + size_t name_len = strlen(name); - ssize_t names_bytes_remaining = ((const char*)reg->registry.funcs) - reg_name_ptr; + size_t names_bytes_remaining = ((const char*)reg->registry.funcs) - reg_name_ptr; if (idx >= reg->max_functions || name_len + 1 > names_bytes_remaining) { return kTvmErrorFunctionRegistryFull; } From 358e40b3c138acf66e132b864314a8de3287270c Mon Sep 17 00:00:00 2001 From: Andrew Reusch Date: Mon, 5 Oct 2020 13:45:03 -0700 Subject: [PATCH 5/5] black format --- python/tvm/micro/build.py | 14 +++++++------- tests/python/unittest/test_crt.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/python/tvm/micro/build.py b/python/tvm/micro/build.py index dbd6b2e1de52..908bc9637dcf 100644 --- a/python/tvm/micro/build.py +++ b/python/tvm/micro/build.py @@ -87,18 +87,18 @@ def path(self): _CRT_GENERATED_LIB_OPTIONS = copy.copy(_CRT_DEFAULT_OPTIONS) - # Disable due to limitation in the TVM C codegen, which generates lots of local variable # declarations at the top of generated code without caring whether they're used. # Example: # void* arg0 = (((TVMValue*)args)[0].v_handle); # int32_t arg0_code = ((int32_t*)arg_type_ids)[(0)]; -_CRT_GENERATED_LIB_OPTIONS['cflags'].append("-Wno-unused-variable") +_CRT_GENERATED_LIB_OPTIONS["cflags"].append("-Wno-unused-variable") # Many TVM-intrinsic operators (i.e. expf, in particular) _CRT_GENERATED_LIB_OPTIONS["cflags"].append("-fno-builtin") + def default_options(target_include_dir): """Return default opts passed to Compile commands.""" bin_opts = copy.deepcopy(_CRT_DEFAULT_OPTIONS) @@ -108,8 +108,9 @@ def default_options(target_include_dir): return {"bin_opts": bin_opts, "lib_opts": lib_opts} -def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=None, - generated_lib_opts=None): +def build_static_runtime( + workspace, compiler, module, lib_opts=None, bin_opts=None, generated_lib_opts=None +): """Build the on-device runtime, statically linking the given modules. Parameters @@ -138,9 +139,8 @@ def build_static_runtime(workspace, compiler, module, lib_opts=None, bin_opts=No lib_opts = _CRT_DEFAULT_OPTIONS if lib_opts is None else lib_opts bin_opts = _CRT_DEFAULT_OPTIONS if bin_opts is None else bin_opts generated_lib_opts = ( - _CRT_GENERATED_LIB_OPTIONS - if generated_lib_opts is None - else generated_lib_opts) + _CRT_GENERATED_LIB_OPTIONS if generated_lib_opts is None else generated_lib_opts + ) mod_build_dir = workspace.relpath(os.path.join("build", "module")) os.makedirs(mod_build_dir) diff --git a/tests/python/unittest/test_crt.py b/tests/python/unittest/test_crt.py index 21c423b0073c..9893a6449d96 100644 --- a/tests/python/unittest/test_crt.py +++ b/tests/python/unittest/test_crt.py @@ -147,7 +147,7 @@ def @main(%a : Tensor[(1, 2), uint8], %b : Tensor[(1, 2), uint8]) { def test_std_math_functions(): """Verify that standard math functions can be used.""" workspace = tvm.micro.Workspace() - A = tvm.te.placeholder((2,), dtype="float32", name='A') + A = tvm.te.placeholder((2,), dtype="float32", name="A") B = tvm.te.compute(A.shape, lambda i: tvm.te.exp(A[i]), name="B") s = tvm.te.create_schedule(B.op) @@ -157,7 +157,7 @@ def test_std_math_functions(): lib = sess.get_system_lib() func = lib["myexpf"] func(A_data, B_data) - np.testing.assert_allclose(B_data.asnumpy(), np.array([ 7.389056, 20.085537])) + np.testing.assert_allclose(B_data.asnumpy(), np.array([7.389056, 20.085537])) if __name__ == "__main__":