Lunderberg commented on PR #16836:
URL: https://github.com/apache/tvm/pull/16836#issuecomment-2138121297
Making a few notes here for later. The key limitation is that a `PrimFunc`
cannot reliably return a `const char*`, even to a statically-allocated string.
* Even when using the same type code, `TVMArgValue` and `TVMRetValue` can
have different internal representations.
* A `TVMRetValue` with type code `kTVMStr` should have `v_handle` as its
active member, and will be cast to a `std::string*`. The `TVMRetValue` owns
the `std::string`, and must delete it when destructed.
* A `TVMArgValue` with type code `kTVMStr` should have `v_handle` as its
active member, and will be cast to a `const char*`. The `TVMArgValue` does not
own the `const char*`, and must not delete it when destructed.
* No other type code exists that could return a value as a
* Allocating and constructing a `std::string*` depends on the c++ stdlib
implementation. Reproducing this, either in the LLVM codegen or in the C
runtime, would be implementation-dependent and error-prone.
* The workaround in the current implementation calls from TIR to a packed
function, which then constructs the `std::string` instance. This workaround is
not available in AOT environments.
Idea for a design:
1. Update `TVMByteArray` to have two additional members, `free_func` and
`free_func_arg`.
```c++
typedef struct {
// A pointer to the byte array.
const char* data;
// The number of bytes in the array
size_t size;
// If the TVMPODValue_ owns the allocation, a function that can be
called to
// free the allocation. If the TVMPODValue_ does not own the
alllocation, should
// be set to NULL.
void (*free_func)(void*);
// If NULL, then `free_func` is called using `data` as an argument.
If non-NULL
// then `free_func` is called using `free_func_arg` as an argument.
This allows
// additional context about the allocation to be provided, but only
if required.
void* free_func_arg;
} TVMByteArray;
```
2. `TVMArgValue` should still hold a non-owning pointer to the buffer. This
would be stored with `free_func = NULL`.
3. `TVMRetValue` should still hold an owning pointer to the buffer. For
existing FFI calls, this would be stored with `free_func = [](void* ptr) {
delete static_cast<std::string*>(ptr); }` and `free_func_arg = &my_str`.
4. When returning a static string from TIR, this would be stored with
`free_func = NULL`.
Steps 1/2/3 would all be in C++, and would be straightforward to implement
with the existing APIs. Step 4 would allow a PrimFunc to return a non-owning
`const char*` pointer, which should be converted to a python `str` by the FFI,
and which should not have a destructor called.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
