Skip to content

Commit 84a11de

Browse files
committed
[UR][Graph] Strengthen in-order command-buffer property
Actions issue #18330 to enhance the semantics of the UR command-buffer in-order property such that sync-point dependencies will always be ignored.
1 parent b0ce088 commit 84a11de

22 files changed

+891
-229
lines changed

unified-runtime/include/ur_api.h

+28-28
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified-runtime/scripts/core/EXP-COMMAND-BUFFER.rst

+9-5
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ Command-Buffer Creation
5353
--------------------------------------------------------------------------------
5454

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

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

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

114115
Sync-points are unique and valid for use only within the command-buffer they
115116
were obtained from.
@@ -550,6 +551,9 @@ Changelog
550551
+-----------+-------------------------------------------------------+
551552
| 1.11 | Support native commands. |
552553
+-----------+-------------------------------------------------------+
554+
| 1.12 | Strengthen in-order property such that sync-points |
555+
| | parameters to append APIs are ignored. |
556+
+-----------+-------------------------------------------------------+
553557

554558
Contributors
555559
--------------------------------------------------------------------------------

unified-runtime/scripts/core/exp-command-buffer.yml

+14-14
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ members:
143143
desc: "[in] Commands in a finalized command-buffer can be updated."
144144
- type: $x_bool_t
145145
name: isInOrder
146-
desc: "[in] Commands in a command-buffer may be executed in-order without explicit dependencies."
146+
desc: "[in] Commands in a command-buffer will execute in-order, explicit sync-point dependencies are ignored."
147147
- type: $x_bool_t
148148
name: enableProfiling
149149
desc: "[in] Command-buffer profiling is enabled."
@@ -375,7 +375,7 @@ params:
375375
- type: "const $x_exp_command_buffer_sync_point_t*"
376376
name: pSyncPointWaitList
377377
desc: "[in][optional] A list of sync points that this command depends on.
378-
May be ignored if command-buffer is in-order."
378+
Will be ignored if command-buffer is in-order."
379379
- type: uint32_t
380380
name: numEventsInWaitList
381381
desc: "[in] Size of the event wait list."
@@ -440,7 +440,7 @@ params:
440440
- type: "const $x_exp_command_buffer_sync_point_t*"
441441
name: pSyncPointWaitList
442442
desc: "[in][optional] A list of sync points that this command depends on.
443-
May be ignored if command-buffer is in-order."
443+
Will be ignored if command-buffer is in-order."
444444
- type: uint32_t
445445
name: numEventsInWaitList
446446
desc: "[in] Size of the event wait list."
@@ -502,7 +502,7 @@ params:
502502
- type: "const $x_exp_command_buffer_sync_point_t*"
503503
name: pSyncPointWaitList
504504
desc: "[in][optional] A list of sync points that this command depends on.
505-
May be ignored if command-buffer is in-order."
505+
Will be ignored if command-buffer is in-order."
506506
- type: uint32_t
507507
name: numEventsInWaitList
508508
desc: "[in] Size of the event wait list."
@@ -569,7 +569,7 @@ params:
569569
- type: "const $x_exp_command_buffer_sync_point_t*"
570570
name: pSyncPointWaitList
571571
desc: "[in][optional] A list of sync points that this command depends on.
572-
May be ignored if command-buffer is in-order."
572+
Will be ignored if command-buffer is in-order."
573573
- type: uint32_t
574574
name: numEventsInWaitList
575575
desc: "[in] Size of the event wait list."
@@ -628,7 +628,7 @@ params:
628628
- type: "const $x_exp_command_buffer_sync_point_t*"
629629
name: pSyncPointWaitList
630630
desc: "[in][optional] A list of sync points that this command depends on.
631-
May be ignored if command-buffer is in-order."
631+
Will be ignored if command-buffer is in-order."
632632
- type: uint32_t
633633
name: numEventsInWaitList
634634
desc: "[in] Size of the event wait list."
@@ -687,7 +687,7 @@ params:
687687
- type: "const $x_exp_command_buffer_sync_point_t*"
688688
name: pSyncPointWaitList
689689
desc: "[in][optional] A list of sync points that this command depends on.
690-
May be ignored if command-buffer is in-order."
690+
Will be ignored if command-buffer is in-order."
691691
- type: uint32_t
692692
name: numEventsInWaitList
693693
desc: "[in] Size of the event wait list."
@@ -761,7 +761,7 @@ params:
761761
- type: "const $x_exp_command_buffer_sync_point_t*"
762762
name: pSyncPointWaitList
763763
desc: "[in][optional] A list of sync points that this command depends on.
764-
May be ignored if command-buffer is in-order."
764+
Will be ignored if command-buffer is in-order."
765765
- type: uint32_t
766766
name: numEventsInWaitList
767767
desc: "[in] Size of the event wait list."
@@ -835,7 +835,7 @@ params:
835835
- type: "const $x_exp_command_buffer_sync_point_t*"
836836
name: pSyncPointWaitList
837837
desc: "[in][optional] A list of sync points that this command depends on.
838-
May be ignored if command-buffer is in-order."
838+
Will be ignored if command-buffer is in-order."
839839
- type: uint32_t
840840
name: numEventsInWaitList
841841
desc: "[in] Size of the event wait list."
@@ -909,7 +909,7 @@ params:
909909
- type: "const $x_exp_command_buffer_sync_point_t*"
910910
name: pSyncPointWaitList
911911
desc: "[in][optional] A list of sync points that this command depends on.
912-
May be ignored if command-buffer is in-order."
912+
Will be ignored if command-buffer is in-order."
913913
- type: uint32_t
914914
name: numEventsInWaitList
915915
desc: "[in] Size of the event wait list."
@@ -971,7 +971,7 @@ params:
971971
- type: "const $x_exp_command_buffer_sync_point_t*"
972972
name: pSyncPointWaitList
973973
desc: "[in][optional] A list of sync points that this command depends on.
974-
May be ignored if command-buffer is in-order."
974+
Will be ignored if command-buffer is in-order."
975975
- type: uint32_t
976976
name: numEventsInWaitList
977977
desc: "[in] Size of the event wait list."
@@ -1032,7 +1032,7 @@ params:
10321032
- type: "const $x_exp_command_buffer_sync_point_t*"
10331033
name: pSyncPointWaitList
10341034
desc: "[in][optional] A list of sync points that this command depends on.
1035-
May be ignored if command-buffer is in-order."
1035+
Will be ignored if command-buffer is in-order."
10361036
- type: uint32_t
10371037
name: numEventsInWaitList
10381038
desc: "[in] Size of the event wait list."
@@ -1095,7 +1095,7 @@ params:
10951095
- type: "const $x_exp_command_buffer_sync_point_t*"
10961096
name: pSyncPointWaitList
10971097
desc: "[in][optional] A list of sync points that this command depends on.
1098-
May be ignored if command-buffer is in-order."
1098+
Will be ignored if command-buffer is in-order."
10991099
- type: uint32_t
11001100
name: numEventsInWaitList
11011101
desc: "[in] Size of the event wait list."
@@ -1166,7 +1166,7 @@ params:
11661166
- type: "const $x_exp_command_buffer_sync_point_t*"
11671167
name: pSyncPointWaitList
11681168
desc: "[in][optional] A list of sync points that this command depends on.
1169-
May be ignored if command-buffer is in-order."
1169+
Will be ignored if command-buffer is in-order."
11701170
- type: "$x_exp_command_buffer_sync_point_t*"
11711171
name: pSyncPoint
11721172
desc: "[out][optional] Sync point associated with this command."

