[uTVM] Initial BYOC support with c-source module #6950

manupak · 2020-11-23T13:34:50Z

Current flow uses saves the module as a c source and compiles it seperately. This blocks multi-module compilation.
This patch upgrades it to use export_library which supports external modules.
Added BYOC support for uTVM and testing
A new member for IRModule to carry external function names as they are required to create the function registry when creating a system-lib.

manupak · 2020-11-23T17:57:11Z

comaniac · 2020-11-23T18:24:09Z

I'm not familiar with uTVM. Out of curiosity, how uTVM uses BYOC codegen C? I ask because BYOC codegen C is just for demonstration purpose so its functionality is not complete.

manupak · 2020-11-23T19:20:36Z

@comaniac , Likewise this is also for demonstration purpose but in the uTVM context. i.e., if we can annotate and offload a set of operations and produce a c-source that can do the required computations and return the DLTensors, this is to show it works in the uTVM context.

Thus, we do not intend to use "ccompiler" as the solution rather to show a external codegen that could create c-source the place of the "ccompiler", the changes done here should enable the compilation of a such a source. Moreover, we do not prefer the metadata packing of Imports (using PackImports* methods) as they will re-create the artifacts in the volatile memory. However, having a c-source gives much better control as to where the artifacts should be placed through a standard linker script.

comaniac

LGTM to the codegen C part.

src/relay/backend/contrib/codegen_c/codegen_c.h

python/tvm/micro/build.py

include/tvm/ir/module.h

src/relay/backend/contrib/codegen_c/codegen.cc

include/tvm/ir/module.h

python/tvm/runtime/module.py

python/tvm/micro/build.py

include/tvm/ir/module.h

manupak · 2021-01-13T09:14:39Z

Hi @areusch @zhiics ,

Happy new year to you all! .

After CSourceMetadata module, this PR got reduced to introducing a simple test for utvm byoc example and some adjustments to external c-source code generator to make it applicable for uTVM as well. We would like to carry this test to ensure external module compilation works in uTVM.

Please take a look when you have time.

src/relay/backend/contrib/codegen_c/codegen.cc

zhiics · 2021-01-13T12:17:39Z

src/relay/backend/contrib/codegen_c/codegen.cc

+      // These are only needed to handle metadata copying
+      code_stream_ << "#include <tvm/runtime/container.h>\n";
+      code_stream_ << "#include <tvm/runtime/packed_func.h>\n";
+      code_stream_ << "#include <dlpack/dlpack.h>\n";


I think packed_func is probably needed to handle packed args. dlpack has to be included if we have DLTensor. The reason it doesn't cause errors is probably because c_runtime_api.h includes it as well or we don't use DLTensor at all.

Yes, dlpack is in the c_runtime_api.h, thus we can remove it altogether.

Do we need packed_func.h ? -- I would like to avoid if the constants copying process is not required. The obvious usage is from TVM_DLL_EXPORT_TYPED_FUNC macro here which creates C++ code. Hence the gating based on presence of constants.

@zhiics to clarify this codegen is just meant to generate C code, correct? we just got away with generating C++ code because export_library always writes .cc files. i also see that the BYOC blogpost has -std=c++14 as a compiler arg, but is that necessary with this simple codegen_c?

it looks like the purpose of TVM_DLL_EXPORT_TYPED_FUNC is to do some limited argument type validation and translate the function call signature from TVMBackendPackedCFunc into a series of primitive pointers. what's confusing is that theoretically we shouldn't need to go through C++ to do this, and you wouldn't expect that to happen since this is called, well, CodegenC.

it seems like the typechecking is actually not buying us much, because it doesn't actually check the element datatype of any DLTensors passed-in. Here, we just static_cast and presume the element datatype matches. if that's true, i'd propose (perhaps not in this PR) that we get rid of the dependency on TVM_DLL_EXPORT_TYPED_FUNC here. I think we should aim to have the same C header dependencies as the c target codegen in src/target/source/codegen_c_host.cc.

@areusch Yeah, we used it to generated C++ code before when working on BYOC. But I think you are right, we should just export C code and get rid of the dependency of TVM_DLL_EXPORT_TYPED_FUNC

I removed dlpack for now and since we need to have Array<NDArray> here, we cant get away not using C++ here. Therefore, I might as well use the macro for convenience here.

However, I made it obvious that this part is not codegen'd/ not visible for a c-compiler.

areusch

