yaoyaoding commented on code in PR #300:
URL: https://github.com/apache/tvm-ffi/pull/300#discussion_r2640256499
##########
include/tvm/ffi/extra/cuda/unify_api.h:
##########
@@ -0,0 +1,183 @@
+/*
Review Comment:
> I suggest keeping a single internal compatibility header (perhaps renamed
to _unify_api.h to reflect its internal use). For APIs like CUDADeviceGuard
that are beyond the current PR’s scope, we can temporarily mark them as “not
implemented” in _unify_api.h and throw an error when driver API is requested.
Sounds good to me as long as the compatibility layer is not exposed to the
user as some public API (at least for now). I asked Gemini on the best parctice
of indicating "internal-only" header and it suggests us to use some "detail"
subfolder and define the internal functions in "details" namespace. Maybe we
can create a "detail" subfolder like `/include/tvm/ffi/extra/cuda/detail` and
define the compatibility layer in a "details" namespace.
Another minor suggestion is to avoid
```c++
#ifdef USE_DRIVERAPI
#define some_func driver_func
#else
#define some_func runtime_func
#endif
```
instead, using the following way
```c++
ret_type some_func(...) {
""" what this function does in the "unify" interface. """
#ifdef USE_DRIVERAPI
driver_func(...)
#else
runtime_func(...)
#endif
}
```
> Currently, we have CUDADeviceGuard, which works as aten::DeviceGuard -
this could also be implemented using the driver API. On the other hand, if
CUDADeviceGuard only works with the runtime API while cubin_launcher can switch
between runtime/driver APIs, it introduces some inconsistency.
The `CUDADeviceGurad` is orthogonal to cubin launcher. We can leave the
support of `CUDADeviceGurad` with driver api in the future when needed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