unified-runtime/source/adapters/cuda/command_buffer.cpp

+21-6
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,11 @@ ur_result_t commandHandleDestroy(
5656
} // end anonymous namespace
5757

5858
ur_exp_command_buffer_handle_t_::ur_exp_command_buffer_handle_t_(
59-
ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable)
59+
ur_context_handle_t Context, ur_device_handle_t Device, bool IsUpdatable,
60+
bool IsInOrder)
6061
: handle_base(), Context(Context), Device(Device), IsUpdatable(IsUpdatable),
61-
CudaGraph{nullptr}, CudaGraphExec{nullptr}, RefCount{1},
62-
NextSyncPoint{0} {
62+
IsInOrder(IsInOrder), CudaGraph{nullptr}, CudaGraphExec{nullptr},
63+
RefCount{1}, NextSyncPoint{0} {
6364
urContextRetain(Context);
6465
}
6566

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

155+
// If command-buffer is in-order use last node in ordered map, and return
156+
// early as other user passed sync-points will be redundant for scheduling.
157+
if (CommandBuffer->IsInOrder && !SyncPoints.empty()) {
158+
auto LastNode = std::prev(SyncPoints.end());
159+
CuNodesList.push_back(LastNode->second);
160+
return UR_RESULT_SUCCESS;
161+
}
162+
154163
// For each sync-point add associated CUDA graph node to the return list.
155164
for (size_t i = 0; i < NumSyncPointsInWaitList; i++) {
156165
if (auto NodeHandle = SyncPoints.find(SyncPointWaitList[i]);
157166
NodeHandle != SyncPoints.end()) {
158-
CuNodesList.push_back(NodeHandle->second);
167+
auto DepNode = NodeHandle->second;
168+
// Cuda driver API won't let you add duplicates to the dependency list
169+
if (std::find(CuNodesList.begin(), CuNodesList.end(), DepNode) ==
170+
CuNodesList.end()) {
171+
CuNodesList.push_back(DepNode);
172+
}
159173
} else {
160174
return UR_RESULT_ERROR_INVALID_VALUE;
161175
}
@@ -346,9 +360,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
346360
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
347361
ur_exp_command_buffer_handle_t *phCommandBuffer) {
348362
const bool IsUpdatable = pCommandBufferDesc->isUpdatable;
363+
const bool IsInOrder = pCommandBufferDesc->isInOrder;
349364
try {
350-
*phCommandBuffer =
351-
new ur_exp_command_buffer_handle_t_(hContext, hDevice, IsUpdatable);
365+
*phCommandBuffer = new ur_exp_command_buffer_handle_t_(
366+
hContext, hDevice, IsUpdatable, IsInOrder);
352367
} catch (const std::bad_alloc &) {
353368
return UR_RESULT_ERROR_OUT_OF_HOST_MEMORY;
354369
} catch (...) {

0 commit comments

Comments
 (0)