gemini-code-assist[bot] commented on code in PR #179:
URL: https://github.com/apache/tvm-ffi/pull/179#discussion_r2445847006
##########
python/tvm_ffi/cython/function.pxi:
##########
@@ -324,6 +324,19 @@ cdef int TVMFFIPyArgSetterDevice_(
out.v_device = (<Device>arg).cdevice
return 0
+cdef int TVMFFIPyArgSetterDLPackDeviceProtocol_(
+ TVMFFIPyArgSetter* handle, TVMFFIPyCallContext* ctx,
+ PyObject* py_arg, TVMFFIAny* out
+) except -1:
+ """Setter for dlpack device protocol"""
+ cdef object arg = <object>py_arg
+ cdef tuple dlpack_device = arg.__dlpack_device__()
+ out.type_index = kTVMFFIDevice
+ out.v_device = TVMFFIDLDeviceFromIntPair(
+ <long long>dlpack_device[0],
+ <long long>dlpack_device[1]
Review Comment:
The `TVMFFIDLDeviceFromIntPair` function expects `int32_t` arguments, but
the values from the `dlpack_device` tuple are cast to `long long`. This could
lead to silent truncation if the Python integers returned by
`__dlpack_device__` are larger than what `int32_t` can hold. It's safer to cast
to `int32_t` to match the function signature and make the narrowing conversion
explicit.
```
<int32_t>dlpack_device[0],
<int32_t>dlpack_device[1]
```
##########
python/tvm_ffi/cython/function.pxi:
##########
@@ -716,6 +729,11 @@ cdef int TVMFFIPyArgSetterFactory_(PyObject* value,
TVMFFIPyArgSetter* out) exce
# prefer dlpack as it covers all DLDataType struct
out.func = TVMFFIPyArgSetterDLPackDataTypeProtocol_
return 0
+ if hasattr(arg_class, "__dlpack_device__") and not hasattr(arg_class,
"__dlpack__"):
+ # if a class have __dlpack_device__ but not __dlpack__
+ # then it is a DLPack device protocol
Review Comment:
This comment has a grammatical error ("have" instead of "has") and is a bit
verbose. A more concise and grammatically correct comment would improve
readability and maintainability.
```
# An object with `__dlpack_device__` but not `__dlpack__`
# is treated as a DLPack device protocol object.
```
--
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]
