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 2021/06/18 21:13:14 UTC
[GitHub] [tvm] Mousius opened a new pull request #8280: Introduce --interface-api={c,packed} parameter
Mousius opened a new pull request #8280:
URL: https://github.com/apache/tvm/pull/8280
This introduces structures generated to provide a documented and stable user
friendly interface to a TVM generated model, as can be seen in the AOT
demo application:
```
struct tvm_default_inputs inputs = {
.input_1 = input_data,
};
struct tvm_default_outputs outputs = {
.output = output_data,
};
int ret_val = tvm_default_run(&inputs, &outputs, NULL, NULL);
```
To facilitate this, some other changes are included:
* Removed dependency on `aot_executor.{c,h}` in tests, pending the
discussion in the [C interface RFC](https://discuss.tvm.apache.org/t/rfc-utvm-embedded-c-runtime-interface/9951) as to whether we keep them.
* Moved creation of test DLTensor's into the AOT test utils, in future this
can be replaced by loading via the Python API or otherwise
* Introduce `parametrize_aot_options` which can be used to test
permutations of AOT which work together - for now this filters C
interface and packed operators
* Updated demo application to generate the header for demonstration
purposes, we should consider porting the demo application to Model
Library Format and using the toolchain in the Zephyr App via CMake
instead?
This patch builds upon the improvements @giuseros made to AOT testing
and will greatly benefit from name mangling from #8014
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] Mousius commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-872956404
@areusch I think this is closer now that name mangling has been added, though I have no idea why Windows CI has failed on this particular PR - any insights appreciated :smile_cat:
cc: @manupa-arm @stoa
--
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 pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-887649398
Given this is the interface that users will use from a C application, we should strive to match that style rather than C++ guidelines. So I'd suggest in user facing C code should be snake case for structs and functions?
--
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] areusch commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r665615332
##########
File path: include/tvm/runtime/module.h
##########
@@ -232,6 +232,8 @@ constexpr const char* tvm_param_prefix = "__tvm_param__";
constexpr const char* tvm_lookup_linked_param = "_lookup_linked_param";
/*! \brief The main AOT executor function */
constexpr const char* tvm_run_func_suffix = "run_model";
Review comment:
do we need both this and the one below?
##########
File path: include/tvm/runtime/module.h
##########
@@ -232,6 +232,8 @@ constexpr const char* tvm_param_prefix = "__tvm_param__";
constexpr const char* tvm_lookup_linked_param = "_lookup_linked_param";
/*! \brief The main AOT executor function */
constexpr const char* tvm_run_func_suffix = "run_model";
+/*! \brief The models entrypoint function which calls the executor */
Review comment:
since Executor is an interface and run_func doesn't conform to that, let's find another name here.
##########
File path: src/target/source/source_module.cc
##########
@@ -220,37 +221,85 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
code_ << "}\n";
}
- void GenerateEntrypointForPackedAPI(const std::string& run_func) {
+ void GenerateEntrypointForPackedAPI(const std::string& entrypoint_name,
+ const std::string& run_func) {
code_ << "TVM_DLL int32_t " << run_func;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle);\n";
- code_ << "static int32_t " << ::tvm::runtime::symbol::tvm_module_main;
+ code_ << "int32_t " << entrypoint_name;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle) {\n";
code_ << "return " << run_func;
code_ << "(args, type_code, num_args, out_value, out_type_code, resource_handle);\n";
code_ << "}\n";
}
+ void GenerateCInterfaceEntrypoint(const std::string& entrypoint_name, const std::string& run_func,
+ const std::string& mod_name) {
+ code_ << "#include <" << mod_name << ".h>\n";
Review comment:
will this also be tvmgen_?
##########
File path: src/target/source/source_module.cc
##########
@@ -220,37 +221,85 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
code_ << "}\n";
}
- void GenerateEntrypointForPackedAPI(const std::string& run_func) {
+ void GenerateEntrypointForPackedAPI(const std::string& entrypoint_name,
+ const std::string& run_func) {
code_ << "TVM_DLL int32_t " << run_func;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle);\n";
- code_ << "static int32_t " << ::tvm::runtime::symbol::tvm_module_main;
+ code_ << "int32_t " << entrypoint_name;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle) {\n";
code_ << "return " << run_func;
code_ << "(args, type_code, num_args, out_value, out_type_code, resource_handle);\n";
code_ << "}\n";
}
+ void GenerateCInterfaceEntrypoint(const std::string& entrypoint_name, const std::string& run_func,
+ const std::string& mod_name) {
+ code_ << "#include <" << mod_name << ".h>\n";
+ code_ << "TVM_DLL int32_t " << run_func << "(";
+ unsigned int total_args = (metadata_->inputs.size() + metadata_->num_outputs);
+ for (unsigned int i = 0; i < total_args; ++i) {
+ code_ << "void* arg" << i;
+ if (i + 1 != total_args) {
+ code_ << ",";
+ }
+ }
+ code_ << ");\n";
+ code_ << "int32_t " << entrypoint_name << "(";
+ code_ << "struct " << runtime::get_name_mangled(mod_name, "inputs") << "* inputs,"
+ << "struct " << runtime::get_name_mangled(mod_name, "outputs") << "* outputs,"
+ << "struct " << runtime::get_name_mangled(mod_name, "memory") << "* memory,"
+ << "struct " << runtime::get_name_mangled(mod_name, "devices") << "* devices"
+ << ") {";
+ code_ << "return " << run_func << "(";
+ for (const auto& input : metadata_->inputs) {
+ code_ << "inputs->" << input->name_hint() << ",";
+ }
+ if (metadata_->num_outputs == 1) {
+ code_ << "outputs->output";
+ } else {
+ for (int i = 0; i < metadata_->num_outputs; ++i) {
+ code_ << "outputs->output" << i;
+ if (i + 1 != metadata_->num_outputs) {
+ code_ << ",";
+ }
+ }
+ }
+ code_ << ");\n";
+ code_ << "}\n";
+ }
+
void GenerateAOTDescriptor() {
- const std::string run_func = ::tvm::runtime::symbol::tvm_run_func_suffix;
- const std::string run_func_mangled = runtime::get_name_mangled(metadata_->mod_name, run_func);
+ const std::string run_func_suffix = ::tvm::runtime::symbol::tvm_run_func_suffix;
+ const std::string tvm_entrypoint_suffix = ::tvm::runtime::symbol::tvm_entrypoint_suffix;
+ const std::string run_func_mangled =
+ runtime::get_name_mangled(metadata_->mod_name, run_func_suffix);
+ const std::string entrypoint_mangled =
+ runtime::get_name_mangled(metadata_->mod_name, tvm_entrypoint_suffix);
const std::string network_mangled = runtime::get_name_mangled(metadata_->mod_name, "network");
- code_ << "#include \"tvm/runtime/crt/internal/aot_executor/aot_executor.h\"\n";
+ auto unpacked_api = target_->GetAttr<Bool>("unpacked-api").value_or(Bool(false));
+ auto interface_api = target_->GetAttr<String>("interface-api").value_or(String("packed"));
+
code_ << "#include \"tvm/runtime/c_runtime_api.h\"\n";
code_ << "#ifdef __cplusplus\n";
- code_ << "extern \"C\"\n";
+ code_ << "extern \"C\" {\n";
code_ << "#endif\n";
- if (target_->GetAttr<Bool>("unpacked-api").value_or(Bool(false))) {
- GenerateEntrypointForUnpackedAPI(run_func_mangled);
+
+ if (unpacked_api) {
+ if (interface_api == "c") {
+ GenerateCInterfaceEntrypoint(entrypoint_mangled, run_func_mangled, metadata_->mod_name);
+ } else {
+ GenerateEntrypointForUnpackedAPI(entrypoint_mangled, run_func_mangled);
+ }
} else {
- GenerateEntrypointForPackedAPI(run_func_mangled);
+ ICHECK_EQ(interface_api, "packed") << "Packed interface required for packed operators";
Review comment:
technically we could support packed API with c interface, correct? we'd just change the input/output tensor types to be TVMValue.
##########
File path: python/tvm/micro/model_library_format.py
##########
@@ -207,6 +208,42 @@ def _build_function_memory_map(function_metadata):
return ret
+def _get_main_relay_func(mod: executor_factory.ExecutorFactoryModule):
+ main_func = mod.function_metadata[MAIN_FUNC_NAME_STR]
+ target = list(main_func.relay_primfuncs.keys())[0]
+ return main_func.relay_primfuncs[target]
+
+
+def _convert_tuple_to_outputs(ret_type, offset=0):
+ outputs = []
+ added_fields = len(ret_type.fields)
+ for output_index in range(added_fields):
+ next_output = offset + len(outputs)
+ if isinstance(ret_type.fields[output_index], TupleType):
+ outputs.extend(_convert_tuple_to_outputs(ret_type.fields[output_index], next_output))
+ else:
+ outputs.append(f"output{next_output}")
+ return outputs
+
+
+def _get_inputs_and_outputs_from_module(mod):
+ main_func = _get_main_relay_func(mod)
+ inputs = [argument.name_hint for argument in main_func.params]
+
+ outputs = ["output"]
+ if isinstance(main_func.ret_type, TupleType):
+ outputs = _convert_tuple_to_outputs(main_func.ret_type)
+
+ return inputs, outputs
+
+
+def _should_generate_interface_header(mod):
+ for _, target in mod.target.items():
Review comment:
i think you can simplify to:
`return any(t.attr.get("interface-api") == "c" for t in mod.target.values())`
--
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] areusch commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-891439644
looking
--
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 pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-886574739
@Raghav-Chakravarthy, as far as I know, this meets the naming requirements for the generated outputs from TVM rather than the runtime portion which typically has the naming conventions you're referring to? @areusch am I correct here?
--
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] areusch merged pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #8280:
URL: https://github.com/apache/tvm/pull/8280
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r674765137
##########
File path: python/tvm/micro/model_library_format.py
##########
@@ -207,6 +208,42 @@ def _build_function_memory_map(function_metadata):
return ret
+def _get_main_relay_func(mod: executor_factory.ExecutorFactoryModule):
+ main_func = mod.function_metadata[MAIN_FUNC_NAME_STR]
+ target = list(main_func.relay_primfuncs.keys())[0]
+ return main_func.relay_primfuncs[target]
+
+
+def _convert_tuple_to_outputs(ret_type, offset=0):
+ outputs = []
+ added_fields = len(ret_type.fields)
+ for output_index in range(added_fields):
+ next_output = offset + len(outputs)
+ if isinstance(ret_type.fields[output_index], TupleType):
+ outputs.extend(_convert_tuple_to_outputs(ret_type.fields[output_index], next_output))
+ else:
+ outputs.append(f"output{next_output}")
+ return outputs
+
+
+def _get_inputs_and_outputs_from_module(mod):
+ main_func = _get_main_relay_func(mod)
+ inputs = [argument.name_hint for argument in main_func.params]
+
+ outputs = ["output"]
+ if isinstance(main_func.ret_type, TupleType):
+ outputs = _convert_tuple_to_outputs(main_func.ret_type)
+
+ return inputs, outputs
+
+
+def _should_generate_interface_header(mod):
+ for _, target in mod.target.items():
Review comment:
Fantastic, I'll give it a go :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] tqchen commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-891445860
Perhaps was due to merge conflict with another 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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r658846046
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,62 @@
+# 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.
+
+import os
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ metadata_header = os.path.join(output_path, f"tvm_{model_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct tvm_{model_name}_inputs {{\n")
+ for input_name in inputs:
+ header_file.write(f"\tvoid* {input_name};\n")
Review comment:
I think the holy war can be postponed given we have clang-format and rules predefined :smile_cat: I will update to use spaces.
##########
File path: src/target/source/source_module.cc
##########
@@ -193,24 +193,24 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
}
void GenerateEntrypointForUnpackedAPI() {
- code_ << "TVM_DLL int32_t " << ::tvm::runtime::symbol::tvm_run_func_prefix << "(";
- int total_args = (metadata_->num_inputs + metadata_->num_outputs);
- for (int i = 0; i < total_args; ++i) {
- code_ << "arg" << i;
+ code_ << "TVM_DLL int " << ::tvm::runtime::symbol::tvm_run_func_prefix << "(";
Review comment:
Ah, good catch, will fix :vulcan_salute:
--
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 pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-888942599
> @Mousius the problem is we have precedent for CamelCase in our C code--i'm not personally opposed to what you say, but i think the change should be a blanket change done in a separate PR and announced with RFC + style guide update.
Oh dear, I see a different conflict here with https://discuss.tvm.apache.org/t/mini-rfc-name-mangling-in-aot/9907 which began using the `tvmgen_` prefixes and C-style signatures? I've used this machinery in this PR following the example set by the existing AOT model descriptor and structures. If we're going to change that to `TVMGen_` we should do that outside this PR also?
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r674764233
##########
File path: include/tvm/runtime/module.h
##########
@@ -232,6 +232,8 @@ constexpr const char* tvm_param_prefix = "__tvm_param__";
constexpr const char* tvm_lookup_linked_param = "_lookup_linked_param";
/*! \brief The main AOT executor function */
constexpr const char* tvm_run_func_suffix = "run_model";
+/*! \brief The models entrypoint function which calls the executor */
Review comment:
I've tried to clarify this in the comments to expand upon why we have both as well.
--
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] guberti commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
guberti commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r678664916
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
+ for input_name in inputs:
+ header_file.write(f" void* {input_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "output tensors")
+ header_file.write(f"struct {mangled_name}_outputs {{\n")
+ for output_name in outputs:
+ header_file.write(f" void* {output_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "memory blocks")
+ header_file.write(f"struct {mangled_name}_memory {{\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "device configurations")
+ header_file.write(f"struct {mangled_name}_devices {{\n")
+ header_file.write("};\n\n")
+
+ header_file.write("/*!\n")
Review comment:
nit: can we do this instead?
```python
header_file.write(
"/*!\n"
f" * \\brief TVM {model_name} model run function \n"
" * \\param inputs Input tensors for the model \n"
" * \\param outputs Output tensors for the model \n"
" * \\param memory Memory blocks for the model to use \n"
" * \\param devices Devices for the model to use \n"
" */\n"
f"int {mangled_name}_run(\n"
f" struct {mangled_name}_inputs* inputs,\n"
f" struct {mangled_name}_outputs* outputs,\n"
f" struct {mangled_name}_memory* memory,\n"
f" struct {mangled_name}_devices* devices\n"
");\n"
)
```
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
Review comment:
Agreed! I've been working on adding Arduino support to microTVM (#8493), and having the input and output dimensions and dtypes in the MLF metadata would allow TVM-generated Arduino code to do type checking.
--
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] areusch commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r654521901
##########
File path: src/target/source/source_module.cc
##########
@@ -193,24 +193,24 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
}
void GenerateEntrypointForUnpackedAPI() {
- code_ << "TVM_DLL int32_t " << ::tvm::runtime::symbol::tvm_run_func_prefix << "(";
- int total_args = (metadata_->num_inputs + metadata_->num_outputs);
- for (int i = 0; i < total_args; ++i) {
- code_ << "arg" << i;
+ code_ << "TVM_DLL int " << ::tvm::runtime::symbol::tvm_run_func_prefix << "(";
Review comment:
since `TVMBackendPackedCFunc` use `int32_t`, i'd suggest we keep it here as well
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,62 @@
+# 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.
+
+import os
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ metadata_header = os.path.join(output_path, f"tvm_{model_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct tvm_{model_name}_inputs {{\n")
+ for input_name in inputs:
+ header_file.write(f"\tvoid* {input_name};\n")
Review comment:
i know this is a holy war but i'd prefer to use spaces when generating code. in this case, i think the argument of "it always renders identically on every machine" is particularly relevant, since it's generated code
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r678611615
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
+ for input_name in inputs:
+ header_file.write(f" void* {input_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "output tensors")
+ header_file.write(f"struct {mangled_name}_outputs {{\n")
+ for output_name in outputs:
+ header_file.write(f" void* {output_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "memory blocks")
+ header_file.write(f"struct {mangled_name}_memory {{\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "device configurations")
Review comment:
per the discuss forum--i think we should leave out devices and introduce it when we have a use case ready to merge. i don't want to get it wrong on first try.
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
Review comment:
could you add struct-level \brief documentation here?
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
Review comment:
perhaps we could favor something explicitly identifying the model_name, since that may not make sense to the user:
```
\\brief {description} for TVM module "{module_name}"
```
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
Review comment:
let's not implicitly rename model_name == module_name inside this interface; either refer to it as module_name throughout or do it in one place e.g. at the top level. i guess if we do the latter, that would suggest to rename mangle_module_name to something more specific to codegen e.g. mangle_c_function_name
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
Review comment:
not required for this PR, but i'd like to add this information as MLF metadata in either `metadata.json` or e.g. `executor-config/aot/metadata.json` cc @guberti
--
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] Raghav-Chakravarthy commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Raghav-Chakravarthy commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r675064775
##########
File path: apps/microtvm/zephyr/aot_demo/src/main.c
##########
@@ -194,18 +195,18 @@ void main(void) {
}
TVMLogf("Zephyr AOT Runtime\n");
- void* inputs[1] = {
- input_data,
+ struct tvmgen_default_inputs inputs = {
+ .input_1 = input_data,
};
- void* outputs[1] = {
- output_data,
+ struct tvmgen_default_outputs outputs = {
Review comment:
TvmGenDefaultOutputs
##########
File path: apps/microtvm/zephyr/aot_demo/src/main.c
##########
@@ -194,18 +195,18 @@ void main(void) {
}
TVMLogf("Zephyr AOT Runtime\n");
- void* inputs[1] = {
- input_data,
+ struct tvmgen_default_inputs inputs = {
Review comment:
TvmGenDefaultInputs
--
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 pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-890858161
@areusch, I think I've covered most of the comments here - do you have any further thoughts on the naming conventions? I'd propose we rectify this outside of this PR if possible.
@guberti, could you please provide an approval if you agree with the changes here?
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r674766363
##########
File path: src/target/source/source_module.cc
##########
@@ -220,37 +221,85 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
code_ << "}\n";
}
- void GenerateEntrypointForPackedAPI(const std::string& run_func) {
+ void GenerateEntrypointForPackedAPI(const std::string& entrypoint_name,
+ const std::string& run_func) {
code_ << "TVM_DLL int32_t " << run_func;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle);\n";
- code_ << "static int32_t " << ::tvm::runtime::symbol::tvm_module_main;
+ code_ << "int32_t " << entrypoint_name;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle) {\n";
code_ << "return " << run_func;
code_ << "(args, type_code, num_args, out_value, out_type_code, resource_handle);\n";
code_ << "}\n";
}
+ void GenerateCInterfaceEntrypoint(const std::string& entrypoint_name, const std::string& run_func,
+ const std::string& mod_name) {
+ code_ << "#include <" << mod_name << ".h>\n";
Review comment:
Yip, it all resolves to a `tvmgen_` prefix, you can see this in the AOT demo:
https://github.com/apache/tvm/pull/8280/files#diff-c552567d634f74cdf47534249b99f69d1cd99872b7af3b3773b9ccfbcaf13569R209
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r674763964
##########
File path: include/tvm/runtime/module.h
##########
@@ -232,6 +232,8 @@ constexpr const char* tvm_param_prefix = "__tvm_param__";
constexpr const char* tvm_lookup_linked_param = "_lookup_linked_param";
/*! \brief The main AOT executor function */
constexpr const char* tvm_run_func_suffix = "run_model";
Review comment:
Unfortunately yes, one is generated from TIR and one in the metadata module to allow us to generate the structures.
--
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] tqchen commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-891445809
E TypeError: compile_and_run() missing 1 required positional argument: 'use_calculated_workspaces'
--
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] areusch commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-888554759
@Mousius the problem is we have precedent for CamelCase in our C code--i'm not personally opposed to what you say, but i think the change should be a blanket change done in a separate PR and announced with RFC + style guide update.
--
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 pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-884970210
@areusch apologies for letting this lapse, I've resolved your comments hopefully :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] areusch commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-888584487
cc @guberti would you like to review this PR as well, since you just tried to consume the previous AOT output?
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r678959973
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
+ for input_name in inputs:
+ header_file.write(f" void* {input_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "output tensors")
+ header_file.write(f"struct {mangled_name}_outputs {{\n")
+ for output_name in outputs:
+ header_file.write(f" void* {output_name};\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "memory blocks")
+ header_file.write(f"struct {mangled_name}_memory {{\n")
+ header_file.write("};\n\n")
+
+ _emit_brief(header_file, model_name, "device configurations")
+ header_file.write(f"struct {mangled_name}_devices {{\n")
+ header_file.write("};\n\n")
+
+ header_file.write("/*!\n")
Review comment:
Sure :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] areusch commented on a change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r681315297
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
Review comment:
since memory and devices was removed, i think what you have is fine now. it's pretty hard to confuse "inputs" and "outputs," since they are core concepts of the model (perhaps parameters may be confusing--are they inputs?). when we add "memory" and "devices," i'd like us to be explicit about which memory is intended to be there, where the user might expect to configure this in TVM, and which memory is not going to be there. likewise with devices--what should be expected, where should it get configured, and what should a user fill in here? but ok to merge now.
--
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] tqchen commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-891436032
@Mousius @areusch this PR seems to lead to a test problem in the mainline, likely due to possible concurrent merge of https://github.com/apache/tvm/pull/8580 or another PR. would be great if we can send in a fix
--
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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r674767361
##########
File path: src/target/source/source_module.cc
##########
@@ -220,37 +221,85 @@ class CSourceCrtMetadataModuleNode : public runtime::ModuleNode {
code_ << "}\n";
}
- void GenerateEntrypointForPackedAPI(const std::string& run_func) {
+ void GenerateEntrypointForPackedAPI(const std::string& entrypoint_name,
+ const std::string& run_func) {
code_ << "TVM_DLL int32_t " << run_func;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle);\n";
- code_ << "static int32_t " << ::tvm::runtime::symbol::tvm_module_main;
+ code_ << "int32_t " << entrypoint_name;
code_ << "(void* args, void* type_code, int num_args, void* out_value, void* "
"out_type_code, void* resource_handle) {\n";
code_ << "return " << run_func;
code_ << "(args, type_code, num_args, out_value, out_type_code, resource_handle);\n";
code_ << "}\n";
}
+ void GenerateCInterfaceEntrypoint(const std::string& entrypoint_name, const std::string& run_func,
+ const std::string& mod_name) {
+ code_ << "#include <" << mod_name << ".h>\n";
+ code_ << "TVM_DLL int32_t " << run_func << "(";
+ unsigned int total_args = (metadata_->inputs.size() + metadata_->num_outputs);
+ for (unsigned int i = 0; i < total_args; ++i) {
+ code_ << "void* arg" << i;
+ if (i + 1 != total_args) {
+ code_ << ",";
+ }
+ }
+ code_ << ");\n";
+ code_ << "int32_t " << entrypoint_name << "(";
+ code_ << "struct " << runtime::get_name_mangled(mod_name, "inputs") << "* inputs,"
+ << "struct " << runtime::get_name_mangled(mod_name, "outputs") << "* outputs,"
+ << "struct " << runtime::get_name_mangled(mod_name, "memory") << "* memory,"
+ << "struct " << runtime::get_name_mangled(mod_name, "devices") << "* devices"
+ << ") {";
+ code_ << "return " << run_func << "(";
+ for (const auto& input : metadata_->inputs) {
+ code_ << "inputs->" << input->name_hint() << ",";
+ }
+ if (metadata_->num_outputs == 1) {
+ code_ << "outputs->output";
+ } else {
+ for (int i = 0; i < metadata_->num_outputs; ++i) {
+ code_ << "outputs->output" << i;
+ if (i + 1 != metadata_->num_outputs) {
+ code_ << ",";
+ }
+ }
+ }
+ code_ << ");\n";
+ code_ << "}\n";
+ }
+
void GenerateAOTDescriptor() {
- const std::string run_func = ::tvm::runtime::symbol::tvm_run_func_suffix;
- const std::string run_func_mangled = runtime::get_name_mangled(metadata_->mod_name, run_func);
+ const std::string run_func_suffix = ::tvm::runtime::symbol::tvm_run_func_suffix;
+ const std::string tvm_entrypoint_suffix = ::tvm::runtime::symbol::tvm_entrypoint_suffix;
+ const std::string run_func_mangled =
+ runtime::get_name_mangled(metadata_->mod_name, run_func_suffix);
+ const std::string entrypoint_mangled =
+ runtime::get_name_mangled(metadata_->mod_name, tvm_entrypoint_suffix);
const std::string network_mangled = runtime::get_name_mangled(metadata_->mod_name, "network");
- code_ << "#include \"tvm/runtime/crt/internal/aot_executor/aot_executor.h\"\n";
+ auto unpacked_api = target_->GetAttr<Bool>("unpacked-api").value_or(Bool(false));
+ auto interface_api = target_->GetAttr<String>("interface-api").value_or(String("packed"));
+
code_ << "#include \"tvm/runtime/c_runtime_api.h\"\n";
code_ << "#ifdef __cplusplus\n";
- code_ << "extern \"C\"\n";
+ code_ << "extern \"C\" {\n";
code_ << "#endif\n";
- if (target_->GetAttr<Bool>("unpacked-api").value_or(Bool(false))) {
- GenerateEntrypointForUnpackedAPI(run_func_mangled);
+
+ if (unpacked_api) {
+ if (interface_api == "c") {
+ GenerateCInterfaceEntrypoint(entrypoint_mangled, run_func_mangled, metadata_->mod_name);
+ } else {
+ GenerateEntrypointForUnpackedAPI(entrypoint_mangled, run_func_mangled);
+ }
} else {
- GenerateEntrypointForPackedAPI(run_func_mangled);
+ ICHECK_EQ(interface_api, "packed") << "Packed interface required for packed operators";
Review comment:
Technically yes, I can imagine a future patch to add support for this.
--
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] areusch commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-891389590
thanks @Mousius, I agree we need to resolve the underscore naming in a few places. my bad for overlooking this earlier. Let's do this in a follow-on PR to avoid bloating this one.
--
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] areusch commented on pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#issuecomment-887083612
@Mousius hmm, I'm not sure we have naming requirements specific to the codegen'd artifacts. @Raghav-Chakravarthy does raise a point--the Google C++ style guide specifies CamelCase for struct names. I'm kind of inclined to follow the style guide here, since we don't have any thing carved out right now for codegen. 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 change in pull request #8280: Introduce --interface-api={c,packed} parameter
Posted by GitBox <gi...@apache.org>.
Mousius commented on a change in pull request #8280:
URL: https://github.com/apache/tvm/pull/8280#discussion_r678954350
##########
File path: python/tvm/micro/interface_api.py
##########
@@ -0,0 +1,81 @@
+# 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.
+
+"""Defines functions for generating a C interface header"""
+
+import os
+
+from tvm.relay.backend.utils import mangle_module_name
+
+
+def _emit_brief(header_file, model_name, description):
+ header_file.write("/*!\n")
+ header_file.write(f" * \\brief TVM {model_name} model {description} \n")
+ header_file.write(" */\n")
+
+
+def generate_c_interface_header(model_name, inputs, outputs, output_path):
+ """Generates a C interface header for a given models inputs and outputs
+
+ Parameters
+ ----------
+ model_name : str
+ Name of the model to be used in defining structs and naming the header
+ inputs : list[str]
+ List of model input names to be placed in generated structs
+ outputs : list[str]
+ List of model output names to be placed in generated structs
+ output_path : str
+ Path to the output folder to generate the header into
+ """
+
+ mangled_name = mangle_module_name(model_name)
+ metadata_header = os.path.join(output_path, f"{mangled_name}.h")
+ with open(metadata_header, "w") as header_file:
+ _emit_brief(header_file, model_name, "input tensors")
+ header_file.write(f"struct {mangled_name}_inputs {{\n")
Review comment:
I've added an example of the emitted header to the PR, the `_emit_brief` function generates a top level comment for each structure - is this what you were getting at?
--
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