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/05/07 17:47:04 UTC
[GitHub] [tvm] rijulg opened a new pull request #8006: Fix executor for different compilers
rijulg opened a new pull request #8006:
URL: https://github.com/apache/tvm/pull/8006
At the moment compiling this file throws multiple errors with C++ compilers, this change proposes to fix them.
1. `tvm_model_t->run_func` of type `TVMBackedPackedFunc` returns an int at the moment which is different from the signature of this function `tvm_runtime_run`, implicit casting is not favorable in many compile chains and throws errors.
2. The index of iterators were of type `int` while that of `model->num_input_tensors` and `model->num_output_tensors` were of type `uint32_t`, this type difference again throws errors in many toolchains, and can potentially cause incorrect calculations.
3. C Style struct initialization of tensors with `(DLTensor){...}` is not supported in many C++ toolchains and throws “non-trivial designated initializers not supported” error. Explicitly setting values should work in all cases even though it looks a little less nice.
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
--
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] rijulg commented on pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
rijulg commented on pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#issuecomment-835726194
@jcf94 Changed the type and tested with gcc as well as g++
--
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] leandron commented on pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#issuecomment-834662148
cc @giuseros @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jcf94 commented on pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#issuecomment-835134183
> > Looks like this is a `.c` file, and I'm guessing is this designed to be compiled by a C compiler?
>
> Yes, it is probably designed to be compiled with a C compiler, but problems 1 and 2 should be encountered with any compilation process that prohibits implicit typecasting. Additionally the file can readily be used with C++ compiler, so it might be worth changing the structure initialisation to work with C++ compilers.
Yeah, I agree. Besides, we should better use `size_t` instead of the `uint32_t`.
--
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] jcf94 commented on a change in pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
jcf94 commented on a change in pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#discussion_r628708401
##########
File path: src/runtime/crt/aot_executor/aot_executor.c
##########
@@ -37,29 +37,26 @@ tvm_crt_error_t tvm_runtime_run(const tvm_model_t* model, void** inputs, void**
TVMValue tvm_values[model->num_input_tensors + model->num_output_tensors]; // NOLINT
int32_t tvm_typeids[model->num_input_tensors + model->num_output_tensors]; // NOLINT
- for (int i = 0; i < model->num_input_tensors; i++) {
- tensors[i] = (DLTensor){
- .device = fake_device,
- .data = inputs[i],
- .shape = &fake_shape,
- .ndim = fake_dims,
- .byte_offset = 0,
- .strides = NULL,
- };
+ for (uint32_t i = 0; i < model->num_input_tensors; i++) {
Review comment:
```suggestion
for (size_t i = 0; i < model->num_input_tensors; i++) {
```
--
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] jcf94 merged pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
jcf94 merged pull request #8006:
URL: https://github.com/apache/tvm/pull/8006
--
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] jcf94 commented on pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
jcf94 commented on pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#issuecomment-834971786
Looks like this is a `.c` file, and I'm guessing is this designed to be compiled by a C compiler?
--
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] rijulg commented on pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
rijulg commented on pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#issuecomment-834993279
> Looks like this is a `.c` file, and I'm guessing is this designed to be compiled by a C compiler?
Yes, it is probably designed to be compiled with a C compiler, but problems 1 and 2 should be encountered with any compilation process that prohibits implicit typecasting. Additionally the file can readily be used with C++ compiler, so it might be worth changing the structure initialisation to work with C++ compilers.
--
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] rijulg commented on a change in pull request #8006: Fix executor for different compilers
Posted by GitBox <gi...@apache.org>.
rijulg commented on a change in pull request #8006:
URL: https://github.com/apache/tvm/pull/8006#discussion_r628709267
##########
File path: src/runtime/crt/aot_executor/aot_executor.c
##########
@@ -37,29 +37,26 @@ tvm_crt_error_t tvm_runtime_run(const tvm_model_t* model, void** inputs, void**
TVMValue tvm_values[model->num_input_tensors + model->num_output_tensors]; // NOLINT
int32_t tvm_typeids[model->num_input_tensors + model->num_output_tensors]; // NOLINT
- for (int i = 0; i < model->num_input_tensors; i++) {
- tensors[i] = (DLTensor){
- .device = fake_device,
- .data = inputs[i],
- .shape = &fake_shape,
- .ndim = fake_dims,
- .byte_offset = 0,
- .strides = NULL,
- };
+ for (uint32_t i = 0; i < model->num_input_tensors; i++) {
Review comment:
I think this is better but we will have to change the type of `model->num_input_tensors` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org