[UR][Offload] Add initial membuffer implementation #18849
Conversation
callumfare
force-pushed
the
offload_mem_buffer
branch
from
65b5ef2 to
d9aaae4
Compare
| #include <OffloadAPI.h> | ||
| #include <unordered_map> | ||
| #include <ur_api.h> | ||
|
|
||
| struct ur_context_handle_t_ : RefCounted { | ||
| ur_context_handle_t_(ur_device_handle_t hDevice) : Device{hDevice} { | ||
| urDeviceRetain(Device); | ||
| // For convenience, store the host device in the context | ||
| OffloadHost = Adapter.HostDevice; |
There was a problem hiding this comment.
Isn't Adapter globally visible and lives for the lifetime of all adapter handles? Why do we need to keep a copy here?
There was a problem hiding this comment.
Yeah this wasn't necessary, fixed now
|
|
||
| // TODO: olMemcpy src should be const | ||
| olMemcpy(hQueue->OffloadQueue, DevPtr + offset, hQueue->OffloadDevice, | ||
| const_cast<void *>(pSrc), hQueue->Context->OffloadHost, size, |
There was a problem hiding this comment.
This makes me a bit uneasy - Could we change olMemcpy to be const void *?
There was a problem hiding this comment.
|
|
||
| void *DevPtr = std::get<BufferMem>(hBuffer->Mem).Ptr; | ||
|
|
||
| olMemcpy(hQueue->OffloadQueue, pDst, hQueue->Context->OffloadHost, |
There was a problem hiding this comment.
Should we not be checking the result of this against OL_SUCCESS?
There was a problem hiding this comment.
Fixed now. I wonder if it would be good to have a OL_CHECK(...) macro or similar to hide the logic of checking the error every time.
There was a problem hiding this comment.
Honestly surprised there isn't one yet. I think it's worth adding, yes.
There was a problem hiding this comment.
Cool, I can do that in a separate PR
Add a basic implementation of buffers to the Offload adapter. The design is loosely based on the CUDA and HIP adapters.
For now contexts only support a single device, so the implementation is relatively simple. When multi-device contexts are supported we will need to handle migration of data between devices (or otherwise change this implementation)