Skip to content

Commit

Permalink
Fix inconsistencies in graph_executor function names handling
Browse files Browse the repository at this point in the history
Updates value of `TVM_CRT_MAX_STRLEN_FUNCTION_NAME` from `80` to `120`

Replace all occurences of `[120]` with `[TVM_CRT_MAX_STRLEN_FUNCTION_NAME]`
to maintain consistency and make the array lengths user-configurable.

Introduces `TVM_CRT_MAX_STRLEN_PARAM_NAME` used for parameter names only

Adds comments to `kMaxFuncNameLength` variabe in src/relay/backend/te_compiler_cache.cc
making sure that the values are kept "in sync". (sort of)

See apache#8953 for more context. The actual bug reported there however can
only be fixed by increasing the TVM_CRT_MAX_STRLEN_FUNCTION_NAME to a
value larger than the maximum possible truncated function name length
(including prefixes and suffices)

Example:

  6	['tvmgen' prefix length]
+ 7	['default' model name length]
+ 5	['fused' fused function name prefix length]
+ 80	[truncated function name length]
+ 19	[length of appended hash]
+ 4     [Number of '_' between components]
= 121
  • Loading branch information
PhilippvK committed Oct 14, 2021
1 parent da1f735 commit a970f79
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 16 deletions.
4 changes: 3 additions & 1 deletion apps/bundle_deploy/crt_config/crt_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
#define TVM_CRT_MAX_STRLEN_DLTYPE 10
/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120
/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! Maximum number of registered modules. */
#define TVM_CRT_MAX_REGISTERED_MODULES 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
#define TVM_CRT_MAX_STRLEN_DLTYPE 10
/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120
/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! Maximum number of registered modules. */
#define TVM_CRT_MAX_REGISTERED_MODULES 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
#define TVM_CRT_MAX_STRLEN_DLTYPE 10
/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120
/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! Maximum number of registered modules. */
#define TVM_CRT_MAX_REGISTERED_MODULES 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@
#define TVM_CRT_MAX_STRLEN_DLTYPE 10

/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120

/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! \brief Maximum length of a PackedFunc function name. */
#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
Expand Down
2 changes: 1 addition & 1 deletion include/tvm/runtime/crt/graph_executor.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct TVMModule;