@manupa-arm happy new year and thanks for continuing to push this forward. this looks pretty good, just a couple small things and some clarifying questions (as I'm not as familiar with codegen_c internals as I should be).

areusch · 2021-01-13T16:54:42Z

src/relay/backend/contrib/codegen_c/codegen.cc

+      code_stream_ << "#include <tvm/runtime/container.h>\n";
+      code_stream_ << "#include <tvm/runtime/packed_func.h>\n";
+      code_stream_ << "#include <dlpack/dlpack.h>\n";
+      code_stream_ << "using namespace tvm::runtime;\n";


how come we dropped std::malloc yet have a using decl here?

Actually this segment is only to be generated in the presence of constant copying process and makes this source a C++ one. To this end, there is a init function that accepts "Array<NDArray> arr" (See in codegen_c.h) that is in that namespace. I could not think of a way to make it a C source immediately.

Honestly, I was less worried about it as this is an example codegen and in utvm we dont want to be doing constant copying anyway. Therefore this segment will not be codegen'd. I understand its not ideal. However, the purpose of this test was to keep c-source base external codegen support (without constant copying -- which is controlled by constant extraction relay pass) in uTVM

hrm. should we then add code like:

#ifdef __cplusplus using std::malloc; #endif

you're right this is a demo codegen, perhaps we don't need to resolve this. it's just confusing to create c++ code in a codegen named codegen_c.

@areusch So I made it obvious this segment is C++ in the code. See if it works.

tests/micro/qemu/test_zephyr.py

areusch · 2021-01-13T17:08:55Z

tests/micro/qemu/test_zephyr.py

+
+def check_result(relay_mod, model, zephyr_board, map_inputs, out_shape, result):
+    """Helper function to verify results"""
+    tol = 1e-5


can you make this a module-level all-CAPS constant?

areusch · 2021-01-13T17:14:30Z

src/relay/backend/contrib/codegen_c/codegen.cc

+    code_stream_ << "#include <tvm/runtime/c_backend_api.h>\n";
+    code_stream_ << "#include <tvm/runtime/crt/module.h>\n";
+    if (!variables.empty()) {
+      // These are only needed to handle metadata copying


nitpick: delete "only" here--that statement is true only at this moment.

Done and added a longer description to say why C++ code is generated, at least in this moment.

areusch · 2021-01-13T17:53:39Z

src/relay/backend/contrib/codegen_c/codegen.cc

+      // These are only needed to handle metadata copying
+      code_stream_ << "#include <tvm/runtime/container.h>\n";
+      code_stream_ << "#include <tvm/runtime/packed_func.h>\n";
+      code_stream_ << "#include <dlpack/dlpack.h>\n";


@zhiics to clarify this codegen is just meant to generate C code, correct? we just got away with generating C++ code because export_library always writes .cc files. i also see that the BYOC blogpost has -std=c++14 as a compiler arg, but is that necessary with this simple codegen_c?

it looks like the purpose of TVM_DLL_EXPORT_TYPED_FUNC is to do some limited argument type validation and translate the function call signature from TVMBackendPackedCFunc into a series of primitive pointers. what's confusing is that theoretically we shouldn't need to go through C++ to do this, and you wouldn't expect that to happen since this is called, well, CodegenC.

it seems like the typechecking is actually not buying us much, because it doesn't actually check the element datatype of any DLTensors passed-in. Here, we just static_cast and presume the element datatype matches. if that's true, i'd propose (perhaps not in this PR) that we get rid of the dependency on TVM_DLL_EXPORT_TYPED_FUNC here. I think we should aim to have the same C header dependencies as the c target codegen in src/target/source/codegen_c_host.cc.

tests/micro/qemu/test_zephyr.py

This commit mainly introduces a byoc c-source module example to uTVM. Moreover, it carries certain modifications to the example codegen_c external module generator code to generate utvm friendly c-source. Change-Id: I09f3a42017d518dd5b6c89e3fe0a0332b80088b0

manupak · 2021-01-15T10:05:32Z

Hi @areusch @zhiics ,

I've addressed all the minor comments.

The current limitation of making this generator a pure C code generator is that is carries a Array<NDArray> for constants that will be copied in, if required by the compilation pipeline. I feel changing this is a bigger work and its only there to test the external codegen/runtime pathway (especially the JSONRuntime) handles the constant copying. Therefore, given that we have Array<NDArray>, I might as well use the macro. However, I clearly commented and made it obvious that bit of the code is C++ (using ifdef __cplusplus as @areusch suggested.)

Therefore, in the absense of the constant copying, this will generate a pure C code -- If you see I have removed macro usage for the main compute function (its just the constant copying one uses it).

Let me know your thoughts and what more could be done about it, if any.

areusch

looks good, thanks for bearing with me @manupa-arm!

zhiics

LGTM. Thanks for the effort.

zhiics · 2021-01-15T23:47:41Z

Thanks @manupa-arm @areusch

This commit mainly introduces a byoc c-source module example to uTVM. Moreover, it carries certain modifications to the example codegen_c external module generator code to generate utvm friendly c-source. Change-Id: I09f3a42017d518dd5b6c89e3fe0a0332b80088b0

comaniac reviewed Nov 23, 2020

View reviewed changes

src/relay/backend/contrib/codegen_c/codegen_c.h Show resolved Hide resolved

areusch reviewed Nov 23, 2020

View reviewed changes

python/tvm/micro/build.py Outdated Show resolved Hide resolved

include/tvm/ir/module.h Outdated Show resolved Hide resolved

src/relay/backend/contrib/codegen_c/codegen.cc Outdated Show resolved Hide resolved

src/relay/backend/contrib/codegen_c/codegen.cc Outdated Show resolved Hide resolved

zhiics reviewed Nov 24, 2020

View reviewed changes

include/tvm/ir/module.h Outdated Show resolved Hide resolved

manupak force-pushed the utvm_byoc branch from 0480af2 to 891e53f Compare November 25, 2020 16:00

areusch reviewed Nov 25, 2020

View reviewed changes

python/tvm/runtime/module.py Outdated Show resolved Hide resolved

python/tvm/micro/build.py Outdated Show resolved Hide resolved

include/tvm/ir/module.h Outdated Show resolved Hide resolved

manupak force-pushed the utvm_byoc branch 2 times, most recently from 891e53f to 16a51f9 Compare January 12, 2021 18:39

zhiics reviewed Jan 13, 2021

View reviewed changes

src/relay/backend/contrib/codegen_c/codegen.cc Outdated Show resolved Hide resolved

zhiics reviewed Jan 13, 2021

View reviewed changes

areusch requested changes Jan 13, 2021

View reviewed changes

manupak force-pushed the utvm_byoc branch from 16a51f9 to 79238b9 Compare January 14, 2021 11:45

manupak force-pushed the utvm_byoc branch from 79238b9 to 68c0c30 Compare January 14, 2021 11:53

areusch approved these changes Jan 15, 2021

View reviewed changes

zhiics approved these changes Jan 15, 2021

View reviewed changes

zhiics merged commit b52267e into apache:main Jan 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[uTVM] Initial BYOC support with c-source module #6950

[uTVM] Initial BYOC support with c-source module #6950

manupak commented Nov 23, 2020

manupak commented Nov 23, 2020

comaniac commented Nov 23, 2020

manupak commented Nov 23, 2020 •

edited

Loading

comaniac left a comment

manupak commented Jan 13, 2021

zhiics Jan 13, 2021 •

edited

Loading

manupak Jan 13, 2021

areusch Jan 13, 2021

zhiics Jan 13, 2021

manupak Jan 14, 2021 •

edited

Loading

areusch left a comment

areusch Jan 13, 2021

manupak Jan 13, 2021 •

edited

Loading

areusch Jan 13, 2021

manupak Jan 14, 2021

areusch Jan 13, 2021

manupak Jan 14, 2021

areusch Jan 13, 2021

manupak Jan 14, 2021 •

edited

Loading

areusch Jan 13, 2021

manupak commented Jan 15, 2021

areusch left a comment

zhiics left a comment

zhiics commented Jan 15, 2021

[uTVM] Initial BYOC support with c-source module #6950

[uTVM] Initial BYOC support with c-source module #6950

Conversation

manupak commented Nov 23, 2020

manupak commented Nov 23, 2020

comaniac commented Nov 23, 2020

manupak commented Nov 23, 2020 • edited Loading

comaniac left a comment

Choose a reason for hiding this comment

manupak commented Jan 13, 2021

zhiics Jan 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Jan 14, 2021 • edited Loading

Choose a reason for hiding this comment

areusch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Jan 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak Jan 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manupak commented Jan 15, 2021

areusch left a comment

Choose a reason for hiding this comment

zhiics left a comment

Choose a reason for hiding this comment

zhiics commented Jan 15, 2021

manupak commented Nov 23, 2020 •

edited

Loading

zhiics Jan 13, 2021 •

edited

Loading

manupak Jan 14, 2021 •

edited

Loading

manupak Jan 13, 2021 •

edited

Loading

manupak Jan 14, 2021 •

edited

Loading