Skip to content

Commit 830a6c9

Browse files
committed
Make double finalizing fail
A command buffer should fail if finalize is called multiple times on the same command buffer. This matches openCL behaviour.
1 parent 098deca commit 830a6c9

13 files changed

+27
-2
lines changed

include/ur_api.h

+1
Original file line numberDiff line numberDiff line change
@@ -8517,6 +8517,7 @@ urCommandBufferReleaseExp(
85178517
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
85188518
/// + `NULL == hCommandBuffer`
85198519
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP
8520+
/// - ::UR_RESULT_ERROR_INVALID_OPERATION - "If `hCommandBuffer` has already been finalized"
85208521
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
85218522
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
85228523
UR_APIEXPORT ur_result_t UR_APICALL

scripts/core/exp-command-buffer.yml

+2
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,8 @@ params:
330330
desc: "[in] Handle of the command-buffer object."
331331
returns:
332332
- $X_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP
333+
- $X_RESULT_ERROR_INVALID_OPERATION
334+
- "If `hCommandBuffer` has already been finalized"
333335
- $X_RESULT_ERROR_OUT_OF_HOST_MEMORY
334336
- $X_RESULT_ERROR_OUT_OF_RESOURCES
335337
--- #--------------------------------------------------------------------------

source/adapters/cuda/command_buffer.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
401401

402402
UR_APIEXPORT ur_result_t UR_APICALL
403403
urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
404+
UR_ASSERT(hCommandBuffer->CudaGraphExec == nullptr,
405+
UR_RESULT_ERROR_INVALID_OPERATION);
404406
try {
405407
const unsigned long long flags = 0;
406408
#if CUDA_VERSION >= 12000

source/adapters/cuda/command_buffer.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ struct ur_exp_command_buffer_handle_t_ {
355355
// Cuda Graph handle
356356
CUgraph CudaGraph;
357357
// Cuda Graph Exec handle
358-
CUgraphExec CudaGraphExec;
358+
CUgraphExec CudaGraphExec = nullptr;
359359
// Atomic variable counting the number of reference to this command_buffer
360360
// using std::atomic prevents data race when incrementing/decrementing.
361361
std::atomic_uint32_t RefCountInternal;

source/adapters/hip/command_buffer.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
306306

307307
UR_APIEXPORT ur_result_t UR_APICALL
308308
urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
309+
UR_ASSERT(hCommandBuffer->HIPGraphExec == nullptr,
310+
UR_RESULT_ERROR_INVALID_OPERATION);
309311
try {
310312
const unsigned long long flags = 0;
311313
UR_CHECK_ERROR(hipGraphInstantiateWithFlags(

source/adapters/hip/command_buffer.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ struct ur_exp_command_buffer_handle_t_ {
175175
// HIP Graph handle
176176
hipGraph_t HIPGraph;
177177
// HIP Graph Exec handle
178-
hipGraphExec_t HIPGraphExec;
178+
hipGraphExec_t HIPGraphExec = nullptr;
179179
// Atomic variable counting the number of reference to this command_buffer
180180
// using std::atomic prevents data race when incrementing/decrementing.
181181
std::atomic_uint32_t RefCountInternal;

source/adapters/level_zero/command_buffer.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,7 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t CommandBuffer) {
764764
* enqueueImmediateAppendPath() which uses the
765765
* zeCommandListImmediateAppendCommandListsExp API. */
766766
ur_result_t
767+
<<<<<<< HEAD
767768
finalizeImmediateAppendPath(ur_exp_command_buffer_handle_t CommandBuffer) {
768769

769770
// Wait for the Copy Queue to finish at the end of the compute command list.
@@ -865,6 +866,7 @@ finalizeWaitEventPath(ur_exp_command_buffer_handle_t CommandBuffer) {
865866
ur_result_t
866867
urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t CommandBuffer) {
867868
UR_ASSERT(CommandBuffer, UR_RESULT_ERROR_INVALID_NULL_POINTER);
869+
UR_ASSERT(!CommandBuffer->IsFinalized, UR_RESULT_ERROR_INVALID_OPERATION);
868870

869871
// It is not allowed to append to command list from multiple threads.
870872
std::scoped_lock<ur_shared_mutex> Guard(CommandBuffer->Mutex);

source/adapters/opencl/command_buffer.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
124124

125125
UR_APIEXPORT ur_result_t UR_APICALL
126126
urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) {
127+
UR_ASSERT(!hCommandBuffer->IsFinalized, UR_RESULT_ERROR_INVALID_OPERATION);
127128
cl_context CLContext = cl_adapter::cast<cl_context>(hCommandBuffer->hContext);
128129
cl_ext::clFinalizeCommandBufferKHR_fn clFinalizeCommandBufferKHR = nullptr;
129130
UR_RETURN_ON_FAILURE(

source/loader/ur_libapi.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -7547,6 +7547,7 @@ ur_result_t UR_APICALL urCommandBufferReleaseExp(
75477547
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
75487548
/// + `NULL == hCommandBuffer`
75497549
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP
7550+
/// - ::UR_RESULT_ERROR_INVALID_OPERATION - "If `hCommandBuffer` has already been finalized"
75507551
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
75517552
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
75527553
ur_result_t UR_APICALL urCommandBufferFinalizeExp(

source/ur_api.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -6410,6 +6410,7 @@ ur_result_t UR_APICALL urCommandBufferReleaseExp(
64106410
/// - ::UR_RESULT_ERROR_INVALID_NULL_HANDLE
64116411
/// + `NULL == hCommandBuffer`
64126412
/// - ::UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP
6413+
/// - ::UR_RESULT_ERROR_INVALID_OPERATION - "If `hCommandBuffer` has already been finalized"
64136414
/// - ::UR_RESULT_ERROR_OUT_OF_HOST_MEMORY
64146415
/// - ::UR_RESULT_ERROR_OUT_OF_RESOURCES
64156416
ur_result_t UR_APICALL urCommandBufferFinalizeExp(

test/conformance/exp_command_buffer/commands.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,14 @@ TEST_P(urCommandBufferAppendKernelLaunchExpTest, Basic) {
204204
ASSERT_EQ(result, ptrZ[i]);
205205
}
206206
}
207+
208+
TEST_P(urCommandBufferAppendKernelLaunchExpTest, FinalizeTwice) {
209+
ASSERT_SUCCESS(urCommandBufferAppendKernelLaunchExp(
210+
cmd_buf_handle, kernel, n_dimensions, &global_offset, &global_size,
211+
&local_size, 0, nullptr, 0, nullptr, 0, nullptr, nullptr, nullptr,
212+
nullptr));
213+
214+
ASSERT_SUCCESS(urCommandBufferFinalizeExp(cmd_buf_handle));
215+
EXPECT_EQ_RESULT(urCommandBufferFinalizeExp(cmd_buf_handle),
216+
UR_RESULT_ERROR_INVALID_OPERATION);
217+
}

test/conformance/exp_command_buffer/exp_command_buffer_adapter_level_zero_v2.match

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ urCommandBufferCommandsTest.urCommandBufferAppendMemBufferFillExp/*
1414
urCommandBufferCommandsTest.urCommandBufferAppendUSMPrefetchExp/*
1515
urCommandBufferCommandsTest.urCommandBufferAppendUSMAdviseExp/*
1616
urCommandBufferAppendKernelLaunchExpTest.Basic/*
17+
urCommandBufferAppendKernelLaunchExpTest.FinalizeTwice/*
1718
urCommandBufferFillCommandsTest.Buffer/*
1819
urCommandBufferFillCommandsTest.USM/*
1920
KernelCommandEventSyncTest.Basic/*

test/conformance/exp_command_buffer/exp_command_buffer_adapter_native_cpu.match

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
{{OPT}}urCommandBufferRetainCommandExpTest.Success/*
66
{{OPT}}urCommandBufferRetainCommandExpTest.InvalidNullHandle/*
77
{{OPT}}urCommandBufferAppendKernelLaunchExpTest.Basic/*
8+
{{OPT}}urCommandBufferAppendKernelLaunchExpTest.FinalizeTwice/*
89
{{OPT}}BufferFillCommandTest.UpdateParameters/*
910
{{OPT}}BufferFillCommandTest.UpdateGlobalSize/*
1011
{{OPT}}BufferFillCommandTest.SeparateUpdateCalls/*

0 commit comments

Comments
 (0)