[GitHub] [incubator-tvm] intheworld commented on pull request #6545: [Rust] change Context.device_id from usize to c_int (#6528)

[GitBox <gi...@apache.org>]

intheworld commented on pull request #6545:
URL: https://github.com/apache/incubator-tvm/pull/6545#issuecomment-698690097


   > @intheworld could you say a little more about why this change is needed? I think the point of Context is that it's a "safer" Rust version of DLContext. As long as you check that `device_id` is positive, the conversion should be fine (typed conversion, not just casting). I'd rather have usize (or u32) on the Rust side, since you don't have to worry about negative device ids
   
   I see the following code in PackedFunc.
   impl_pod_value!(Int, i64, [i8, i16, i32, i64, isize]);
   impl_pod_value!(UInt, i64, [u8, u16, u32, u64, usize]);
   
   usize will be passed in i64, but considered as UInt, which caused type-check of device_id fail. Which I have mentioned in issue #6528. 
   
   In dlpack.h, 
   typedef struct {
     /*! \brief The device type used in the device. */
     DLDeviceType device_type;
     /*! \brief The device index */
     int device_id;
   } DLContext;
   
   My thought is to make the type consistent. 
   


----------------------------------------------------------------
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:
users@infra.apache.org