areusch commented on a change in pull request #6948:
URL: https://github.com/apache/tvm/pull/6948#discussion_r536391342
##########
File path: src/runtime/crt/common/crt_runtime_api.c
##########
@@ -315,21 +315,30 @@ int TVMFuncFree(TVMFunctionHandle func) {
return 0;
}
-tvm_crt_error_t TVMInitializeRuntime(uint8_t* memory_pool, size_t
memory_pool_size_bytes,
- size_t page_size_bytes_log2) {
+tvm_crt_error_t TVMInitializeRuntime() {
int idx;
tvm_crt_error_t error;
+ void* func_registry_memory;
- error =
- TVMInitializeGlobalMemoryManager(memory_pool, memory_pool_size_bytes,
page_size_bytes_log2);
+ system_lib_handle = kTVMModuleHandleUninitialized;
+
+ DLContext ctx = {kDLCPU, 0};
+ error = TVMPlatformMemoryAllocate(TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES,
ctx,
+ &func_registry_memory);
if (error != kTvmErrorNoError) {
return error;
}
system_lib_handle = kTVMModuleHandleUninitialized;
- TVMMutableFuncRegistry_Create(&global_func_registry,
-
vmalloc(TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES),
+ void* registry_backing_memory;
+ error = TVMPlatformMemoryAllocate(TVM_CRT_GLOBAL_FUNC_REGISTRY_SIZE_BYTES,
ctx,
+ ®istry_backing_memory);
+ if (error != kTvmErrorNoError) {
+ return error;
Review comment:
ah this is a great point @liangfu! i've split this function into two
parts:
1. allocating memory
2. initializing
in the first part, I added TVMPlatformMemoryFree() calls to any early
returns. in the second part, I followed your advice and consolidated the
returns at the end. there, I added calls to free the allocated memory on error.
I also audited the rest of the codebase for calls to
TVMPlatformMemoryAllocate. I found that the rest of the calls could be split
into two categories:
1. calls where a single TVMPlatformMemoryAllocate is performed and no other
errors could be generated before returning the allocated memory. in this case,
the caller takes responsibility for the memory.
2. initialization in the GraphRuntime. If something errors here, we will
definitely leak memory right now. since most examples of this bail if a failure
happens on initialization, and fixing this is quite an invasive change, I'd
propose we defer fixing that to another PR. let me know if you think
differently.
----------------------------------------------------------------
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:
[email protected]
