[hip][cuda] Merged pending_queue_actions implementations. #18220
Conversation
This creates a single deferred work queue that merges the almost identical implementations of pending_queue_actions. This leaves the actual underlying implementation unchanged, and simply moves the work into a shared location. There are some future cleanups that can be made in order to improve this even further. Signed-off-by: Andrew Woloszyn <[email protected]>
AWoloszyn
requested review from
antiagainst,
nithinsubbiah and
benvanik
as code owners
| } iree_hal_deferred_work_queue_device_interface_t; | ||
|
|
||
| typedef struct iree_hal_deferred_work_queue_device_interface_vtable_t { | ||
| void (*destroy)(iree_hal_deferred_work_queue_device_interface_t*); |
There was a problem hiding this comment.
| void (*destroy)(iree_hal_deferred_work_queue_device_interface_t*); | |
| void(IREE_API_PTR* destroy)(iree_hal_deferred_work_queue_device_interface_t*); |
(as in other files)
There was a problem hiding this comment.
(still need the IREE_API_PTR and arg name on destroy)
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
Signed-off-by: Andrew Woloszyn <[email protected]>
| @@ -77,7 +77,7 @@ typedef struct iree_hal_hip_device_t { | |||
| // are met. It buffers submissions and allocations internally before they | |||
| // are ready. This queue couples with HAL semaphores backed by iree_event_t | |||
| // and hipEvent_t objects. | |||
| iree_hal_hip_pending_queue_actions_t* pending_queue_actions; | |||
There was a problem hiding this comment.
this is how it was and not something done in this PR, but something to think about is how we'll extend this for multiple queues (hopefully just making this an array and selecting the right one on the queue operations) - the nice thing about all the logic being wrapped up in the work queue is that we can move anything queue specific that's bled out (hip_dispatch_stream, tracing_context, etc) into the work queue and stamp them out as multiple per physical device or one or more per physical device.
| } iree_hal_deferred_work_queue_device_interface_t; | ||
|
|
||
| typedef struct iree_hal_deferred_work_queue_device_interface_vtable_t { | ||
| void (*destroy)(iree_hal_deferred_work_queue_device_interface_t*); |
There was a problem hiding this comment.
(still need the IREE_API_PTR and arg name on destroy)
| typedef void(IREE_API_PTR* iree_hal_deferred_work_queue_cleanup_callback_t)( | ||
| void* user_data); | ||
|
|
||
| // Enques command buffer submissions into the work queue to be executed |
There was a problem hiding this comment.
| // Enques command buffer submissions into the work queue to be executed | |
| // Enqueues command buffer submissions into the work queue to be executed |
|
|
||
| // Enques command buffer submissions into the work queue to be executed | ||
| // once all semaphores have been satisfied. | ||
| iree_status_t iree_hal_deferred_work_queue_enque( |
There was a problem hiding this comment.
| iree_status_t iree_hal_deferred_work_queue_enque( | |
| iree_status_t iree_hal_deferred_work_queue_enqueue( |
|
ah, didn't realize you had that on auto-submit - please do a follow up with the required fixes |
|
| iree_cc_library( | ||
| NAME | ||
| deferred_work_queue | ||
| HDRS | ||
| "deferred_work_queue.h" | ||
| SRCS | ||
| "deferred_work_queue.c" | ||
| DEPS | ||
| ::deferred_command_buffer | ||
| ::resource_set | ||
| ::semaphore_base | ||
| iree::base | ||
| iree::base::internal::arena | ||
| iree::base::internal::synchronization | ||
| iree::base::internal::threading | ||
| iree::hal | ||
| PUBLIC | ||
| ) |
There was a problem hiding this comment.
Should this be wrapped in:
if (IREE_ENABLE_THREADING)
My builds is now failing as iree::base::internal::threading does not exist, presumably because IREE_ENABLE_THREADING is not set:
iree/runtime/src/iree/base/internal/CMakeLists.txt
Lines 383 to 406 in b6602e8
This creates a single deferred work queue that merges the almost identical implementations of pending_queue_actions.
This leaves the actual underlying implementation unchanged, and simply moves the work into a shared location. There are some future cleanups that can be made in order to improve this even further.