ved1beta commented on issue #578:
URL: https://github.com/apache/tvm-ffi/issues/578#issuecomment-4365295894
Quick follow-up on (2) you're right, I confirmed PyTorch's MPS DLPack
export already puts `id<MTLBuffer>` into `DLTensor.data`:
- The MPS allocator constructs `c10::DataPtr` directly from the
`id<MTLBuffer>` (`MPSAllocator.mm` ), so `storage().data()` is
the buffer object cast to `void*`.
- `toDLPackImpl` then writes that base pointer into `DLTensor.data` and
sets `byte_offset = storage_offset * elemsize` (`DLConvertor.cpp`).
The DLPack spec covers this `data` is allowed to be an opaque handle
("cl_mem in OpenCL, may be opaque on some device types"), but the spec
doesn't *explicitly* call out Metal. Worth a one-line clarification to the
spec to name Metal alongside OpenCL? Could make a quick PR to dlpack if
that'd help future producers.
One small wrinkle: `toDLPackNonOwning` (the path tvm-ffi uses for the fast
`dltensor_from_py_object_no_sync` exchange) doesn't have the MPS
special-case — it stuffs `data_ptr()` (= MTLBuffer + storage_offset*elemsize)
into `data` with `byte_offset=0`, which produces an invalid `id<MTLBuffer>`
for sliced MPS tensors. That's a PyTorch bug I'll file separately, but
flagging in case it changes anything on the tvm-ffi side.
So (2) drops from scope. Back to (1) — happy to dig into the
`AcquireEncoder`/`ReleaseEncoder` design questions whenever you have time.
--
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]
