Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Using AOT with UMA and TFLITE #12410

Closed
MichaelJKlaiber opened this issue Aug 12, 2022 · 5 comments
Closed

[Bug] Using AOT with UMA and TFLITE #12410

MichaelJKlaiber opened this issue Aug 12, 2022 · 5 comments

Comments

@MichaelJKlaiber
Copy link
Contributor

MichaelJKlaiber commented Aug 12, 2022

Hi everyone,

found a possible issue.
When executing a testcase using UMA and generating the test .c files via aot.py, it seems that the pointers of the offloaded functions are not in the right order.
Can anyone help?

Thanks,
Michael

CC: @cgerum @PaulPalomeroBernardo @manupa-arm @areusch @lhutton1 @d-smirnov @mehrdadh @SebastianBoblest @sezgin1947

Expected behavior

Correct execution of provided NN imported from TFLITE file.

Actual behavior

Segmentation fault, due to mixed up pointers in generated file.
In particular: in default_lib2.c the pointers for placeholder_1, placeholder are mixed up

Environment

Ubuntu 18.04
TVM c3c7c4c

Steps to reproduce

Run tests/python/contrib/test_uma/test_uma_pipeline.py from this branch
https://github.com/MichaelJKlaiber/tvm/tree/issue/uma-tflite

Generated file looks like this:

#include "/home/michael/Documents/code/tvm/apps/uma/_template/conv2dnchw.cc"// tvm target: vanilla_accelerator
#define TVM_EXPORTS
#include "tvm/runtime/c_runtime_api.h"
#include "tvm/runtime/c_backend_api.h"
#include <math.h>
#ifdef __cplusplus
extern "C"
#endif
TVM_DLL int32_t tvmgen_default_vanilla_accelerator_main_0(float* placeholder, float* placeholder_1, float* T_matmul_NT, uint8_t* global_const_workspace_4_var, uint8_t* global_workspace_5_var) {
  for (int32_t i1 = 0; i1 < 10; ++i1) {
    for (int32_t i2 = 0; i2 < 784; ++i2) {
      if (i2 == 0) {
        T_matmul_NT[i1] = 0.000000e+00f;
      }
      T_matmul_NT[i1] = (T_matmul_NT[i1] + (placeholder_1[i2] * placeholder[((i1 * 784) + i2)]));
    }
  }
  return 0;
}

when changing the function signature manually to
TVM_DLL int32_t tvmgen_default_vanilla_accelerator_main_0(float* placeholder_1, float* placeholder, float* T_matmul_NT, uint8_t* global_const_workspace_4_var, uint8_t* global_workspace_5_var)
the test is successful

@MichaelJKlaiber
Copy link
Contributor Author

@areusch
Copy link
Contributor

areusch commented Aug 12, 2022

this looks odd--it's late for today for me to investigate but hopefully we can look into it next week.

@MichaelJKlaiber
Copy link
Contributor Author

Seems the bug is in the lowering step from relay_prim_func to tir_prim_func.

When collecting the input arguments, the order gets messed up. By replacing the function _get_tensors here
https://github.com/MichaelJKlaiber/tvm/blob/issue/uma-tflite/python/tvm/relay/backend/contrib/uma/api/lower.py#L83

by

            return list(te_cached_func.inputs) + list(te_cached_func.outputs)

everything works perfectly fine for all the test case we provided including the tflite import

Does anyone know if it is sufficient to rely on the .inputs parameters from te_cached_func ?

manupak pushed a commit that referenced this issue Aug 22, 2022
There was a flaw in uma_lower (see issue #12410) that lead in some case to a different argument ordering of the cached_func and the Relay function. This results in an incorrect lowering of the primfunc and eventually a wrong result of a run-time error, in some cases.

This commit adds code to correct the described misbehavior and a unit test case to check this end-to-end functionality with a TFLITE model.
@areusch
Copy link
Contributor

areusch commented Aug 29, 2022

@MichaelJKlaiber is this one fixed?

@MichaelJKlaiber
Copy link
Contributor Author

yes, the bugfix is in main

xinetzone pushed a commit to daobook/tvm that referenced this issue Nov 25, 2022
There was a flaw in uma_lower (see issue apache#12410) that lead in some case to a different argument ordering of the cached_func and the Relay function. This results in an incorrect lowering of the primfunc and eventually a wrong result of a run-time error, in some cases.

This commit adds code to correct the described misbehavior and a unit test case to check this end-to-end functionality with a TFLITE model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants