From f685bb3b9a3b6ff49cd188987c5768cd1029aef1 Mon Sep 17 00:00:00 2001 From: Hugh Delaney Date: Thu, 14 Nov 2024 16:04:33 +0000 Subject: [PATCH] Make double finalizing fail A command buffer should fail if finalize is called multiple times on the same command buffer. This matches openCL behaviour. --- source/adapters/cuda/command_buffer.cpp | 3 +++ source/adapters/cuda/command_buffer.hpp | 2 ++ source/adapters/hip/command_buffer.cpp | 3 +++ source/adapters/hip/command_buffer.hpp | 2 ++ source/adapters/level_zero/command_buffer.cpp | 2 ++ source/adapters/opencl/command_buffer.cpp | 2 ++ test/conformance/exp_command_buffer/commands.cpp | 12 ++++++++++++ 7 files changed, 26 insertions(+) diff --git a/source/adapters/cuda/command_buffer.cpp b/source/adapters/cuda/command_buffer.cpp index 527c339783..93321a9b27 100644 --- a/source/adapters/cuda/command_buffer.cpp +++ b/source/adapters/cuda/command_buffer.cpp @@ -401,6 +401,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) { UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) { + UR_ASSERT(!hCommandBuffer->IsFinalized, + UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP); try { const unsigned long long flags = 0; #if CUDA_VERSION >= 12000 @@ -418,6 +420,7 @@ urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) { } catch (...) { return UR_RESULT_ERROR_UNKNOWN; } + hCommandBuffer->IsFinalized = true; return UR_RESULT_SUCCESS; } diff --git a/source/adapters/cuda/command_buffer.hpp b/source/adapters/cuda/command_buffer.hpp index c82409104f..aa7968959b 100644 --- a/source/adapters/cuda/command_buffer.hpp +++ b/source/adapters/cuda/command_buffer.hpp @@ -352,6 +352,8 @@ struct ur_exp_command_buffer_handle_t_ { ur_device_handle_t Device; // Whether commands in the command-buffer can be updated bool IsUpdatable; + // Keep track of whether command buffer is finalized + bool IsFinalized = false; // Cuda Graph handle CUgraph CudaGraph; // Cuda Graph Exec handle diff --git a/source/adapters/hip/command_buffer.cpp b/source/adapters/hip/command_buffer.cpp index 9fed5db2f8..22b73c4dd4 100644 --- a/source/adapters/hip/command_buffer.cpp +++ b/source/adapters/hip/command_buffer.cpp @@ -306,6 +306,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) { UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) { + UR_ASSERT(!hCommandBuffer->IsFinalized, + UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP); try { const unsigned long long flags = 0; UR_CHECK_ERROR(hipGraphInstantiateWithFlags( @@ -313,6 +315,7 @@ urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) { } catch (...) { return UR_RESULT_ERROR_UNKNOWN; } + hCommandBuffer->IsFinalized = true; return UR_RESULT_SUCCESS; } diff --git a/source/adapters/hip/command_buffer.hpp b/source/adapters/hip/command_buffer.hpp index e162b8e640..27ad07b1cf 100644 --- a/source/adapters/hip/command_buffer.hpp +++ b/source/adapters/hip/command_buffer.hpp @@ -172,6 +172,8 @@ struct ur_exp_command_buffer_handle_t_ { ur_device_handle_t Device; // Whether commands in the command-buffer can be updated bool IsUpdatable; + // Keep track of whether command buffer is finalized + bool IsFinalized = false; // HIP Graph handle hipGraph_t HIPGraph; // HIP Graph Exec handle diff --git a/source/adapters/level_zero/command_buffer.cpp b/source/adapters/level_zero/command_buffer.cpp index 8f094ee373..f4770a1f02 100644 --- a/source/adapters/level_zero/command_buffer.cpp +++ b/source/adapters/level_zero/command_buffer.cpp @@ -595,6 +595,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t CommandBuffer) { ur_result_t urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t CommandBuffer) { UR_ASSERT(CommandBuffer, UR_RESULT_ERROR_INVALID_NULL_POINTER); + UR_ASSERT(!CommandBuffer->IsFinalized, + UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP); // It is not allowed to append to command list from multiple threads. std::scoped_lock Guard(CommandBuffer->Mutex); diff --git a/source/adapters/opencl/command_buffer.cpp b/source/adapters/opencl/command_buffer.cpp index a161a5b32b..c723d7ef7d 100644 --- a/source/adapters/opencl/command_buffer.cpp +++ b/source/adapters/opencl/command_buffer.cpp @@ -124,6 +124,8 @@ urCommandBufferReleaseExp(ur_exp_command_buffer_handle_t hCommandBuffer) { UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferFinalizeExp(ur_exp_command_buffer_handle_t hCommandBuffer) { + UR_ASSERT(!hCommandBuffer->IsFinalized, + UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP); cl_context CLContext = cl_adapter::cast(hCommandBuffer->hContext); cl_ext::clFinalizeCommandBufferKHR_fn clFinalizeCommandBufferKHR = nullptr; UR_RETURN_ON_FAILURE( diff --git a/test/conformance/exp_command_buffer/commands.cpp b/test/conformance/exp_command_buffer/commands.cpp index 49b2444176..5d0fefad32 100644 --- a/test/conformance/exp_command_buffer/commands.cpp +++ b/test/conformance/exp_command_buffer/commands.cpp @@ -204,3 +204,15 @@ TEST_P(urCommandBufferAppendKernelLaunchExpTest, Basic) { ASSERT_EQ(result, ptrZ[i]); } } + +TEST_P(urCommandBufferAppendKernelLaunchExpTest, FinalizeTwice) { + ASSERT_SUCCESS(urCommandBufferAppendKernelLaunchExp( + cmd_buf_handle, kernel, n_dimensions, &global_offset, &global_size, + &local_size, 0, nullptr, 0, nullptr, 0, nullptr, nullptr, nullptr, + nullptr)); + + ASSERT_SUCCESS(urCommandBufferFinalizeExp(cmd_buf_handle)); + EXPECT_EQ_RESULT(urCommandBufferFinalizeExp(cmd_buf_handle), + UR_RESULT_ERROR_INVALID_COMMAND_BUFFER_EXP); + +}