Skip to content

[UR][Graph] Strengthen in-order command-buffer property #18444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: sycl
Choose a base branch
from
Draft

[UR][Graph] Strengthen in-order command-buffer property #18444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 28 additions & 28 deletions unified-runtime/include/ur_api.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 9 additions & 5 deletions unified-runtime/scripts/core/EXP-COMMAND-BUFFER.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ Command-Buffer Creation
--------------------------------------------------------------------------------

Command-Buffers are tied to a specific ${x}_context_handle_t and
${x}_device_handle_t. ${x}CommandBufferCreateExp optionally takes a descriptor
${x}_device_handle_t. ${x}CommandBufferCreateExp takes a descriptor
to provide additional properties for how the command-buffer should be
constructed. The members defined in ${x}_exp_command_buffer_desc_t are:

* ``isUpdatable``, which should be set to ``true`` to support :ref:`updating
command-buffer commands`.
* ``isInOrder``, which should be set to ``true`` to enable commands enqueued to
a command-buffer to be executed in an in-order fashion where possible.
* ``isInOrder``, which should be set to ``true`` to enforce commands appended
to a command-buffer to be executed in an in-order fashion.
* ``enableProfiling``, which should be set to ``true`` to enable profiling of
the command-buffer.

Expand Down Expand Up @@ -108,8 +108,9 @@ Sync-Points
A sync-point is a value which represents a command inside of a command-buffer
which is returned from command-buffer append function calls. These can be
optionally passed to these functions to define execution dependencies on other
commands within the command-buffer. Sync-points passed to functions may be
ignored if the command-buffer was created in-order.
commands within the command-buffer. Both wait-list and return sync-point
parameters to append functions are ignored if the command-buffer was created
with the in-order property.

Sync-points are unique and valid for use only within the command-buffer they
were obtained from.
Expand Down Expand Up @@ -550,6 +551,9 @@ Changelog
+-----------+-------------------------------------------------------+
| 1.11 | Support native commands. |
+-----------+-------------------------------------------------------+
| 1.12 | Strengthen in-order property such that sync-points |
| | parameters to append APIs are ignored. |
+-----------+-------------------------------------------------------+

Contributors
--------------------------------------------------------------------------------
Expand Down
28 changes: 14 additions & 14 deletions unified-runtime/scripts/core/exp-command-buffer.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ members:
desc: "[in] Commands in a finalized command-buffer can be updated."
- type: $x_bool_t
name: isInOrder
desc: "[in] Commands in a command-buffer may be executed in-order without explicit dependencies."
desc: "[in] Commands in a command-buffer will execute in-order, explicit sync-point dependencies are ignored."
- type: $x_bool_t
name: enableProfiling
desc: "[in] Command-buffer profiling is enabled."
Expand Down Expand Up @@ -375,7 +375,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -440,7 +440,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -502,7 +502,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -569,7 +569,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -628,7 +628,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -687,7 +687,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -761,7 +761,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -835,7 +835,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -909,7 +909,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -971,7 +971,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -1032,7 +1032,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -1095,7 +1095,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: uint32_t
name: numEventsInWaitList
desc: "[in] Size of the event wait list."
Expand Down Expand Up @@ -1166,7 +1166,7 @@ params:
- type: "const $x_exp_command_buffer_sync_point_t*"
name: pSyncPointWaitList
desc: "[in][optional] A list of sync points that this command depends on.
May be ignored if command-buffer is in-order."
Will be ignored if command-buffer is in-order."
- type: "$x_exp_command_buffer_sync_point_t*"
name: pSyncPoint
desc: "[out][optional] Sync point associated with this command."
Expand Down
27 changes: 21 additions & 6 deletions unified-runtime/source/adapters/cuda/command_buffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ ur_result_t commandHandleDestroy(
} // end anonymous namespace

ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable)
ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable,
bool IsInOrder)
: handle_base(), Context(Context), Device(Device), IsUpdatable(IsUpdatable),
CudaGraph{nullptr}, CudaGraphExec{nullptr}, RefCount{1},
NextSyncPoint{0} {
IsInOrder(IsInOrder), CudaGraph{nullptr}, CudaGraphExec{nullptr},
RefCount{1}, NextSyncPoint{0} {
urContextRetain(Context);
}

Expand Down Expand Up @@ -151,11 +152,24 @@ static ur_result_t getNodesFromSyncPoints(
// the event associated with each sync-point
auto SyncPoints = CommandBuffer->SyncPoints;

// If command-buffer is in-order use last node in ordered map, and return
// early as other user passed sync-points will be redundant for scheduling.
if (CommandBuffer->IsInOrder && !SyncPoints.empty()) {
auto LastNode = std::prev(SyncPoints.end());
CuNodesList.push_back(LastNode->second);
return UR_RESULT_SUCCESS;
}

// For each sync-point add associated CUDA graph node to the return list.
for (size_t i = 0; i < NumSyncPointsInWaitList; i++) {
if (auto NodeHandle = SyncPoints.find(SyncPointWaitList[i]);
NodeHandle != SyncPoints.end()) {
CuNodesList.push_back(NodeHandle->second);
auto DepNode = NodeHandle->second;
// Cuda driver API won't let you add duplicates to the dependency list
if (std::find(CuNodesList.begin(), CuNodesList.end(), DepNode) ==
CuNodesList.end()) {
CuNodesList.push_back(DepNode);
}
} else {
return UR_RESULT_ERROR_INVALID_VALUE;
}
Expand Down Expand Up @@ -346,9 +360,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
ur_exp_command_buffer_handle_t *phCommandBuffer) {
const bool IsUpdatable = pCommandBufferDesc->isUpdatable;
const bool IsInOrder = pCommandBufferDesc->isInOrder;
try {
*phCommandBuffer =
new ur_exp_command_buffer_handle_t_(hContext, hDevice, IsUpdatable);
*phCommandBuffer = new ur_exp_command_buffer_handle_t_(
hContext, hDevice, IsUpdatable, IsInOrder);
} catch (const std::bad_alloc &) {
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
} catch (...) {
Expand Down
Loading