Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make command-buffer creation descriptor mandatory #2676

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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