Skip to content

Commit 3a85197

Browse files
aarongreigaarongreig
authored
Merge pull request #913 from aarongreig/aaron/eventCallbackFixes
Add extra validation to EventSetCallback and fix execution_info enum.
2 parents 8b19319 + ecd5ffd commit 3a85197

File tree

9 files changed

+62
-57
lines changed

9 files changed

+62
-57
lines changed

‎include/ur.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,10 +2106,10 @@ class ur_event_native_properties_t(Structure):
21062106
###############################################################################
21072107
## @brief Event states for all events.
21082108
class ur_execution_info_v(IntEnum):
2109-
EXECUTION_INFO_COMPLETE = 0 ## Indicates that the event has completed.
2110-
EXECUTION_INFO_RUNNING = 1 ## Indicates that the device has started processing this event.
2111-
EXECUTION_INFO_SUBMITTED = 2 ## Indicates that the event has been submitted by the host to the device.
2112-
EXECUTION_INFO_QUEUED = 3 ## Indicates that the event has been queued, this is the initial state of
2109+
COMPLETE = 0 ## Indicates that the event has completed.
2110+
RUNNING = 1 ## Indicates that the device has started processing this event.
2111+
SUBMITTED = 2 ## Indicates that the event has been submitted by the host to the device.
2112+
QUEUED = 3 ## Indicates that the event has been queued, this is the initial state of
21132113
## events.
21142114

21152115
class ur_execution_info_t(c_int):

