[GitHub] [tvm] PhilippvK commented on issue #8953: [Bug] [microTVM] Truncated names of fused operators exceed CRT graph executor buffer lengths

[GitBox <gi...@apache.org>]

PhilippvK commented on issue #8953:
URL: https://github.com/apache/tvm/issues/8953#issuecomment-931979845


   @areusch I was going to implement this and I found an inconsistency I want to get rid off first:
   
   Two different constant values  (`80` & `120`) are hardcoded at different locations in the TVM codebase:
   1. `char func_name[120];` in `graph_executor.h`: Makes sense to me
   2. `char key[20], value[120];` in `graph_executor.c`: Used by JSON parser for every field.
   3. `#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80` in `crt_config.h`: Used to allocate enough memory in `graph_executor.c` to store names? However this seems to be unrelated to JSON parsing and rather affects the parsing process of the binary parameters for the model. (The parameter names stored in that `names` array have are much shorter than 80 characters, e.g. `p35`)
   4. `constexpr static size_t kMaxFuncNameLength = 80;` in `te_compiler_cache.cc`: Used to limit the generated function names BEFORE before the `tvmgen_model_name` prefix is added to the string.
   
   It makes sense to me that 1. and 2. match but however the name `TVM_CRT_MAX_STRLEN_FUNCTION_NAME` seems to be misleading as it is not really related to function names.
   
   Please let me know if I am missing anything.
   


-- 
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