/*! \brief operator attributes about tvm op */
typedef struct TVMOpParam {
char func_name[120];
char func_name[TVM_CRT_MAX_STRLEN_FUNCTION_NAME];
uint32_t num_inputs;
uint32_t num_outputs;
uint32_t flatten_data;
Expand Down
4 changes: 4 additions & 0 deletions src/relay/backend/te_compiler_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class ScheduleBuilder : public backend::MemoizedExprTranslator<Array<te::Tensor>
auto outputs = this->VisitExpr(prim_func->body);
auto candidate_name = readable_name_stream_.str();
constexpr static size_t kMaxFuncNameLength = 80;
// WARNING: Please make sure to also update TVM_CRT_MAX_STRLEN_FUNCTION_NAME
// whenever the value of kMaxFuncNameLength changes
if (candidate_name.size() > kMaxFuncNameLength) {
std::stringstream truncated_name;
truncated_name << candidate_name.substr(0, kMaxFuncNameLength);
Expand Down Expand Up @@ -394,6 +396,8 @@ class MakeShapeFunc : public backend::MemoizedExprTranslator<Array<te::Tensor>>
// Generate a name.
auto candidate_name = readable_name_stream_.str();
constexpr static size_t kMaxFuncNameLength = 80;
// WARNING: Please make sure to also update TVM_CRT_MAX_STRLEN_FUNCTION_NAME
// whenever the value of kMaxFuncNameLength changes
if (candidate_name.size() > kMaxFuncNameLength) {
std::stringstream truncated_name;
truncated_name << candidate_name.substr(0, kMaxFuncNameLength);
Expand Down
5 changes: 4 additions & 1 deletion src/runtime/crt/crt_config-template.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@
#define TVM_CRT_MAX_STRLEN_DLTYPE 10

/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120

/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! \brief Maximum length of a PackedFunc function name. */
#define TVM_CRT_MAX_FUNCTION_NAME_LENGTH_BYTES 30
Expand Down
16 changes: 8 additions & 8 deletions src/runtime/crt/graph_executor/graph_executor.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ int NodeEntry_Load(TVMGraphExecutorNodeEntry* entry, JSONReader* reader) {
void TVMGraphExecutorNode_LoadAttrs(TVMGraphExecutorNode* node, JSONReader* reader,
TVMOpParam* param) {
int bitmask = 0;
char key[20], value[120];
char key[20], value[TVM_CRT_MAX_STRLEN_FUNCTION_NAME];
memset(param, 0, sizeof(TVMOpParam));
memset(key, 0, sizeof(key));
memset(value, 0, sizeof(value));
Expand Down Expand Up @@ -796,13 +796,13 @@ int TVMGraphExecutor_LoadParams(TVMGraphExecutor* executor, const char* param_bl
char* names = NULL;
DLDevice dev = {kDLCPU, 0};
tvm_crt_error_t err = TVMPlatformMemoryAllocate(
TVM_CRT_MAX_STRLEN_FUNCTION_NAME * executor->nodes_count, dev, (void**)&names);
TVM_CRT_MAX_STRLEN_PARAM_NAME * executor->nodes_count, dev, (void**)&names);
if (err != kTvmErrorNoError) {
fprintf(stderr, "memory allocate error: %08x", err);
status = -1;
return status;
}
memset(names, 0, TVM_CRT_MAX_STRLEN_FUNCTION_NAME * executor->nodes_count);
memset(names, 0, TVM_CRT_MAX_STRLEN_PARAM_NAME * executor->nodes_count);
uint64_t names_count;
int idx;
memcpy(&names_count, bptr, sizeof(names_count));
Expand All @@ -811,11 +811,11 @@ int TVMGraphExecutor_LoadParams(TVMGraphExecutor* executor, const char* param_bl
uint64_t name_length;
memcpy(&name_length, bptr, sizeof(name_length));
bptr += sizeof(name_length);
if (name_length >= TVM_CRT_MAX_STRLEN_FUNCTION_NAME) {
if (name_length >= TVM_CRT_MAX_STRLEN_PARAM_NAME) {
fprintf(stderr, "Error: function name longer than expected.\n");
status = -1;
}
memcpy(names + TVM_CRT_MAX_STRLEN_FUNCTION_NAME * idx, bptr, name_length);
memcpy(names + TVM_CRT_MAX_STRLEN_PARAM_NAME * idx, bptr, name_length);
bptr += name_length;
}

Expand All @@ -831,9 +831,9 @@ int TVMGraphExecutor_LoadParams(TVMGraphExecutor* executor, const char* param_bl

for (idx = 0; idx < size; idx++) {
int32_t in_idx =
TVMGraphExecutor_GetInputIndex(executor, names + TVM_CRT_MAX_STRLEN_FUNCTION_NAME * idx);
TVMGraphExecutor_GetInputIndex(executor, names + TVM_CRT_MAX_STRLEN_PARAM_NAME * idx);
CHECK_GT(in_idx, 0, "Found param for non-existent input: %s\n",
names + TVM_CRT_MAX_STRLEN_FUNCTION_NAME * idx);
names + TVM_CRT_MAX_STRLEN_PARAM_NAME * idx);
uint32_t eid = TVMGraphExecutor_GetEntryId(executor, executor->input_nodes[in_idx], 0);
if (!(eid < executor->data_entry_count)) {
fprintf(stderr, "`entry_id`=%d is greater than expected(%d).\n", eid,
Expand All @@ -859,7 +859,7 @@ int TVMGraphExecutor_LoadParams(TVMGraphExecutor* executor, const char* param_bl
#if TVM_CRT_DEBUG
TVMNDArray* entry = &(executor->data_entry[eid]);
printf("loading: param %s loaded, in_idx=%d, eid=%d, ndim=%d, data[0]=%f\n",
names + TVM_CRT_MAX_STRLEN_FUNCTION_NAME * idx, in_idx, eid, entry->dl_tensor.ndim,
names + TVM_CRT_MAX_STRLEN_PARAM_NAME * idx, in_idx, eid, entry->dl_tensor.ndim,
((float*)entry->dl_tensor.data)[0]); // NOLINT(*)
#endif // TVM_CRT_DEBUG
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ typedef struct TVMGraphExecutorNode {
// operator type in string
char op_type[16];
// name of the op
char name[120];
char name[TVM_CRT_MAX_STRLEN_FUNCTION_NAME];
// parameters
TVMOpParam param;
// inputs
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/micro/crt_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
/*! Maximum supported string length in dltype, e.g. "int8", "int16", "float32" */
#define TVM_CRT_MAX_STRLEN_DLTYPE 10
/*! Maximum supported string length in function names */
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 80
#define TVM_CRT_MAX_STRLEN_FUNCTION_NAME 120
/*! Maximum supported string length in parameter names */
#define TVM_CRT_MAX_STRLEN_PARAM_NAME 80

/*! Maximum number of registered modules. */
#define TVM_CRT_MAX_REGISTERED_MODULES 2
Expand Down

0 comments on commit a970f79

Please sign in to comment.