Skip to content

Commit

Permalink
Make command-buffer creation descriptor mandatory
Browse files Browse the repository at this point in the history
Closes #2673
  • Loading branch information
EwanC committed Feb 12, 2025
1 parent 064d356 commit e23d115
Show file tree
Hide file tree
Showing 16 changed files with 49 additions and 40 deletions.
3 changes: 2 additions & 1 deletion include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -10104,6 +10104,7 @@ typedef struct ur_exp_command_buffer_command_handle_t_
/// + `NULL == hContext`
/// + `NULL == hDevice`
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
/// + `NULL == pCommandBufferDesc`
/// + `NULL == phCommandBuffer`
/// - ::UR_RESULT_ERROR_INVALID_CONTEXT
/// - ::UR_RESULT_ERROR_INVALID_DEVICE
Expand All @@ -10118,7 +10119,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer);
Expand Down
2 changes: 1 addition & 1 deletion scripts/core/exp-command-buffer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ params:
desc: "[in] Handle of the device object."
- type: "const $x_exp_command_buffer_desc_t*"
name: pCommandBufferDesc
desc: "[in][optional] command-buffer descriptor."
desc: "[in] Command-buffer descriptor."
- type: "$x_exp_command_buffer_handle_t*"
name: phCommandBuffer
desc: "[out] Pointer to command-Buffer handle."
Expand Down
5 changes: 1 addition & 4 deletions source/adapters/cuda/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext, ur_device_handle_t hDevice,
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
ur_exp_command_buffer_handle_t *phCommandBuffer) {

const bool IsUpdatable =
pCommandBufferDesc ? pCommandBufferDesc->isUpdatable : false;

const bool IsUpdatable = pCommandBufferDesc->isUpdatable;
try {
*phCommandBuffer =
new ur_exp_command_buffer_handle_t_(hContext, hDevice, IsUpdatable);
Expand Down
4 changes: 1 addition & 3 deletions source/adapters/hip/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext, ur_device_handle_t hDevice,
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
ur_exp_command_buffer_handle_t *phCommandBuffer) {
const bool IsUpdatable =
pCommandBufferDesc ? pCommandBufferDesc->isUpdatable : false;

const bool IsUpdatable = pCommandBufferDesc->isUpdatable;
try {
*phCommandBuffer =
new ur_exp_command_buffer_handle_t_(hContext, hDevice, IsUpdatable);
Expand Down
9 changes: 3 additions & 6 deletions source/adapters/level_zero/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,7 @@ bool canBeInOrder(ur_context_handle_t Context,
bool CompatibleDriver = Context->getPlatform()->isDriverVersionNewerOrSimilar(
1, 3, L0_DRIVER_INORDER_MIN_VERSION);
bool CanUseDriverInOrderLists = CompatibleDriver && DriverInOrderRequested;
return CanUseDriverInOrderLists
? (CommandBufferDesc ? CommandBufferDesc->isInOrder : false)
: false;
return CanUseDriverInOrderLists ? CommandBufferDesc->isInOrder : false;
}

/**
Expand Down Expand Up @@ -624,9 +622,8 @@ urCommandBufferCreateExp(ur_context_handle_t Context, ur_device_handle_t Device,
const ur_exp_command_buffer_desc_t *CommandBufferDesc,
ur_exp_command_buffer_handle_t *CommandBuffer) {
bool IsInOrder = canBeInOrder(Context, CommandBufferDesc);
bool EnableProfiling =
CommandBufferDesc && CommandBufferDesc->enableProfiling && !IsInOrder;
bool IsUpdatable = CommandBufferDesc && CommandBufferDesc->isUpdatable;
bool EnableProfiling = CommandBufferDesc->enableProfiling && !IsInOrder;
bool IsUpdatable = CommandBufferDesc->isUpdatable;
bool ImmediateAppendPath = checkImmediateAppendSupport(Context, Device);
const bool WaitEventPath = !ImmediateAppendPath;
bool UseCounterBasedEvents = checkCounterBasedEventsSupport(Device) &&
Expand Down
2 changes: 1 addition & 1 deletion source/adapters/mock/ur_mockddi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8395,7 +8395,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) try {
Expand Down
3 changes: 1 addition & 2 deletions source/adapters/opencl/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urCommandBufferCreateExp(
CLContext, cl_ext::ExtFuncPtrCache->clCreateCommandBufferKHRCache,
cl_ext::CreateCommandBufferName, &clCreateCommandBufferKHR));

const bool IsUpdatable =
pCommandBufferDesc ? pCommandBufferDesc->isUpdatable : false;
const bool IsUpdatable = pCommandBufferDesc->isUpdatable;

ur_device_command_buffer_update_capability_flags_t UpdateCapabilities;
cl_device_id CLDevice = cl_adapter::cast<cl_device_id>(hDevice);
Expand Down
2 changes: 1 addition & 1 deletion source/loader/layers/tracing/ur_trcddi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6970,7 +6970,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) {
Expand Down
5 changes: 4 additions & 1 deletion source/loader/layers/validation/ur_valddi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7607,7 +7607,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) {
Expand All @@ -7624,6 +7624,9 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferCreateExp(
if (NULL == hDevice)
return UR_RESULT_ERROR_INVALID_NULL_HANDLE;

if (NULL == pCommandBufferDesc)
return UR_RESULT_ERROR_INVALID_NULL_POINTER;

if (NULL == phCommandBuffer)
return UR_RESULT_ERROR_INVALID_NULL_POINTER;
}
Expand Down
2 changes: 1 addition & 1 deletion source/loader/ur_ldrddi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7036,7 +7036,7 @@ __urdlllocal ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) {
Expand Down
3 changes: 2 additions & 1 deletion source/loader/ur_libapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7612,6 +7612,7 @@ ur_result_t UR_APICALL urBindlessImagesSignalExternalSemaphoreExp(
/// + `NULL == hContext`
/// + `NULL == hDevice`
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
/// + `NULL == pCommandBufferDesc`
/// + `NULL == phCommandBuffer`
/// - ::UR_RESULT_ERROR_INVALID_CONTEXT
/// - ::UR_RESULT_ERROR_INVALID_DEVICE
Expand All @@ -7626,7 +7627,7 @@ ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) try {
Expand Down
3 changes: 2 additions & 1 deletion source/ur_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6657,6 +6657,7 @@ ur_result_t UR_APICALL urBindlessImagesSignalExternalSemaphoreExp(
/// + `NULL == hContext`
/// + `NULL == hDevice`
/// - ::UR_RESULT_ERROR_INVALID_NULL_POINTER
/// + `NULL == pCommandBufferDesc`
/// + `NULL == phCommandBuffer`
/// - ::UR_RESULT_ERROR_INVALID_CONTEXT
/// - ::UR_RESULT_ERROR_INVALID_DEVICE
Expand All @@ -6671,7 +6672,7 @@ ur_result_t UR_APICALL urCommandBufferCreateExp(
ur_context_handle_t hContext,
/// [in] Handle of the device object.
ur_device_handle_t hDevice,
/// [in][optional] command-buffer descriptor.
/// [in] Command-buffer descriptor.
const ur_exp_command_buffer_desc_t *pCommandBufferDesc,
/// [out] Pointer to command-Buffer handle.
ur_exp_command_buffer_handle_t *phCommandBuffer) {
Expand Down
29 changes: 16 additions & 13 deletions test/conformance/exp_command_buffer/fixtures.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ struct urCommandBufferExpTest : uur::urContextTest {
UUR_RETURN_ON_FATAL_FAILURE(uur::urContextTest::SetUp());

UUR_RETURN_ON_FATAL_FAILURE(checkCommandBufferSupport(device));

ur_exp_command_buffer_desc_t desc{
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, false, false, false,
};
ASSERT_SUCCESS(
urCommandBufferCreateExp(context, device, nullptr, &cmd_buf_handle));
urCommandBufferCreateExp(context, device, &desc, &cmd_buf_handle));
ASSERT_NE(cmd_buf_handle, nullptr);
}

Expand All @@ -83,8 +87,11 @@ struct urCommandBufferExpTestWithParam : urQueueTestWithParam<T> {
UUR_RETURN_ON_FATAL_FAILURE(uur::urQueueTestWithParam<T>::SetUp());

UUR_RETURN_ON_FATAL_FAILURE(checkCommandBufferSupport(this->device));
ASSERT_SUCCESS(urCommandBufferCreateExp(this->context, this->device,
nullptr, &cmd_buf_handle));

ur_exp_command_buffer_desc_t desc{UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC,
nullptr, false, false, false};
ASSERT_SUCCESS(urCommandBufferCreateExp(this->context, this->device, &desc,
&cmd_buf_handle));
ASSERT_NE(cmd_buf_handle, nullptr);
}

Expand All @@ -105,8 +112,11 @@ struct urCommandBufferExpExecutionTest : uur::urKernelExecutionTest {
UUR_RETURN_ON_FATAL_FAILURE(uur::urKernelExecutionTest::SetUp());

UUR_RETURN_ON_FATAL_FAILURE(checkCommandBufferSupport(device));

ur_exp_command_buffer_desc_t desc{UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC,
nullptr, false, false, false};
ASSERT_SUCCESS(
urCommandBufferCreateExp(context, device, nullptr, &cmd_buf_handle));
urCommandBufferCreateExp(context, device, &desc, &cmd_buf_handle));
ASSERT_NE(cmd_buf_handle, nullptr);
}

Expand Down Expand Up @@ -333,15 +343,8 @@ struct urCommandEventSyncTest : urCommandBufferExpTest {
ASSERT_NE(buffer, nullptr);
}

// Create a command-buffer with update enabled.
ur_exp_command_buffer_desc_t desc{
/*.stype=*/UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC,
/*.pNext =*/nullptr,
/*.isUpdatable =*/false,
/*.isInOrder =*/false,
/*.enableProfiling =*/false,
};

ur_exp_command_buffer_desc_t desc{UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC,
nullptr, true, false, false};
ASSERT_SUCCESS(urCommandBufferCreateExp(context, device, &desc,
&second_cmd_buf_handle));
ASSERT_NE(second_cmd_buf_handle, nullptr);
Expand Down
5 changes: 4 additions & 1 deletion test/conformance/exp_command_buffer/kernel_event_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ struct KernelCommandEventSyncTest
ASSERT_SUCCESS(urKernelSetArgPointer(kernel, 3, nullptr, device_ptrs[1]));

// Create second command-buffer
ASSERT_SUCCESS(urCommandBufferCreateExp(context, device, nullptr,
ur_exp_command_buffer_desc_t desc{
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, false, false, false,
};
ASSERT_SUCCESS(urCommandBufferCreateExp(context, device, &desc,
&second_cmd_buf_handle));
ASSERT_NE(second_cmd_buf_handle, nullptr);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,11 @@ TEST_P(InvalidUpdateTest, NotFinalizedCommandBuffer) {
TEST_P(InvalidUpdateTest, NotUpdatableCommandBuffer) {
// Create a command-buffer without isUpdatable
ur_exp_command_buffer_handle_t test_cmd_buf_handle = nullptr;
ur_exp_command_buffer_desc_t desc{
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, false, false, false,
};
ASSERT_SUCCESS(
urCommandBufferCreateExp(context, device, nullptr, &test_cmd_buf_handle));
urCommandBufferCreateExp(context, device, &desc, &test_cmd_buf_handle));
EXPECT_NE(test_cmd_buf_handle, nullptr);

// Append a kernel commands to command-buffer and close command-buffer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,12 @@ TEST_P(urMultiDeviceCommandBufferExpTest, Enqueue) {
}

// Create command-buffer
ur_exp_command_buffer_desc_t desc{
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_DESC, nullptr, false, false, false,
};
uur::raii::CommandBuffer cmd_buf_handle;
ASSERT_SUCCESS(urCommandBufferCreateExp(context, device, nullptr,
cmd_buf_handle.ptr()));
ASSERT_SUCCESS(
urCommandBufferCreateExp(context, device, &desc, cmd_buf_handle.ptr()));

// Append kernel command to command-buffer and close command-buffer
ASSERT_SUCCESS(urCommandBufferAppendKernelLaunchExp(
Expand Down

0 comments on commit e23d115

Please sign in to comment.