‎include/ur_api.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5687,11 +5687,11 @@ urEventCreateWithNativeHandle(
56875687
///////////////////////////////////////////////////////////////////////////////
56885688
/// @brief Event states for all events.
56895689
typedef enum ur_execution_info_t {
5690-
UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE = 0, ///< Indicates that the event has completed.
5691-
UR_EXECUTION_INFO_EXECUTION_INFO_RUNNING = 1, ///< Indicates that the device has started processing this event.
5692-
UR_EXECUTION_INFO_EXECUTION_INFO_SUBMITTED = 2, ///< Indicates that the event has been submitted by the host to the device.
5693-
UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED = 3, ///< Indicates that the event has been queued, this is the initial state of
5694-
///< events.
5690+
UR_EXECUTION_INFO_COMPLETE = 0, ///< Indicates that the event has completed.
5691+
UR_EXECUTION_INFO_RUNNING = 1, ///< Indicates that the device has started processing this event.
5692+
UR_EXECUTION_INFO_SUBMITTED = 2, ///< Indicates that the event has been submitted by the host to the device.
5693+
UR_EXECUTION_INFO_QUEUED = 3, ///< Indicates that the event has been queued, this is the initial state of
5694+
///< events.
56955695
/// @cond
56965696
UR_EXECUTION_INFO_FORCE_UINT32 = 0x7fffffff
56975697
/// @endcond
@@ -5714,6 +5714,8 @@ typedef void (*ur_event_callback_t)(
57145714
/// - The registered callback function will be called when the execution
57155715
/// status of command associated with event changes to an execution status
57165716
/// equal to or past the status specified by command_exec_status.
5717+
/// - `execStatus` must not be `UR_EXECUTION_INFO_QUEUED` as this is the
5718+
/// initial state of all events.
57175719
/// - The application may call this function from simultaneous threads for
57185720
/// the same context.
57195721
/// - The implementation of this function should be thread-safe.
@@ -5726,9 +5728,11 @@ typedef void (*ur_event_callback_t)(
57265728
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
57275729
/// + `NULL == hEvent`
57285730
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
5729-
/// + `::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED < execStatus`
5731+
/// + `::UR_EXECUTION_INFO_QUEUED < execStatus`
57305732
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
57315733
/// + `NULL == pfnNotify`
5734+
/// - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
5735+
/// + `execStatus == UR_EXECUTION_INFO_QUEUED`
57325736
UR_APIEXPORT ur_result_t UR_APICALL
57335737
urEventSetCallback(
57345738
ur_event_handle_t hEvent, ///< [in] handle of the event object

‎scripts/core/event.yml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,13 @@ desc: "Event states for all events."
319319
class: $xEvent
320320
name: $x_execution_info_t
321321
etors:
322-
- name: EXECUTION_INFO_COMPLETE
322+
- name: COMPLETE
323323
desc: "Indicates that the event has completed."
324-
- name: EXECUTION_INFO_RUNNING
324+
- name: RUNNING
325325
desc: "Indicates that the device has started processing this event."
326-
- name: EXECUTION_INFO_SUBMITTED
326+
- name: SUBMITTED
327327
desc: "Indicates that the event has been submitted by the host to the device."
328-
- name: EXECUTION_INFO_QUEUED
328+
- name: QUEUED
329329
desc: "Indicates that the event has been queued, this is the initial state of events."
330330
--- #--------------------------------------------------------------------------
331331
type: fptr_typedef
@@ -351,6 +351,7 @@ decl: static
351351
ordinal: "0"
352352
details:
353353
- "The registered callback function will be called when the execution status of command associated with event changes to an execution status equal to or past the status specified by command_exec_status."
354+
- "`execStatus` must not be `UR_EXECUTION_INFO_QUEUED` as this is the initial state of all events."
354355
- "The application may call this function from simultaneous threads for the same context."
355356
- "The implementation of this function should be thread-safe."
356357
params:
@@ -366,3 +367,6 @@ params:
366367
- type: void*
367368
name: pUserData
368369
desc: "[in][out][optional] pointer to data to be passed to callback."
370+
returns:
371+
- $X_RESULT_ERROR_UNSUPPORTED_ENUMERATION:
372+
- "`execStatus == UR_EXECUTION_INFO_QUEUED`"

‎source/common/ur_params.hpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9649,20 +9649,20 @@ inline std::ostream &operator<<(std::ostream &os,
96499649
enum ur_execution_info_t value) {
96509650
switch (value) {
96519651

9652-
case UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE:
9653-
os << "UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE";
9652+
case UR_EXECUTION_INFO_COMPLETE:
9653+
os << "UR_EXECUTION_INFO_COMPLETE";
96549654
break;
96559655

9656-
case UR_EXECUTION_INFO_EXECUTION_INFO_RUNNING:
9657-
os << "UR_EXECUTION_INFO_EXECUTION_INFO_RUNNING";
9656+
case UR_EXECUTION_INFO_RUNNING:
9657+
os << "UR_EXECUTION_INFO_RUNNING";
96589658
break;
96599659

9660-
case UR_EXECUTION_INFO_EXECUTION_INFO_SUBMITTED:
9661-
os << "UR_EXECUTION_INFO_EXECUTION_INFO_SUBMITTED";
9660+
case UR_EXECUTION_INFO_SUBMITTED:
9661+
os << "UR_EXECUTION_INFO_SUBMITTED";
96629662
break;
96639663

9664-
case UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED:
9665-
os << "UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED";
9664+
case UR_EXECUTION_INFO_QUEUED:
9665+
os << "UR_EXECUTION_INFO_QUEUED";
96669666
break;
96679667
default:
96689668
os << "unknown enumerator";

‎source/loader/layers/validation/ur_valddi.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3888,9 +3888,13 @@ __urdlllocal ur_result_t UR_APICALL urEventSetCallback(
38883888
return UR_RESULT_ERROR_INVALID_NULL_POINTER;
38893889
}
38903890

3891-
if (UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED < execStatus) {
3891+
if (UR_EXECUTION_INFO_QUEUED < execStatus) {
38923892
return UR_RESULT_ERROR_INVALID_ENUMERATION;
38933893
}
3894+
3895+
if (execStatus == UR_EXECUTION_INFO_QUEUED) {
3896+
return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
3897+
}
38943898
}
38953899

38963900
ur_result_t result =

‎source/loader/ur_libapi.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4550,6 +4550,8 @@ ur_result_t UR_APICALL urEventCreateWithNativeHandle(
45504550
/// - The registered callback function will be called when the execution
45514551
/// status of command associated with event changes to an execution status
45524552
/// equal to or past the status specified by command_exec_status.
4553+
/// - `execStatus` must not be `UR_EXECUTION_INFO_QUEUED` as this is the
4554+
/// initial state of all events.
45534555
/// - The application may call this function from simultaneous threads for
45544556
/// the same context.
45554557
/// - The implementation of this function should be thread-safe.
@@ -4562,9 +4564,11 @@ ur_result_t UR_APICALL urEventCreateWithNativeHandle(
45624564
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
45634565
/// + `NULL == hEvent`
45644566
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
4565-
/// + `::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED < execStatus`
4567+
/// + `::UR_EXECUTION_INFO_QUEUED < execStatus`
45664568
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
45674569
/// + `NULL == pfnNotify`
4570+
/// - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
4571+
/// + `execStatus == UR_EXECUTION_INFO_QUEUED`
45684572
ur_result_t UR_APICALL urEventSetCallback(
45694573
ur_event_handle_t hEvent, ///< [in] handle of the event object
45704574
ur_execution_info_t execStatus, ///< [in] execution status of the event

‎source/ur_api.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3841,6 +3841,8 @@ ur_result_t UR_APICALL urEventCreateWithNativeHandle(
38413841
/// - The registered callback function will be called when the execution
38423842
/// status of command associated with event changes to an execution status
38433843
/// equal to or past the status specified by command_exec_status.
3844+
/// - `execStatus` must not be `UR_EXECUTION_INFO_QUEUED` as this is the
3845+
/// initial state of all events.
38443846
/// - The application may call this function from simultaneous threads for
38453847
/// the same context.
38463848
/// - The implementation of this function should be thread-safe.
@@ -3853,9 +3855,11 @@ ur_result_t UR_APICALL urEventCreateWithNativeHandle(
38533855
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
38543856
/// + `NULL == hEvent`
38553857
/// - ::UR_RESULT_ERROR_INVALID_ENUMERATION
3856-
/// + `::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED < execStatus`
3858+
/// + `::UR_EXECUTION_INFO_QUEUED < execStatus`
38573859
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
38583860
/// + `NULL == pfnNotify`
3861+
/// - ::UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION
3862+
/// + `execStatus == UR_EXECUTION_INFO_QUEUED`
38593863
ur_result_t UR_APICALL urEventSetCallback(
38603864
ur_event_handle_t hEvent, ///< [in] handle of the event object
38613865
ur_execution_info_t execStatus, ///< [in] execution status of the event

‎test/conformance/event/urEventSetCallback.cpp

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ TEST_P(urEventSetCallbackTest, Success) {
2525

2626
bool didRun = false;
2727
ASSERT_SUCCESS(urEventSetCallback(
28-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE,
28+
event, ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE,
2929
Callback::callback, &didRun));
3030

3131
ASSERT_SUCCESS(urEventWait(1, &event));
@@ -56,13 +56,13 @@ TEST_P(urEventSetCallbackTest, ValidateParameters) {
5656
CallbackParameters parameters{};
5757

5858
ASSERT_SUCCESS(urEventSetCallback(
59-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE,
59+
event, ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE,
6060
Callback::callback, &parameters));
6161

6262
ASSERT_SUCCESS(urEventWait(1, &event));
6363
ASSERT_SUCCESS(urEventRelease(event));
6464
ASSERT_EQ(event, parameters.event);
65-
ASSERT_EQ(ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE,
65+
ASSERT_EQ(ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE,
6666
parameters.execStatus);
6767
}
6868

@@ -72,7 +72,6 @@ TEST_P(urEventSetCallbackTest, ValidateParameters) {
7272
TEST_P(urEventSetCallbackTest, AllStates) {
7373

7474
struct CallbackStatus {
75-
bool queued = false;
7675
bool submitted = false;
7776
bool running = false;
7877
bool complete = false;
@@ -84,22 +83,15 @@ TEST_P(urEventSetCallbackTest, AllStates) {
8483

8584
auto status = reinterpret_cast<CallbackStatus *>(pUserData);
8685
switch (execStatus) {
87-
case ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED: {
88-
status->queued = true;
89-
break;
90-
}
91-
case ur_execution_info_t::
92-
UR_EXECUTION_INFO_EXECUTION_INFO_SUBMITTED: {
86+
case ur_execution_info_t::UR_EXECUTION_INFO_SUBMITTED: {
9387
status->submitted = true;
9488
break;
9589
}
96-
case ur_execution_info_t::
97-
UR_EXECUTION_INFO_EXECUTION_INFO_RUNNING: {
90+
case ur_execution_info_t::UR_EXECUTION_INFO_RUNNING: {
9891
status->running = true;
9992
break;
10093
}
101-
case ur_execution_info_t::
102-
UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE: {
94+
case ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE: {
10395
status->complete = true;
10496
break;
10597
}
@@ -113,22 +105,18 @@ TEST_P(urEventSetCallbackTest, AllStates) {
113105
CallbackStatus status{};
114106

115107
ASSERT_SUCCESS(urEventSetCallback(
116-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED,
108+
event, ur_execution_info_t::UR_EXECUTION_INFO_SUBMITTED,
117109
Callback::callback, &status));
118110
ASSERT_SUCCESS(urEventSetCallback(
119-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_SUBMITTED,
111+
event, ur_execution_info_t::UR_EXECUTION_INFO_RUNNING,
120112
Callback::callback, &status));
121113
ASSERT_SUCCESS(urEventSetCallback(
122-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_RUNNING,
123-
Callback::callback, &status));
124-
ASSERT_SUCCESS(urEventSetCallback(
125-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE,
114+
event, ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE,
126115
Callback::callback, &status));
127116

128117
ASSERT_SUCCESS(urEventWait(1, &event));
129118
ASSERT_SUCCESS(urEventRelease(event));
130119

131-
ASSERT_TRUE(status.queued);
132120
ASSERT_TRUE(status.submitted);
133121
ASSERT_TRUE(status.running);
134122
ASSERT_TRUE(status.complete);
@@ -155,7 +143,7 @@ TEST_P(urEventSetCallbackTest, EventAlreadyCompleted) {
155143
bool didRun = false;
156144

157145
ASSERT_SUCCESS(urEventSetCallback(
158-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE,
146+
event, ur_execution_info_t::UR_EXECUTION_INFO_COMPLETE,
159147
Callback::callback, &didRun));
160148

161149
ASSERT_SUCCESS(urEventRelease(event));
@@ -170,19 +158,16 @@ using urEventSetCallbackNegativeTest = uur::event::urEventTest;
170158
void emptyCallback(ur_event_handle_t, ur_execution_info_t, void *) {}
171159

172160
TEST_P(urEventSetCallbackNegativeTest, InvalidNullHandleEvent) {
173-
ASSERT_EQ_RESULT(
174-
urEventSetCallback(
175-
nullptr,
176-
ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED,
177-
emptyCallback, nullptr),
178-
UR_RESULT_ERROR_INVALID_NULL_HANDLE);
161+
ASSERT_EQ_RESULT(urEventSetCallback(
162+
nullptr, ur_execution_info_t::UR_EXECUTION_INFO_QUEUED,
163+
emptyCallback, nullptr),
164+
UR_RESULT_ERROR_INVALID_NULL_HANDLE);
179165
}
180166

181167
TEST_P(urEventSetCallbackNegativeTest, InvalidNullPointerCallback) {
182168
ASSERT_EQ_RESULT(
183-
urEventSetCallback(
184-
event, ur_execution_info_t::UR_EXECUTION_INFO_EXECUTION_INFO_QUEUED,
185-
nullptr, nullptr),
169+
urEventSetCallback(event, ur_execution_info_t::UR_EXECUTION_INFO_QUEUED,
170+
nullptr, nullptr),
186171
UR_RESULT_ERROR_INVALID_NULL_POINTER);
187172
}
188173

‎test/conformance/queue/urQueueFinish.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ TEST_P(urQueueFinishTest, Success) {
2525
ur_event_status_t exec_status;
2626
ASSERT_SUCCESS(urEventGetInfo(event, UR_EVENT_INFO_COMMAND_EXECUTION_STATUS,
2727
sizeof(exec_status), &exec_status, nullptr));
28-
ASSERT_EQ(exec_status, UR_EXECUTION_INFO_EXECUTION_INFO_COMPLETE);
28+
ASSERT_EQ(exec_status, UR_EXECUTION_INFO_COMPLETE);
2929
}
3030

3131
TEST_P(urQueueFinishTest, InvalidNullHandleQueue) {

0 commit comments

Comments
 (0)